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.
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