Re: clang-10 warning in hash.c
Hi Bruno, On 1/27/20 2:09 PM, Bruno Haible wrote: >> Not sure if the compiler is correct here, but maybe worth a look: >> >> hash.c:549:11: error: implicit conversion from 'unsigned long' to >> 'float' changes value from 18446744073709551615 to 18446744073709551616 >> [-Werror,-Wimplicit-int-float-conversion] >> if (SIZE_MAX <= new_candidate) >> ^~~~ ~~ >> /usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX' >> # define SIZE_MAX (18446744073709551615UL) >> ^~ > > This warning is pointless, because > - Since the next float below 18446744073709551616 = 0x1 > would be 18446742974197923840 = 0xFF00 > the comparison result is the same for the two values ...615 and ...616. > - The compiler inserts the implicit conversion only because of the '<=' > operator. > IMO you should file a ticket with the clang people. > > Inserting a cast to 'double' > > if ((double) SIZE_MAX <= (double) new_candidate) > > would not help, because > the next double-float below 18446744073709551616 = 0x1 > would be18446744073709549568 = 0xF800 Thanks for the understandable explanation ! Sadly, bugs.llvm.org disabled self-registration. So they won't get a bug report from me (looks like they want to encapsulate from the rest of the world). Instead -Wno-implicit-int-float-conversion will be added to the clang options. Regards, Tim signature.asc Description: OpenPGP digital signature
Re: clang-10 warning in hash.c
I wrote: > And I dislike it because the transformation is correct only if > (float) SIZE_MAX >= SIZE_MAX. In the other case, > (float) SIZE_MAX < SIZE_MAX, the transformation is incorrect > [think of the particular value new_candidate = (float) SIZE_MAX]. Ouch, I was assuming that the comparison should be done according to exact mathematical value (infinite precision), like it is in Common Lisp. It's not the case in C! The integer gets rounded to the floating-point type first, leading to surprising results: === #include volatile unsigned long long pi; volatile long long ni; volatile float pf = 65536.0f * 65536.0f * 65536.0f * 65536.0f; volatile float nf = - 32768.0f * 65536.0f; int compare1 (void) { return pi <= pf; } int compare1ld (void) { return (long double) pi <= (long double) pf; } int compare2 (void) { return ni <= nf; } int compare2d (void) { return (double) ni <= (double) nf; } int main () { pi = 0xUL; printf ("%d %d\n", compare1 (), compare1ld ()); ni = -0x7FFF; printf ("%d %d\n", compare2 (), compare2d ()); } === Produces: 1 1 1 0 And with '<' instead of '<=': === #include volatile unsigned long long pi; volatile long long ni; volatile float pf = 65536.0f * 65536.0f * 65536.0f * 65536.0f; volatile float nf = - 32768.0f * 65536.0f; int compare1 (void) { return pi < pf; } int compare1ld (void) { return (long double) pi < (long double) pf; } int compare2 (void) { return ni < nf; } int compare2d (void) { return (double) ni < (double) nf; } int main () { pi = 0xUL; printf ("%d %d\n", compare1 (), compare1ld ()); ni = -0x7FFF; printf ("%d %d\n", compare2 (), compare2d ()); } === Produces: 0 1 0 0 Bruno
Re: clang-10 warning in hash.c
Paul Eggert wrote: > In the past I've worked around problems like these by writing things > like "SIZE_MAX + 1.0f <= new_candidate" instead of "SIZE_MAX <= > new_candidate" This may deserve a warning that adding 1.0f does not change the value. > Presumably the Clang folks want us to insert a cast to 'float', e.g., > "(float) SIZE_MAX <= new_candidate". However, I dislike casts because > they're too powerful. And I dislike it because the transformation is correct only if (float) SIZE_MAX >= SIZE_MAX. In the other case, (float) SIZE_MAX < SIZE_MAX, the transformation is incorrect [think of the particular value new_candidate = (float) SIZE_MAX]. The compiler should make correct transformations of comparisons; that's not the job of the programmer. > If there's no way to pacify Clang without casting, then I suggest > disabling the warning instead. I agree. When a compiler makes a normal and correct optimization, we don't want to hear about it. Only when the compiler makes a dangerous optimization, we want to see a warning. Bruno
Re: clang-10 warning in hash.c
On 1/27/20 5:09 AM, Bruno Haible wrote: IMO you should file a ticket with the clang people. That sounds appropriate. In the past I've worked around problems like these by writing things like "SIZE_MAX + 1.0f <= new_candidate" instead of "SIZE_MAX <= new_candidate" (partly under the argument that this better matches the mathematical comparison we're actually trying to do), but it sounds like Clang 10 would warn about that too. Inserting a cast to 'double' if ((double) SIZE_MAX <= (double) new_candidate) would not help Presumably the Clang folks want us to insert a cast to 'float', e.g., "(float) SIZE_MAX <= new_candidate". However, I dislike casts because they're too powerful. If there's no way to pacify Clang without casting, then I suggest disabling the warning instead. In GCC, we already disable Clang's -Wswitch, -Wpointer-sign, -Wstring-plus-int and -Wunknown-attributes warnings, and perhaps after Clang 10 comes out we'll disable -Wimplicit-int-float-conversion too. Some diagnostics are just too much trouble to pacify.
Re: clang-10 warning in hash.c
Hi Tim, > Not sure if the compiler is correct here, but maybe worth a look: > > hash.c:549:11: error: implicit conversion from 'unsigned long' to > 'float' changes value from 18446744073709551615 to 18446744073709551616 > [-Werror,-Wimplicit-int-float-conversion] > if (SIZE_MAX <= new_candidate) > ^~~~ ~~ > /usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX' > # define SIZE_MAX (18446744073709551615UL) > ^~ This warning is pointless, because - Since the next float below 18446744073709551616 = 0x1 would be 18446742974197923840 = 0xFF00 the comparison result is the same for the two values ...615 and ...616. - The compiler inserts the implicit conversion only because of the '<=' operator. IMO you should file a ticket with the clang people. Inserting a cast to 'double' if ((double) SIZE_MAX <= (double) new_candidate) would not help, because the next double-float below 18446744073709551616 = 0x1 would be18446744073709549568 = 0xF800 Bruno
clang-10 warning in hash.c
Not sure if the compiler is correct here, but maybe worth a look: hash.c:549:11: error: implicit conversion from 'unsigned long' to 'float' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion] if (SIZE_MAX <= new_candidate) ^~~~ ~~ /usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX' # define SIZE_MAX (18446744073709551615UL) ^~ hash.c:1079:15: error: implicit conversion from 'unsigned long' to 'float' changes value from 18446744073709551615 to 18446744073709551616 [-Werror,-Wimplicit-int-float-conversion] if (SIZE_MAX <= candidate) ^~~~ ~~ /usr/include/stdint.h:227:22: note: expanded from macro 'SIZE_MAX' # define SIZE_MAX (18446744073709551615UL) ^~ 2 errors generated. Regards, Tim signature.asc Description: OpenPGP digital signature