Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-07 Thread Yuya Watari
Hello Tom, Thank you for your comments. On Fri, Nov 8, 2019 at 1:30 AM Tom Lane wrote: > failure: not only did you fail to add any commentary about the new macros, > but you removed most of the commentary that had been in-line in the > existing usages. I apologize for the insufficient

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-07 Thread Tom Lane
Yuya Watari writes: > On Thu, Nov 7, 2019 at 3:10 PM Kyotaro Horiguchi > wrote: >> + if (unlikely(!FLOAT8_FITS_IN_INT32(num)) || isnan(num)) >> If compiler doesn't any fancy, num is fed to an arithmetic before >> checking if it is NaN. That seems have a chance of exception. > Thank you

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-07 Thread Yuya Watari
Hello Horiguchi-san, On Thu, Nov 7, 2019 at 3:10 PM Kyotaro Horiguchi wrote: > Mmm? See the bit in the patch cited below (v5). > > + /* Range check */ > + if (unlikely(!FLOAT8_FITS_IN_INT32(num)) || isnan(num)) > > If compiler doesn't any fancy, num is fed to an arithmetic before >

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-06 Thread Kyotaro Horiguchi
At Wed, 6 Nov 2019 13:56:46 +0900, Yuya Watari wrote in > Hello Tom, Thomas, and Andrew, > > > Tom> That commit presumes that floats follow the IEEE bitwise > > Tom> representation, I think; > > > > Correct. (It notably does _not_ make any assumptions about how floating > > point arithmetic

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Yuya Watari
Hello Tom, Thomas, and Andrew, > Tom> That commit presumes that floats follow the IEEE bitwise > Tom> representation, I think; > > Correct. (It notably does _not_ make any assumptions about how floating > point arithmetic or comparisons work - all the computation is done in > integers.) > >

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Andrew Gierth
> "Tom" == Tom Lane writes: >> But PostgreSQL effectively requires IEEE 754 since commit >> 02ddd499322ab6f2f0d58692955dc9633c2150fc, right? Tom> That commit presumes that floats follow the IEEE bitwise Tom> representation, I think; Correct. (It notably does _not_ make any assumptions

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Tom Lane
Thomas Munro writes: > On Wed, Nov 6, 2019 at 3:33 PM Yuya Watari wrote: >> However, this behavior depends on the platform architecture. As you >> have said, C language does not always follow IEEE-754. I think adding >> explicit checking of NaN is necessary. > I'm curious about this point. C

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Thomas Munro
On Wed, Nov 6, 2019 at 3:33 PM Yuya Watari wrote: > However, this behavior depends on the platform architecture. As you > have said, C language does not always follow IEEE-754. I think adding > explicit checking of NaN is necessary. I'm curious about this point. C may not require IEEE 754 (for

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Yuya Watari
Hello Tom, Thank you for replying. On Wed, Nov 6, 2019 at 12:04 AM Tom Lane wrote: > > Yuya Watari writes: > > The added macro FLOAT8_FITS_IN_INT32() does not check NaN explicitly, > > but it sufficiently handles the case. > > Really? I don't think anything is guaranteed about how a NaN will

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Tom Lane
Yuya Watari writes: > The added macro FLOAT8_FITS_IN_INT32() does not check NaN explicitly, > but it sufficiently handles the case. Really? I don't think anything is guaranteed about how a NaN will compare when using C's non-NaN-aware comparison operators. My thought about this was to annotate

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-05 Thread Yuya Watari
Hello Tom and Horiguchi-san, On Tue, Nov 5, 2019 at 1:59 PM Kyotaro Horiguchi wrote: > At Mon, 04 Nov 2019 12:53:48 -0500, Tom Lane wrote in > > I do concur with creating a macro that encapsulates a correct version > > of this test, maybe like > > > > #define DOUBLE_FITS_IN_INT64(num) \ > >

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-04 Thread Kyotaro Horiguchi
At Mon, 04 Nov 2019 12:53:48 -0500, Tom Lane wrote in > Yuya Watari writes: > > I attached the modified patch. In the patch, I placed the macro in > > "src/include/c.h", but this may not be a good choice because c.h is > > widely included from a lot of files. Do you have any good ideas about >

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-11-04 Thread Tom Lane
Yuya Watari writes: > I attached the modified patch. In the patch, I placed the macro in > "src/include/c.h", but this may not be a good choice because c.h is > widely included from a lot of files. Do you have any good ideas about > its placement? I agree that there's an actual bug here; it can

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-10-02 Thread Yuya Watari
Horiguchi-san, On Tue, Oct 1, 2019 at 3:41 PM Kyotaro Horiguchi wrote: > I found a trick seems workable generically (*1). (2.0 * > (PG_INT64_MAX/2 + 1)) will generate the value next to the > PG_INT64_MAX based on some assumptions > (*1). IS_DOUBLE_SAFE_IN_INT64() below would be able to check if

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-10-01 Thread Kyotaro Horiguchi
Hello. At Fri, 27 Sep 2019 16:43:53 +0900, Yuya Watari wrote in > Hello, > > I add further information. This issue also has a problem about > *overflow checking*. > > The original code is as follows. > > src/backend/utils/adt/timestamp.c:3222 > - > if (result_double > PG_INT64_MAX ||

Re: Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-09-27 Thread Yuya Watari
Hello, I add further information. This issue also has a problem about *overflow checking*. The original code is as follows. src/backend/utils/adt/timestamp.c:3222 - if (result_double > PG_INT64_MAX || result_double < PG_INT64_MIN) ereport(ERROR,

Keep compiler silence (clang 10, implicit conversion from 'long' to 'double' )

2019-09-26 Thread Yuya Watari
Hello, I found the problem that clang compiler introduces warnings when building PostgreSQL. Attached patch fixes it. === Compiler version === clang version 10.0.0-svn372772-1~exp1+0~20190924181208.2504~1.gbpb209ff (trunk) Older versions of clang may not generate this warning. === Warning ===