From 64c32411549e1ea94a0e0ba557b90fa4e962d328 Mon Sep 17 00:00:00 2001
From: Gabor Horvath <xazax.hun@gmail.com>
Date: Wed, 9 Aug 2017 08:57:09 +0000
Subject: [PATCH] [Sema] Extend -Wenum-compare to handle mixed enum comparisons
 in switch statements

Patch by: Reka Nikolett Kovacs

Differential Revision: https://reviews.llvm.org/D36407

llvm-svn: 310449
---
 clang/lib/Sema/SemaStmt.cpp              | 36 +++++++++++++++++++++++++-------
 clang/test/Sema/switch.c                 |  1 +
 clang/test/SemaCXX/warn-enum-compare.cpp | 17 +++++++++++++++
 3 files changed, 46 insertions(+), 8 deletions(-)

diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 2a38a1f..6bdc71b 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -602,14 +602,14 @@ static bool EqEnumVals(const std::pair<llvm::APSInt, EnumConstantDecl*>& lhs,
 
 /// 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))
-    expr = cleanups->getSubExpr();
-  while (ImplicitCastExpr *impcast = dyn_cast<ImplicitCastExpr>(expr)) {
-    if (impcast->getCastKind() != CK_IntegralCast) break;
-    expr = impcast->getSubExpr();
+static QualType GetTypeBeforeIntegralPromotion(const Expr *&E) {
+  if (const auto *CleanUps = dyn_cast<ExprWithCleanups>(E))
+    E = CleanUps->getSubExpr();
+  while (const auto *ImpCast = dyn_cast<ImplicitCastExpr>(E)) {
+    if (ImpCast->getCastKind() != CK_IntegralCast) break;
+    E = ImpCast->getSubExpr();
   }
-  return expr->getType();
+  return E->getType();
 }
 
 ExprResult Sema::CheckSwitchCondition(SourceLocation SwitchLoc, Expr *Cond) {
@@ -743,6 +743,24 @@ static bool ShouldDiagnoseSwitchCaseNotInEnum(const Sema &S,
   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 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
 
   QualType CondType = CondExpr->getType();
 
-  Expr *CondExprBeforePromotion = CondExpr;
+  const Expr *CondExprBeforePromotion = CondExpr;
   QualType CondTypeBeforePromotion =
       GetTypeBeforeIntegralPromotion(CondExprBeforePromotion);
 
@@ -843,6 +861,8 @@ Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
         break;
       }
 
+      checkEnumTypesInSwitchStmt(*this, CondExpr, Lo);
+
       llvm::APSInt LoVal;
 
       if (getLangOpts().CPlusPlus11) {
diff --git a/clang/test/Sema/switch.c b/clang/test/Sema/switch.c
index 7aa695d..e4e4ee5 100644
--- a/clang/test/Sema/switch.c
+++ b/clang/test/Sema/switch.c
@@ -372,6 +372,7 @@ void switch_on_ExtendedEnum1(enum ExtendedEnum1 e) {
   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@-1 {{comparison of two values with different enumeration types ('enum ExtendedEnum1' and 'const enum ExtendedEnum1_unrelated')}}
   }
 }
 
diff --git a/clang/test/SemaCXX/warn-enum-compare.cpp b/clang/test/SemaCXX/warn-enum-compare.cpp
index 0c28794..d1f6beb 100644
--- a/clang/test/SemaCXX/warn-enum-compare.cpp
+++ b/clang/test/SemaCXX/warn-enum-compare.cpp
@@ -209,4 +209,21 @@ void test () {
   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')}}
+  }
 }
-- 
2.7.4