Suppress Wsign-conversion for enums with matching underlying type

[erichkeane]

Summary

As reported here: https://bugs.llvm.org/show_bug.cgi?id=34692

A non-defined enum with a backing type was always defaulting to
being treated as a signed type. IN the case where it IS defined,
the signed-ness of the actual items is used.

This patch uses the underlying type's signed-ness in the non-defined
case to test signed-comparision.

[efriedma]

lib/Sema/SemaChecking.cpp

8179

Maybe add a comment noting what can trigger this case?

test/SemaCXX/sign-conversion.cpp

21 ↗

(On Diff #116220)

There's an existing file test/SemaCXX/warn-sign-conversion.cpp for C++ sign-conversion tests.

[erichkeane]

Updated based on Eli's feedback.

[efriedma]

lib/Sema/SemaChecking.cpp

8176

Explicitly referencing sign-conversion warnings here isn't really helpful. Maybe something more like "Incomplete enums without definitions can have an explicitly specified underlying type. Use that type here to compute the range."

test/SemaCXX/warn-sign-conversion-cpp11.cpp

11

The signedness of "char" can vary based on the host; probably simplest to write out "signed char".

[erichkeane]

lib/Sema/SemaChecking.cpp

8176

Ok, got it. I DO note that (IIRC) enums with an underlying type are not considered 'incomplete', so I modified your text slightly. Thanks for the help.

test/SemaCXX/sign-conversion.cpp

21 ↗

(On Diff #116220)

Thanks for pointing that out! Unfortunately, that file is very dependent on it being C++98, and this test requires c++11 or greater. I DID rename the test file to better match that one however.

test/SemaCXX/warn-sign-conversion-cpp11.cpp

11

Right, good catch.

[efriedma]

LGTM

lib/Sema/SemaChecking.cpp

static IntRange forValueOfCanonicalType(ASTContext &C, const Type *T) {
     if (const ComplexType *CT = dyn_cast<ComplexType>(T))
       T = CT->getElementType().getTypePtr();
     if (const AtomicType *AT = dyn_cast<AtomicType>(T))
       T = AT->getValueType().getTypePtr();
 
     // For enum types, use the known bit width of the enumerators.
     if (const EnumType *ET = dyn_cast<EnumType>(T)) {
       EnumDecl *Enum = ET->getDecl();
+      // Enums without definitions should use the signed-ness of the
+      // underlying type, so that when someone specifies the type as
+      // "unsigned" it doesn't cause sign-conversion type warnings.
       if (!Enum->isCompleteDefinition())
-        return IntRange(C.getIntWidth(QualType(T, 0)), false);
+        return IntRange(C.getIntWidth(QualType(T, 0)),
+                        !ET->isSignedIntegerOrEnumerationType());
 
       unsigned NumPositive = Enum->getNumPositiveBits();
       unsigned NumNegative = Enum->getNumNegativeBits();
 
       if (NumNegative == 0)
         return IntRange(NumPositive, true/*NonNegative*/);
       else
         return IntRange(std::max(NumPositive + 1, NumNegative),

test/SemaCXX/warn-sign-conversion-cpp11.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify -Wsign-conversion -std=c++11 %s
+
+unsigned int test() {
+  short foo;
+  return foo; // expected-warning {{implicit conversion changes signedness}}
+
+}
+
+unsigned int test3() {
+  // For a non-defined enum, use the underlying type.
+  enum u8 : char;
+  u8 foo{static_cast<u8>(0)};
+  return foo; // expected-warning {{implicit conversion changes signedness}}
+
+}
+unsigned int test2() {
+  // For a non-defined enum, use the underlying type.
+  enum u8 : unsigned char;
+  u8 foo{static_cast<u8>(0)};
+  return foo;
+}