r319875 - Fix a bunch of wrong "tautological unsigned enum compare" diagnostics in C++.

Hans Wennborg via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 6 10:35:37 PST 2017


This made Clang start warning about unsigned vs enum compares on Windows.

$ echo 'enum E { foo }; bool f(unsigned a, E b) { return a == b; }' |
bin/clang -Wsign-compare -c -x c++ - -target i686-pc-win32
<stdin>:1:52: warning: comparison of integers of different signs:
'unsigned int' and 'E' [-Wsign-compare]
enum E { foo }; bool f(unsigned a, E b) { return a == b; }
                                                 ~ ^  ~

That's probably intentional and I think we can fix the ones that came
up, just wanted to let you know.


On Tue, Dec 5, 2017 at 7:00 PM, Richard Smith via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
> Author: rsmith
> Date: Tue Dec  5 19:00:51 2017
> New Revision: 319875
>
> URL: http://llvm.org/viewvc/llvm-project?rev=319875&view=rev
> Log:
> Fix a bunch of wrong "tautological unsigned enum compare" diagnostics in C++.
>
> An enumeration with a fixed underlying type can have any value in its
> underlying type, not just those spanned by the values of its enumerators.
>
> Modified:
>     cfe/trunk/lib/Sema/SemaChecking.cpp
>     cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=319875&r1=319874&r2=319875&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Tue Dec  5 19:00:51 2017
> @@ -8260,11 +8260,12 @@ struct IntRange {
>      } else if (const EnumType *ET = dyn_cast<EnumType>(T)) {
>        // For enum types in C++, use the known bit width of the enumerators.
>        EnumDecl *Enum = ET->getDecl();
> -      // In C++11, enums without definitions can have an explicitly specified
> -      // underlying type.  Use this type to compute the range.
> -      if (!Enum->isCompleteDefinition())
> +      // In C++11, enums can have a fixed underlying type. Use this type to
> +      // compute the range.
> +      if (Enum->isFixed()) {
>          return IntRange(C.getIntWidth(QualType(T, 0)),
>                          !ET->isSignedIntegerOrEnumerationType());
> +      }
>
>        unsigned NumPositive = Enum->getNumPositiveBits();
>        unsigned NumNegative = Enum->getNumNegativeBits();
>
> Modified: cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp?rev=319875&r1=319874&r2=319875&view=diff
> ==============================================================================
> --- cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp (original)
> +++ cfe/trunk/test/Sema/tautological-unsigned-enum-zero-compare.cpp Tue Dec  5 19:00:51 2017
> @@ -2,11 +2,11 @@
>  // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSIGNED -verify %s
>  // RUN: %clang_cc1 -std=c++11 -triple=x86_64-pc-win32 -fsyntax-only -DSILENCE -Wno-tautological-unsigned-enum-zero-compare -verify %s
>
> -// Okay, this is where it gets complicated.
> -// Then default enum sigdness is target-specific.
> -// On windows, it is signed by default. We do not want to warn in that case.
> -
>  int main() {
> +  // On Windows, all enumerations have a fixed underlying type, which is 'int'
> +  // if not otherwise specified, so A is identical to C on Windows. Otherwise,
> +  // we follow the C++ rules, which say that the only valid values of A are 0
> +  // and 1.
>    enum A { A_foo = 0, A_bar, };
>    enum A a;
>
> @@ -87,21 +87,23 @@ int main() {
>
>    if (c < 0)
>      return 0;
> -  if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}
> +  if (0 >= c)
>      return 0;
> -  if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}
> +  if (c > 0)
>      return 0;
>    if (0 <= c)
>      return 0;
> -  if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}
> +  if (c <= 0)
>      return 0;
>    if (0 > c)
>      return 0;
>    if (c >= 0)
>      return 0;
> -  if (0 < c) // expected-warning {{0 < 'enum C' is always false}}
> +  if (0 < c)
>      return 0;
>
> +  // FIXME: These diagnostics are terrible. The issue here is that the signed
> +  // enumeration value was promoted to an unsigned type.
>    if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
>      return 0;
>    if (0U >= c)
> @@ -121,21 +123,23 @@ int main() {
>  #elif defined(SIGNED)
>    if (a < 0)
>      return 0;
> -  if (0 >= a) // expected-warning {{comparison 0 >= 'enum A' is always true}}
> +  if (0 >= a)
>      return 0;
> -  if (a > 0) // expected-warning {{comparison 'enum A' > 0 is always false}}
> +  if (a > 0)
>      return 0;
>    if (0 <= a)
>      return 0;
> -  if (a <= 0) // expected-warning {{comparison 'enum A' <= 0 is always true}}
> +  if (a <= 0)
>      return 0;
>    if (0 > a)
>      return 0;
>    if (a >= 0)
>      return 0;
> -  if (0 < a) // expected-warning {{comparison 0 < 'enum A' is always false}}
> +  if (0 < a)
>      return 0;
>
> +  // FIXME: As above, the issue here is that the enumeration is promoted to
> +  // unsigned.
>    if (a < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
>      return 0;
>    if (0U >= a)
> @@ -189,19 +193,19 @@ int main() {
>
>    if (c < 0)
>      return 0;
> -  if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}
> +  if (0 >= c)
>      return 0;
> -  if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}
> +  if (c > 0)
>      return 0;
>    if (0 <= c)
>      return 0;
> -  if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}
> +  if (c <= 0)
>      return 0;
>    if (0 > c)
>      return 0;
>    if (c >= 0)
>      return 0;
> -  if (0 < c) // expected-warning {{0 < 'enum C' is always false}}
> +  if (0 < c)
>      return 0;
>
>    if (c < 0U) // expected-warning {{comparison of unsigned enum expression < 0 is always false}}
> @@ -221,21 +225,22 @@ int main() {
>    if (0U < c)
>      return 0;
>  #else
> +  // expected-no-diagnostics
>    if (a < 0)
>      return 0;
> -  if (0 >= a) // expected-warning {{comparison 0 >= 'enum A' is always true}}
> +  if (0 >= a)
>      return 0;
> -  if (a > 0) // expected-warning {{comparison 'enum A' > 0 is always false}}
> +  if (a > 0)
>      return 0;
>    if (0 <= a)
>      return 0;
> -  if (a <= 0) // expected-warning {{comparison 'enum A' <= 0 is always true}}
> +  if (a <= 0)
>      return 0;
>    if (0 > a)
>      return 0;
>    if (a >= 0)
>      return 0;
> -  if (0 < a) // expected-warning {{comparison 0 < 'enum A' is always false}}
> +  if (0 < a)
>      return 0;
>
>    if (a < 0U)
> @@ -291,19 +296,19 @@ int main() {
>
>    if (c < 0)
>      return 0;
> -  if (0 >= c) // expected-warning {{comparison 0 >= 'enum C' is always true}}
> +  if (0 >= c)
>      return 0;
> -  if (c > 0) // expected-warning {{comparison 'enum C' > 0 is always false}}
> +  if (c > 0)
>      return 0;
>    if (0 <= c)
>      return 0;
> -  if (c <= 0) // expected-warning {{comparison 'enum C' <= 0 is always true}}
> +  if (c <= 0)
>      return 0;
>    if (0 > c)
>      return 0;
>    if (c >= 0)
>      return 0;
> -  if (0 < c) // expected-warning {{0 < 'enum C' is always false}}
> +  if (0 < c)
>      return 0;
>
>    if (c < 0U)
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


More information about the cfe-commits mailing list