Re: clang-10 warning in hash.c

2020-01-30 Thread Tim Rühsen
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

2020-01-27 Thread Bruno Haible
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

2020-01-27 Thread Bruno Haible
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

2020-01-27 Thread Paul Eggert

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

2020-01-27 Thread Bruno Haible
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

2020-01-27 Thread Tim Rühsen
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