clang-10 warning in hash.c

clang-10 warning in hash.c

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 969 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: clang-10 warning in hash.c

[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 = 0x10000000000000000
    would be                   18446742974197923840 =  0xFFFFFF0000000000
    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 = 0x10000000000000000
would be                    18446744073709549568 =  0xFFFFFFFFFFFFF800

Bruno

Re: clang-10 warning in hash.c

[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

[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

[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 <stdio.h>

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 = 0xFFFFFFFFFFFFFFFFUL;
  printf ("%d %d\n", compare1 (), compare1ld ());

  ni = -0x7FFFFFFF;
  printf ("%d %d\n", compare2 (), compare2d ());
}
===============================================================

Produces:

1 1
1 0

And with '<' instead of '<=':

===============================================================
#include <stdio.h>

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 = 0xFFFFFFFFFFFFFFFFUL;
  printf ("%d %d\n", compare1 (), compare1ld ());

  ni = -0x7FFFFFFF;
  printf ("%d %d\n", compare2 (), compare2d ());
}
===============================================================

Produces:

0 1
0 0

Bruno

Re: clang-10 warning in hash.c

[Tim Rühsen]

[-- Attachment #1.1: Type: text/plain, Size: 1580 bytes --]

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 = 0x10000000000000000
>     would be                   18446742974197923840 =  0xFFFFFF0000000000
>     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 = 0x10000000000000000
> would be                    18446744073709549568 =  0xFFFFFFFFFFFFF800

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: bugs.llvm.org

[Bruno Haible]

Tim Rühsen wrote:
> 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).

The text says: "New user self-registration is disabled due to spam.
For an account please email bugs-admin@lists.llvm.org with your e-mail
address and full name." In other words, you write a hand-written mail,
and someone adds your account manually. Same procedure as, for example,
for the bug tracker on dev.gnupg.org.

Bruno