Yeah, I moved this warning into own subgroup in rL373345. Thanks.
ut 1. 10. 2019 o 16:24 Nico Weber <tha...@chromium.org> napísal(a): > > Can we move this behind a Wenum-compare subgroup, say Wenum-compare-type? Our > codebase is (well, was) Wenum-compare clean, and this new warnings fires > pretty often and from a first quick glance the warning looks pretty > low-signal anyways (*). Maybe the subgroup shouldn't even be on by default -- > do you have any data on true / false positive rate of this? > > (But for starters, just having a way to turn this off is enough.) > > For example, we have a windows common control that's either a PGRP_DOWN or a > PGRP_UP page control and depending on which you store the control state in > the same int, then stuff like `return extra.inner_spin.spin_up ? UPS_DISABLED > : DNS_DISABLED;`. > > On Mon, Sep 30, 2019 at 3:53 PM David Bolvansky via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> >> Author: xbolva00 >> Date: Mon Sep 30 12:55:50 2019 >> New Revision: 373252 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=373252&view=rev >> Log: >> [Diagnostics] Warn if enumeration type mismatch in conditional expression >> >> Summary: >> - Useful warning >> - GCC compatibility (GCC warns in C++ mode) >> >> Reviewers: rsmith, aaron.ballman >> >> Reviewed By: aaron.ballman >> >> Subscribers: cfe-commits >> >> Tags: #clang >> >> Differential Revision: https://reviews.llvm.org/D67919 >> >> Added: >> cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c >> Modified: >> cfe/trunk/lib/Sema/SemaChecking.cpp >> >> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=373252&r1=373251&r2=373252&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Sep 30 12:55:50 2019 >> @@ -11308,6 +11308,32 @@ static const IntegerLiteral *getIntegerL >> return IL; >> } >> >> +static void CheckConditionalWithEnumTypes(Sema &S, SourceLocation Loc, >> + Expr *LHS, Expr *RHS) { >> + QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType(); >> + QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType(); >> + >> + const auto *LHSEnumType = LHSStrippedType->getAs<EnumType>(); >> + if (!LHSEnumType) >> + return; >> + const auto *RHSEnumType = RHSStrippedType->getAs<EnumType>(); >> + if (!RHSEnumType) >> + return; >> + >> + // Ignore anonymous enums. >> + if (!LHSEnumType->getDecl()->hasNameForLinkage()) >> + return; >> + if (!RHSEnumType->getDecl()->hasNameForLinkage()) >> + return; >> + >> + if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType)) >> + return; >> + >> + S.Diag(Loc, diag::warn_conditional_mixed_enum_types) >> + << LHSStrippedType << RHSStrippedType << LHS->getSourceRange() >> + << RHS->getSourceRange(); >> +} >> + >> static void DiagnoseIntInBoolContext(Sema &S, Expr *E) { >> E = E->IgnoreParenImpCasts(); >> SourceLocation ExprLoc = E->getExprLoc(); >> @@ -11799,6 +11825,8 @@ static void CheckConditionalOperator(Sem >> bool Suspicious = false; >> CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious); >> CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious); >> + CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(), >> + E->getFalseExpr()); >> >> if (T->isBooleanType()) >> DiagnoseIntInBoolContext(S, E); >> >> Added: cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c?rev=373252&view=auto >> ============================================================================== >> --- cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c (added) >> +++ cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c Mon Sep 30 >> 12:55:50 2019 >> @@ -0,0 +1,39 @@ >> +// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s >> +// RUN: %clang_cc1 -x c -fsyntax-only -verify %s >> +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s >> +// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s >> + >> +enum ro { A = 0x10 }; >> +enum rw { B = 0xFF }; >> +enum { C = 0x1A}; >> + >> +enum { >> + STATUS_SUCCESS, >> + STATUS_FAILURE, >> + MAX_BASE_STATUS_CODE >> +}; >> + >> +enum ExtendedStatusCodes { >> + STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000, >> +}; >> + >> + >> +int get_flag(int cond) { >> + return cond ? A : B; >> + #ifdef __cplusplus >> + // expected-warning@-2 {{enumeration type mismatch in conditional >> expression ('ro' and 'rw')}} >> + #else >> + // expected-no-diagnostics >> + #endif >> +} >> + >> +// In the following cases we purposefully differ from GCC and dont warn >> because >> +// this code pattern is quite sensitive and we dont want to produce so many >> false positives. >> + >> +int get_flag_anon_enum(int cond) { >> + return cond ? A : C; >> +} >> + >> +int foo(int c) { >> + return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS; >> +} >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
