Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-06-01 Thread Tom Lane
Chapman Flack writes: > It might be fun to see how big a chunk of the 4106 would vanish just > with the first tweak to one of the causes that's mentioned in a lot of > them. (Unless your figures were already after culling to distinct causes, > which would sound like a

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-06-01 Thread Chapman Flack
On 06/01/17 17:41, Tom Lane wrote: > 12169 warnings generated by -Wconversion > 4106 warnings generated by -Wconversion -Wno-sign-conversion > ... > So it's better with -Wno-sign-conversion, but I'd say we're still not > going there anytime soon. On an optimistic note, there might not turn out to

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-06-01 Thread Tom Lane
Chapman Flack writes: > On 05/31/2017 11:36 AM, Tom Lane wrote: >> However, I grant your point that some extensions may have outside >> constraints that mandate using -Wconversion, so to the extent that >> we can keep key headers like postgres.h from triggering those

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-31 Thread Chapman Flack
On 05/31/2017 11:36 AM, Tom Lane wrote: > However, I grant your point that some extensions may have outside > constraints that mandate using -Wconversion, so to the extent that > we can keep key headers like postgres.h from triggering those warnings, > it's probably worth doing. I suspect you're

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-31 Thread Tom Lane
Chapman Flack writes: > On 05/31/17 01:26, Tom Lane wrote: >> Hm. I think it would be better to use DatumGetInt32 here. Arguably, >> direct use of GET_4_BYTES and its siblings should only appear in >> DatumGetFoo macros. > Like so? These are the 4 sites where

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-30 Thread Tom Lane
Chapman Flack writes: > On 05/31/17 01:26, Tom Lane wrote: >> Hm. I think it would be better to use DatumGetInt32 here. Arguably, >> direct use of GET_4_BYTES and its siblings should only appear in >> DatumGetFoo macros. > Like so? Looks promising offhand, but I'm too

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-30 Thread Chapman Flack
On 05/31/17 01:26, Tom Lane wrote: > Hm. I think it would be better to use DatumGetInt32 here. Arguably, > direct use of GET_4_BYTES and its siblings should only appear in > DatumGetFoo macros. Like so? These are the 4 sites where {GET,SET}_n_BYTES got introduced in 14cca1b (for consistency,

Re: [HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-30 Thread Tom Lane
Chapman Flack writes: > - myunion.value = GET_4_BYTES(X); > + myunion.value = (int32)GET_4_BYTES(X); Hm. I think it would be better to use DatumGetInt32 here. Arguably, direct use of GET_4_BYTES and its siblings should only appear in DatumGetFoo macros.

[HACKERS] [PATCH] quiet conversion warning in DatumGetFloat4

2017-05-30 Thread Chapman Flack
It seems that 14cca1b (use static inline functions for float <-> Datum conversions) has an implicit narrowing conversion in one of those functions. If building an extension with gcc's -Wconversion warning enabled (*cough* pljava *cough* ... the Maven plugin that runs the compiler enables the