Hi,
The attached patch adds a warning for using |= without parentheses in
conditionals, as in:
if (a |= b) {}
and suggests replacing it with != or adding parentheses.
This is for http://llvm.org/bugs/show_bug.cgi?id=9001
Also, I considered whether it would make sense to warn for all
assignment operators in conditionals, and not just common typos (i.e.
= and |=). Do you think that would make sense?
Please review.
Thanks,
Hans
Index: test/SemaCXX/warn-assignment-condition.cpp
===================================================================
--- test/SemaCXX/warn-assignment-condition.cpp (revision 123815)
+++ test/SemaCXX/warn-assignment-condition.cpp (working copy)
@@ -3,6 +3,7 @@
struct A {
int foo();
friend A operator+(const A&, const A&);
+ A operator|=(const A&);
operator bool();
};
@@ -95,4 +96,13 @@
// expected-note{{use '==' to turn this assignment into an equality comparison}} \
// expected-note{{place parentheses around the assignment to silence this warning}}
for (; (a = b + b); ) {}
+
+ // Compound assignments.
+ if (x |= 2) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+ // expected-note{{use '!=' to turn this compound assignment into an inequality comparison}} \
+ // expected-note{{place parentheses around the assignment to silence this warning}}
+
+ if (a |= b) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \
+ // expected-note{{use '!=' to turn this compound assignment into an inequality comparison}} \
+ // expected-note{{place parentheses around the assignment to silence this warning}}
}
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td (revision 123815)
+++ include/clang/Basic/DiagnosticSemaKinds.td (working copy)
@@ -2796,6 +2796,8 @@
InGroup<DiagGroup<"idiomatic-parentheses">>, DefaultIgnore;
def note_condition_assign_to_comparison : Note<
"use '==' to turn this assignment into an equality comparison">;
+def note_condition_or_assign_to_comparison : Note<
+ "use '!=' to turn this compound assignment into an inequality comparison">;
def note_condition_assign_silence : Note<
"place parentheses around the assignment to silence this warning">;
Index: lib/Sema/SemaExpr.cpp
===================================================================
--- lib/Sema/SemaExpr.cpp (revision 123815)
+++ lib/Sema/SemaExpr.cpp (working copy)
@@ -9118,18 +9118,21 @@
return false;
}
-// Diagnose the common s/=/==/ typo. Note that adding parentheses
+// Diagnose the s/=/==/ and s/\|=/!=/ typos. Note that adding parentheses
// will prevent this condition from triggering, which is what we want.
void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
SourceLocation Loc;
unsigned diagnostic = diag::warn_condition_is_assignment;
+ bool IsOrAssign = false;
if (isa<BinaryOperator>(E)) {
BinaryOperator *Op = cast<BinaryOperator>(E);
- if (Op->getOpcode() != BO_Assign)
+ if (Op->getOpcode() != BO_Assign && Op->getOpcode() != BO_OrAssign)
return;
+ IsOrAssign = Op->getOpcode() == BO_OrAssign;
+
// Greylist some idioms by putting them into a warning subcategory.
if (ObjCMessageExpr *ME
= dyn_cast<ObjCMessageExpr>(Op->getRHS()->IgnoreParenCasts())) {
@@ -9149,9 +9152,10 @@
Loc = Op->getOperatorLoc();
} else if (isa<CXXOperatorCallExpr>(E)) {
CXXOperatorCallExpr *Op = cast<CXXOperatorCallExpr>(E);
- if (Op->getOperator() != OO_Equal)
+ if (Op->getOperator() != OO_Equal && Op->getOperator() != OO_PipeEqual)
return;
+ IsOrAssign = Op->getOperator() == OO_PipeEqual;
Loc = Op->getOperatorLoc();
} else {
// Not an assignment.
@@ -9162,8 +9166,14 @@
SourceLocation Close = PP.getLocForEndOfToken(E->getSourceRange().getEnd());
Diag(Loc, diagnostic) << E->getSourceRange();
- Diag(Loc, diag::note_condition_assign_to_comparison)
- << FixItHint::CreateReplacement(Loc, "==");
+
+ if (IsOrAssign)
+ Diag(Loc, diag::note_condition_or_assign_to_comparison)
+ << FixItHint::CreateReplacement(Loc, "!=");
+ else
+ Diag(Loc, diag::note_condition_assign_to_comparison)
+ << FixItHint::CreateReplacement(Loc, "==");
+
Diag(Loc, diag::note_condition_assign_silence)
<< FixItHint::CreateInsertion(Open, "(")
<< FixItHint::CreateInsertion(Close, ")");
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
