[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
This revision was automatically updated to reflect the committed changes.
Closed by commit rL310449: [Sema] Extend -Wenum-compare to handle mixed enum
comparisons in switch… (authored by xazax).
Changed prior to commit:
https://reviews.llvm.org/D36407?vs=110219=110333#toc
Repository:
rL LLVM
https://reviews.llvm.org/D36407
Files:
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/test/Sema/switch.c
cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
Index: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
===
--- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp
+++ cfe/trunk/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: cfe/trunk/test/Sema/switch.c
===
--- cfe/trunk/test/Sema/switch.c
+++ cfe/trunk/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@-1 {{comparison of two values with different enumeration types ('enum ExtendedEnum1' and 'const enum ExtendedEnum1_unrelated')}}
}
}
Index: cfe/trunk/lib/Sema/SemaStmt.cpp
===
--- cfe/trunk/lib/Sema/SemaStmt.cpp
+++ cfe/trunk/lib/Sema/SemaStmt.cpp
@@ -602,14 +602,14 @@
/// GetTypeBeforeIntegralPromotion - Returns the pre-promotion type of
/// potentially integral-promoted expression @p expr.
-static QualType GetTypeBeforeIntegralPromotion(Expr *) {
- if (ExprWithCleanups *cleanups = dyn_cast(expr))
-expr = cleanups->getSubExpr();
- while (ImplicitCastExpr *impcast = dyn_cast(expr)) {
-if (impcast->getCastKind() != CK_IntegralCast) break;
-expr = impcast->getSubExpr();
+static QualType GetTypeBeforeIntegralPromotion(const Expr *) {
+ if (const auto *CleanUps = dyn_cast(E))
+E = CleanUps->getSubExpr();
+ while (const auto *ImpCast = dyn_cast(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 @@
return true;
}
+static void checkEnumTypesInSwitchStmt(Sema , const Expr *Cond,
+ const Expr *Case) {
+ QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+ QualType CaseType = Case->getType();
+
+ const EnumType *CondEnumType = CondType->getAs();
+ const EnumType *CaseEnumType = CaseType->getAs();
+ 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) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
Aside from a minor naming nit, LGTM!
Comment at: lib/Sema/SemaStmt.cpp:605-608
+static QualType GetTypeBeforeIntegralPromotion(const Expr *) {
+ if (const auto *cleanups = dyn_cast(expr))
expr = cleanups->getSubExpr();
+ while (const auto *impcast = dyn_cast(expr)) {
Since you're touching the code -- can you change the names to `E` (or some
other, more descriptive name), `Cleanups` and `Impcast` to meet the coding
standards?
https://reviews.llvm.org/D36407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
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@-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 *) {
- if (ExprWithCleanups *cleanups = dyn_cast(expr))
+static QualType GetTypeBeforeIntegralPromotion(const Expr *) {
+ if (const auto *cleanups = dyn_cast(expr))
expr = cleanups->getSubExpr();
- while (ImplicitCastExpr *impcast = dyn_cast(expr)) {
+ while (const auto *impcast = dyn_cast(expr)) {
if (impcast->getCastKind() != CK_IntegralCast) break;
expr = impcast->getSubExpr();
}
@@ -743,6 +743,24 @@
return true;
}
+static void checkEnumTypesInSwitchStmt(Sema , const Expr *Cond,
+ const Expr *Case) {
+ QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+ QualType CaseType = Case->getType();
+
+ const EnumType *CondEnumType = CondType->getAs();
+ const EnumType *CaseEnumType = CaseType->getAs();
+ 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) {
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
[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
aaron.ballman added a comment.
Can you generate the updated patch with more context
(https://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface)?
Comment at: lib/Sema/SemaStmt.cpp:746
+static void checkEnumTypesInSwitchStmt(Sema , Expr *Cond, Expr *Case) {
+ QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
`Cond` and `Case` can be declared as `const` pointers.
Comment at: lib/Sema/SemaStmt.cpp:758
+
+ SourceLocation Loc = Case->getExprLoc();
+ S.Diag(Loc, diag::warn_comparison_of_mixed_enum_types)
You can lower this in to the diagnostic.
https://reviews.llvm.org/D36407
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36407: [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
rnkovacs created this revision.
`-Wenum-compare` warns if two values with different enumeration types are
compared in expressions with binary operators. This patch extends this
diagnostic so that comparisons of mixed enumeration types are recognized in
switch statements as well.
Example:
enum MixedA { A1, A2, A3 };
enum MixedB { B1, B2, B3 };
void MixedEnums(MixedA a) {
switch (a) {
case A1: break; // OK, same enum types.
case B1: break; // Warn, different enum types.
}
}
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@-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
@@ -743,6 +743,24 @@
return true;
}
+static void checkEnumTypesInSwitchStmt(Sema , Expr *Cond, Expr *Case) {
+ QualType CondType = GetTypeBeforeIntegralPromotion(Cond);
+ QualType CaseType = Case->getType();
+
+ const EnumType *CondEnumType = CondType->getAs();
+ const EnumType *CaseEnumType = CaseType->getAs();
+ if (!CondEnumType || !CaseEnumType)
+return;
+
+ if (S.Context.hasSameUnqualifiedType(CondType, CaseType))
+return;
+
+ SourceLocation Loc = Case->getExprLoc();
+ S.Diag(Loc, diag::warn_comparison_of_mixed_enum_types)
+ << CondType << CaseType << Cond->getSourceRange()
+ << Case->getSourceRange();
+}
+
StmtResult
Sema::ActOnFinishSwitchStmt(SourceLocation SwitchLoc, Stmt *Switch,
Stmt *BodyStmt) {
@@ -843,6 +861,8 @@
break;
}
+ checkEnumTypesInSwitchStmt(*this, CondExpr, Lo);
+
llvm::APSInt LoVal;
if (getLangOpts().CPlusPlus11) {
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@-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
+++
