[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements

[Reka Kovacs via Phabricator via cfe-commits]

rnkovacs updated this revision to Diff 110219.
rnkovacs marked 2 inline comments as done.
rnkovacs added a comment.

Uploaded the full diff and addressed comments. Added `const` qualifiers to `GetTypeBeforeIntegralPromotion()` function.


https://reviews.llvm.org/D36407

Files:
  lib/Sema/SemaStmt.cpp
  test/Sema/switch.c
  test/SemaCXX/warn-enum-compare.cpp


Index: test/SemaCXX/warn-enum-compare.cpp
===================================================================
--- test/SemaCXX/warn-enum-compare.cpp
+++ test/SemaCXX/warn-enum-compare.cpp
@@ -209,4 +209,21 @@
   while (getBar() > x); // expected-warning  {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
   while (getBar() < x); // expected-warning  {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
 
+  switch (a) {
+    case name1::F1: break;
+    case name1::F3: break;
+    case name2::B2: break; // expected-warning {{comparison of two values with different enumeration types ('name1::Foo' and 'name2::Baz')}}
+  }
+
+  switch (x) {
+    case FooB: break;
+    case FooC: break;
+    case BarD: break; // expected-warning {{comparison of two values with different enumeration types ('Foo' and 'Bar')}}
+  }
+
+  switch(getBar()) {
+    case BarE: break;
+    case BarF: break;
+    case FooA: break; // expected-warning {{comparison of two values with different enumeration types ('Bar' and 'Foo')}}
+  }
 }
Index: test/Sema/switch.c
===================================================================
--- test/Sema/switch.c
+++ test/Sema/switch.c
@@ -372,6 +372,7 @@
   case EE1_b: break;
   case EE1_c: break; // no-warning
   case EE1_d: break; // expected-warning {{case value not in enumerated type 'enum ExtendedEnum1'}}
+  // expected-warning at -1 {{comparison of two values with different enumeration types ('enum ExtendedEnum1' and 'const enum ExtendedEnum1_unrelated')}}
   }
 }
 
Index: lib/Sema/SemaStmt.cpp
===================================================================
--- lib/Sema/SemaStmt.cpp
+++ lib/Sema/SemaStmt.cpp
@@ -602,10 +602,10 @@
 
 /// GetTypeBeforeIntegralPromotion - Returns the pre-promotion type of
 /// potentially integral-promoted expression @p expr.
-static QualType GetTypeBeforeIntegralPromotion(Expr *&expr) {
-  if (ExprWithCleanups *cleanups = dyn_cast<ExprWithCleanups>(expr))
+static QualType GetTypeBeforeIntegralPromotion(const Expr *&expr) {
+  if (const auto *cleanups = dyn_cast<ExprWithCleanups>(expr))
     expr = cleanups->getSubExpr();
-  while (ImplicitCastExpr *impcast = dyn_cast<ImplicitCastExpr>(expr)) {
+  while (const auto *impcast = dyn_cast<ImplicitCastExpr>(expr)) {
     if (impcast->getCastKind() != CK_IntegralCast) break;
     expr = impcast->getSubExpr();
   }
@@ -743,6 +743,24 @@
   return true;
 }
 
+static void checkEnumTypesInSwitchStmt(Sema &S, const Expr *Cond,
+                                       const Expr *Case) {
+  QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+  QualType CaseType = Case->getType();
+
+  const EnumType *CondEnumType = CondType->getAs<EnumType>();
+  const EnumType *CaseEnumType = CaseType->getAs<EnumType>();
+  if (!CondEnumType || !CaseEnumType)
+    return;
+
+  if (S.Context.hasSameUnqualifiedType(CondType, CaseType))
+    return;
+
+  S.Diag(Case->getExprLoc(), diag::warn_comparison_of_mixed_enum_types)
+      << CondType << CaseType << Cond->getSourceRange()
+      << Case->getSourceRange();
+}
+
 StmtResult
 Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
                             Stmt *BodyStmt) {
@@ -760,7 +778,7 @@
 
   QualType CondType = CondExpr->getType();
 
-  Expr *CondExprBeforePromotion = CondExpr;
+  const Expr *CondExprBeforePromotion = CondExpr;
   QualType CondTypeBeforePromotion =
       GetTypeBeforeIntegralPromotion(CondExprBeforePromotion);
 
@@ -843,6 +861,8 @@
         break;
       }
 
+      checkEnumTypesInSwitchStmt(*this, CondExpr, Lo);
+
       llvm::APSInt LoVal;
 
       if (getLangOpts().CPlusPlus11) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D36407.110219.patch
Type: text/x-patch
Size: 3666 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170808/549b473c/attachment-0001.bin>