On Wed, Aug 9, 2017 at 10:07 AM, Nico Weber via cfe-commits <cfe-commits@lists.llvm.org> wrote: > Any way we could put this behind a different flag (say, > -Wenum-compare-switch, in the same group as -Wenum-compare, so that > -W(no-)enum-compare affects both)? Our codebase was -Wenum-compare clean > before this commit but is very not clean now, so we'd now have to disable > -Wenum-compare altogether, but then we won't catch regressions in non-switch > statements either.
I would not be opposed to that. ~Aaron > > On Wed, Aug 9, 2017 at 4:57 AM, Gabor Horvath via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Author: xazax >> Date: Wed Aug 9 01:57:09 2017 >> New Revision: 310449 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=310449&view=rev >> Log: >> [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch >> statements >> >> Patch by: Reka Nikolett Kovacs >> >> Differential Revision: https://reviews.llvm.org/D36407 >> >> Modified: >> cfe/trunk/lib/Sema/SemaStmt.cpp >> cfe/trunk/test/Sema/switch.c >> cfe/trunk/test/SemaCXX/warn-enum-compare.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaStmt.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaStmt.cpp?rev=310449&r1=310448&r2=310449&view=diff >> >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaStmt.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaStmt.cpp Wed Aug 9 01:57:09 2017 >> @@ -602,14 +602,14 @@ static bool EqEnumVals(const std::pair<l >> >> /// 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 ShouldDiagnoseSwitchCaseNotI >> 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(SourceLocati >> >> QualType CondType = CondExpr->getType(); >> >> - Expr *CondExprBeforePromotion = CondExpr; >> + const Expr *CondExprBeforePromotion = CondExpr; >> QualType CondTypeBeforePromotion = >> GetTypeBeforeIntegralPromotion(CondExprBeforePromotion); >> >> @@ -843,6 +861,8 @@ Sema::ActOnFinishSwitchStmt(SourceLocati >> break; >> } >> >> + checkEnumTypesInSwitchStmt(*this, CondExpr, Lo); >> + >> llvm::APSInt LoVal; >> >> if (getLangOpts().CPlusPlus11) { >> >> Modified: cfe/trunk/test/Sema/switch.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/switch.c?rev=310449&r1=310448&r2=310449&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/Sema/switch.c (original) >> +++ cfe/trunk/test/Sema/switch.c Wed Aug 9 01:57:09 2017 >> @@ -372,6 +372,7 @@ void switch_on_ExtendedEnum1(enum Extend >> 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')}} >> } >> } >> >> >> Modified: cfe/trunk/test/SemaCXX/warn-enum-compare.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-enum-compare.cpp?rev=310449&r1=310448&r2=310449&view=diff >> >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/warn-enum-compare.cpp (original) >> +++ cfe/trunk/test/SemaCXX/warn-enum-compare.cpp Wed Aug 9 01:57:09 2017 >> @@ -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')}} >> + } >> } >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits