<div dir="ltr">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?<div><br></div><div>(But for starters, just having a way to turn this off is enough.)<br><div><br></div><div>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;`.</div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Sep 30, 2019 at 3:53 PM David Bolvansky via cfe-commits <<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: xbolva00<br>
Date: Mon Sep 30 12:55:50 2019<br>
New Revision: 373252<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=373252&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=373252&view=rev</a><br>
Log:<br>
[Diagnostics] Warn if enumeration type mismatch in conditional expression<br>
<br>
Summary:<br>
- Useful warning<br>
- GCC compatibility (GCC warns in C++ mode)<br>
<br>
Reviewers: rsmith, aaron.ballman<br>
<br>
Reviewed By: aaron.ballman<br>
<br>
Subscribers: cfe-commits<br>
<br>
Tags: #clang<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D67919" rel="noreferrer" target="_blank">https://reviews.llvm.org/D67919</a><br>
<br>
Added:<br>
cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c<br>
Modified:<br>
cfe/trunk/lib/Sema/SemaChecking.cpp<br>
<br>
Modified: cfe/trunk/lib/Sema/SemaChecking.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=373252&r1=373251&r2=373252&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=373252&r1=373251&r2=373252&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)<br>
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Mon Sep 30 12:55:50 2019<br>
@@ -11308,6 +11308,32 @@ static const IntegerLiteral *getIntegerL<br>
return IL;<br>
}<br>
<br>
+static void CheckConditionalWithEnumTypes(Sema &S, SourceLocation Loc,<br>
+ Expr *LHS, Expr *RHS) {<br>
+ QualType LHSStrippedType = LHS->IgnoreParenImpCasts()->getType();<br>
+ QualType RHSStrippedType = RHS->IgnoreParenImpCasts()->getType();<br>
+<br>
+ const auto *LHSEnumType = LHSStrippedType->getAs<EnumType>();<br>
+ if (!LHSEnumType)<br>
+ return;<br>
+ const auto *RHSEnumType = RHSStrippedType->getAs<EnumType>();<br>
+ if (!RHSEnumType)<br>
+ return;<br>
+<br>
+ // Ignore anonymous enums.<br>
+ if (!LHSEnumType->getDecl()->hasNameForLinkage())<br>
+ return;<br>
+ if (!RHSEnumType->getDecl()->hasNameForLinkage())<br>
+ return;<br>
+<br>
+ if (S.Context.hasSameUnqualifiedType(LHSStrippedType, RHSStrippedType))<br>
+ return;<br>
+<br>
+ S.Diag(Loc, diag::warn_conditional_mixed_enum_types)<br>
+ << LHSStrippedType << RHSStrippedType << LHS->getSourceRange()<br>
+ << RHS->getSourceRange();<br>
+}<br>
+<br>
static void DiagnoseIntInBoolContext(Sema &S, Expr *E) {<br>
E = E->IgnoreParenImpCasts();<br>
SourceLocation ExprLoc = E->getExprLoc();<br>
@@ -11799,6 +11825,8 @@ static void CheckConditionalOperator(Sem<br>
bool Suspicious = false;<br>
CheckConditionalOperand(S, E->getTrueExpr(), T, CC, Suspicious);<br>
CheckConditionalOperand(S, E->getFalseExpr(), T, CC, Suspicious);<br>
+ CheckConditionalWithEnumTypes(S, E->getBeginLoc(), E->getTrueExpr(),<br>
+ E->getFalseExpr());<br>
<br>
if (T->isBooleanType())<br>
DiagnoseIntInBoolContext(S, E);<br>
<br>
Added: cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c?rev=373252&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c?rev=373252&view=auto</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c (added)<br>
+++ cfe/trunk/test/Sema/warn-conditional-emum-types-mismatch.c Mon Sep 30 12:55:50 2019<br>
@@ -0,0 +1,39 @@<br>
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wenum-compare %s<br>
+// RUN: %clang_cc1 -x c -fsyntax-only -verify %s<br>
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wenum-compare %s<br>
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify %s<br>
+<br>
+enum ro { A = 0x10 };<br>
+enum rw { B = 0xFF };<br>
+enum { C = 0x1A};<br>
+<br>
+enum {<br>
+ STATUS_SUCCESS,<br>
+ STATUS_FAILURE,<br>
+ MAX_BASE_STATUS_CODE<br>
+};<br>
+<br>
+enum ExtendedStatusCodes {<br>
+ STATUS_SOMETHING_INTERESTING = MAX_BASE_STATUS_CODE + 1000,<br>
+};<br>
+<br>
+<br>
+int get_flag(int cond) {<br>
+ return cond ? A : B; <br>
+ #ifdef __cplusplus<br>
+ // expected-warning@-2 {{enumeration type mismatch in conditional expression ('ro' and 'rw')}}<br>
+ #else <br>
+ // expected-no-diagnostics<br>
+ #endif<br>
+}<br>
+<br>
+// In the following cases we purposefully differ from GCC and dont warn because<br>
+// this code pattern is quite sensitive and we dont want to produce so many false positives.<br>
+<br>
+int get_flag_anon_enum(int cond) {<br>
+ return cond ? A : C;<br>
+}<br>
+<br>
+int foo(int c) {<br>
+ return c ? STATUS_SOMETHING_INTERESTING : STATUS_SUCCESS;<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div>