r310449 - [Sema] Extend -Wenum-compare to handle mixed enum comparisons in switch statements
Gábor Horváth via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 9 07:11:31 PDT 2017
Sure!
There was a follow-up patch that excluded anonymous enums. Is it still that
bad to introduce it under a new flag?
Regards,
Gábor
On 9 August 2017 at 16:07, Nico Weber <thakis at chromium.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.
>
> On Wed, Aug 9, 2017 at 4:57 AM, Gabor Horvath via cfe-commits <
> cfe-commits at 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/SemaS
>> tmt.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/swit
>> ch.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 at -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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170809/7afe5626/attachment-0001.html>
More information about the cfe-commits
mailing list