Re: Ryu floating point output patch - Mailing list pgsql-hackers

From Andrew Gierth
Subject Re: Ryu floating point output patch
Date
Msg-id 87mup9192t.fsf@news-spur.riddles.org.uk
Whole thread Raw
In response to Re: Ryu floating point output patch  (Andres Freund <andres@anarazel.de>)
Responses Re: Ryu floating point output patch
Re: Ryu floating point output patch
List pgsql-hackers
>>>>> "Andres" == Andres Freund <andres@anarazel.de> writes:

 >> From the upstream, I've taken only specific files which are
 >> Boost-licensed, removed code not of interest to us, removed
 >> C99-isms, applied project style for things like type names and use
 >> of INT64CONST, removed some ad-hoc configuration #ifs in favour of
 >> using values established by pg_config.h, and ran the whole thing
 >> through pgindent and fixed up the resulting wreckage.

 Andres> Removed C99isms? Why, we allow that these days? Did you mean
 Andres> C11?

My understanding is that we do not allow // comments or mixed
declarations and code (other than loop control variables).

This code in particular was very heavily into using mixed declarations
and code throughout. Removing those was moderately painful.

 Andres> What's with all the commented out code?

Just stuff from upstream I didn't take out.

 Andres> I'm not sure that keeping close alignment with the original
 Andres> codebase with things like that has much value given the extent
 Andres> of the reformatting and functional changes?

Probably not, but otherwise I did not remove much in the way of upstream
comments or edit them except for formatting.

 >> +/*  These tables are generated by PrintDoubleLookupTable. */

 Andres> This kind of reference is essentially dangling now, so perhaps
 Andres> we should rewrite that?

Well, it's probably still useful to point out that they're generated
(though not by us); I guess it should make clear where the generator is.

 >> +++ b/src/common/d2s_intrinsics.h

 >> +static inline uint64
 >> +umul128(const uint64 a, const uint64 b, uint64 *const productHi)
 >> +{
 >> +    return _umul128(a, b, productHi);
 >> +}

 Andres> These are fairly generic names, perhaps we ought to prefix
 Andres> them?

Maybe, but presumably nothing else is going to include this file.

-- 
Andrew (irc:RhodiumToad)


pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Reorganize collation lookup time and place
Next
From: Andres Freund
Date:
Subject: Re: Ryu floating point output patch