[clang] 882915d - Enum conversion warning when one signed and other unsigned.

[Aaron Ballman via cfe-commits]

Author: Micah Weston
Date: 2022-05-09T10:16:19-04:00
New Revision: 882915df61e33f3a2b7f58e52f572717e1c11499

URL: https://github.com/llvm/llvm-project/commit/882915df61e33f3a2b7f58e52f572717e1c11499
DIFF: https://github.com/llvm/llvm-project/commit/882915df61e33f3a2b7f58e52f572717e1c11499.diff

LOG: Enum conversion warning when one signed and other unsigned.

Ensures an -Wenum-conversion warning happens when one of the enums is
signed and the other is unsigned. Also adds a test file to verify these
warnings.

This warning would not happen since the -Wsign-conversion would make a
diagnostic then return, never allowing the -Wenum-conversion checks.

For example:

C
enum PE { P = -1 };
enum NE { N };
enum NE conv(enum PE E) { return E; }
Before this would only create a diagnostic with -Wsign-conversion and
never on -Wenum-conversion. Now it will create a diagnostic for both
-Wsign-conversion and -Wenum-conversion.

I could change it to just warn on -Wenum-conversion as that was what I
initially did. Seeing PR35200 (or GitHub Issue 316268), I let both
diagnostics check so that the sign conversion could generate a warning.

Added: 
    clang/test/Sema/enum-enum-conversion.c

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/lib/Sema/SemaChecking.cpp
    clang/test/Sema/enum-sign-conversion.c

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index a82ae8845e65..d4b38f3551fa 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -207,6 +207,9 @@ Improvements to Clang's diagnostics
 - ``-Wthread-safety-analysis`` now considers overloaded compound assignment and
   increment/decrement operators as writing to their first argument, thus
   requiring an exclusive lock if the argument is guarded.
+- ``-Wenum-conversion`` now warns on converting a signed enum of one type to an
+  unsigned enum of a 
diff erent type (or vice versa) rather than
+  ``-Wsign-conversion``.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 31459af5033f..0cdceb47b738 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13858,9 +13858,10 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
     // Fall through for non-constants to give a sign conversion warning.
   }
 
-  if ((TargetRange.NonNegative && !LikelySourceRange.NonNegative) ||
-      (!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
-       LikelySourceRange.Width == TargetRange.Width)) {
+  if ((!isa<EnumType>(Target) || !isa<EnumType>(Source)) &&
+      ((TargetRange.NonNegative && !LikelySourceRange.NonNegative) ||
+       (!TargetRange.NonNegative && LikelySourceRange.NonNegative &&
+        LikelySourceRange.Width == TargetRange.Width))) {
     if (S.SourceMgr.isInSystemMacro(CC))
       return;
 

diff  --git a/clang/test/Sema/enum-enum-conversion.c b/clang/test/Sema/enum-enum-conversion.c
new file mode 100644
index 000000000000..b6cfe722f63b
--- /dev/null
+++ b/clang/test/Sema/enum-enum-conversion.c
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -Wenum-conversion -verify %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -Wconversion -verify %s
+
+// Signed enums
+enum SE1 { N1 = -1 };
+enum SE2 { N2 = -2 };
+// Unsigned unums
+enum UE1 { P1 };
+enum UE2 { P2 };
+
+enum UE2 f1(enum UE1 E) {
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum UE1' to 
diff erent enumeration type 'enum UE2'}}
+}
+
+enum SE1 f2(enum UE1 E) {
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum UE1' to 
diff erent enumeration type 'enum SE1'}}
+}
+
+enum UE1 f3(enum SE1 E) {
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum SE1' to 
diff erent enumeration type 'enum UE1'}}
+}
+
+enum SE2 f4(enum SE1 E) {
+  return E; // expected-warning {{implicit conversion from enumeration type 'enum SE1' to 
diff erent enumeration type 'enum SE2'}}
+}

diff  --git a/clang/test/Sema/enum-sign-conversion.c b/clang/test/Sema/enum-sign-conversion.c
index 518fc670d314..ecc7de2da857 100644
--- a/clang/test/Sema/enum-sign-conversion.c
+++ b/clang/test/Sema/enum-sign-conversion.c
@@ -1,13 +1,45 @@
-// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -verify -DUNSIGNED -Wsign-conversion %s
-// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -verify -Wsign-conversion %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -Wsign-conversion -verify=unsigned,both %s
+// RUN: %clang_cc1 -triple=x86_64-pc-linux-gnu -fsyntax-only -Wconversion -verify=unsigned,both %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fsyntax-only -verify -Wsign-conversion -verify=win32,both %s
 
 // PR35200
 enum X { A,B,C};
 int f(enum X x) {
-#ifdef UNSIGNED
-  return x; // expected-warning {{implicit conversion changes signedness: 'enum X' to 'int'}}
-#else
-  // expected-no-diagnostics
-  return x;
-#endif
+  return x; // unsigned-warning {{implicit conversion changes signedness: 'enum X' to 'int'}}
+}
+
+enum SE1 { N1 = -1 }; // Always a signed underlying type.
+enum E1 { P1 };       // Unsigned underlying type except on Windows.
+
+// ensure no regression with enum to sign (related to enum-enum-conversion.c)
+int f1(enum E1 E) {
+  return E; // unsigned-warning {{implicit conversion changes signedness: 'enum E1' to 'int'}}
+}
+
+enum E1 f2(int E) {
+  return E; // unsigned-warning {{implicit conversion changes signedness: 'int' to 'enum E1'}}
+}
+
+int f3(enum SE1 E) {
+  return E; // shouldn't warn
+}
+
+enum SE1 f4(int E) {
+  return E; // shouldn't warn
+}
+
+unsigned f5(enum E1 E) {
+  return E; // win32-warning {{implicit conversion changes signedness: 'enum E1' to 'unsigned int'}}
+}
+
+enum E1 f6(unsigned E) {
+  return E; // win32-warning {{implicit conversion changes signedness: 'unsigned int' to 'enum E1'}}
+}
+
+unsigned f7(enum SE1 E) {
+  return E; // both-warning {{implicit conversion changes signedness: 'enum SE1' to 'unsigned int'}}
+}
+
+enum SE1 f8(unsigned E) {
+  return E; // both-warning {{implicit conversion changes signedness: 'unsigned int' to 'enum SE1'}}
 }