search

llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.13k stars 12.01k forks source link

isless( int, float ) not caught with -pedantic-errors #46139

Open dbc7d0cf-60de-4ba2-bc9a-c1b7219315df opened 4 years ago

dbc7d0cf-60de-4ba2-bc9a-c1b7219315df commented 4 years ago
Bugzilla Link 46795
Version 10.0
OS Linux
CC @DougGregor,@zygoloid

Extended Description

Using clang 10.0.0 on ARM (and I believe Intel x86) with -std-c2x -pedantic-errors is not flagging invalid code that uses any of the comparison operators (isless, islessequal, isgreater, isgreaterequal, ...) along with an argument that is not a real floating type. The C standard, since C99, has the requirement: In the synopses in this subclause [Comparison macros], real-floating indicates that the argument shall be an expression of real floating type. There is not even a warning.

dbc7d0cf-60de-4ba2-bc9a-c1b7219315df commented 4 years ago

In more testing, I do get error messages if both arguments to isless() are int or both are a complex floating point type.

Perhaps a warning (that it is undefined) would be enough for the mixed int and float.

ec04fc15-fa35-46f2-80e1-5d271f2ef708 commented 4 years ago

This rule is not in a Constraints section, and has an attached footnote that goes on to say: "If any argument is of integer type, or any other type that is not a real floating type, the behavior is undefined." So this doesn't seem to be a conformance issue.

So instead this is an extension, and appears to be an incredibly common one: at least GCC's builtin, ICC's implementation of that builtin, Clang's implementation of that builtin, glibc's implementation, uclibc's implementation, musl's implementation, and the MSVCRT implementation of isless all accept isless(0, 0.0f) without even a warning. I couldn't find any common implementation that doesn't share this extension.

Given the above, do you still think it would be worthwhile for us to diagnose this not-strictly-conforming code under -pedantic-errors? I'm not opposed, but it's not clear that doing so has utility. Perhaps a better course would be to suggest to WG14 that the usual arithmetic conversions should be performed and shall result in a real floating point type, which seems to largely match what implementations actually do.

AaronBallman commented 2 years ago

Perhaps a better course would be to suggest to WG14 that the usual arithmetic conversions should be performed and shall result in a real floating point type, which seems to largely match what implementations actually do.

+1 to this suggestion -- WG14 should standardize existing practice here, if possible. I would consider this to be a pretty low-quality pedantic warning unless there was a runtime issue stemming from it.