r373252 - [Diagnostics] Warn if enumeration type mismatch in conditional expression

[Dávid Bolvanský via cfe-commits]

Yeah, I moved this warning into own subgroup in rL373345. Thanks.

ut 1. 10. 2019 o 16:24 Nico Weber <thakis at 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 at 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 at -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 at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits