From: Douglas Gregor Date: Wed, 14 Apr 2010 16:09:52 +0000 (+0000) Subject: When diagnosing suspicious precedence or assignments, move the fix-it X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=55b38842d12ffd9f9ff3a0e16fae2cfe61ab0fe6;p=clang When diagnosing suspicious precedence or assignments, move the fix-it that adds parentheses from the main diagnostic down to a new note. This way, when the fix-it represents a choice between two options, each of the options is associted with a note. There is no default option in such cases. For example: /Users/dgregor/t.c:2:9: warning: & has lower precedence than ==; == will be evaluated first [-Wparentheses] if (x & y == 0) { ^~~~~~~~ /Users/dgregor/t.c:2:9: note: place parentheses around the & expression to evaluate it first if (x & y == 0) { ^ ( ) /Users/dgregor/t.c:2:9: note: place parentheses around the == expression to silence this warning if (x & y == 0) { ^ ( ) git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@101249 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index bb468ddb23..4b4780e09b 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1781,6 +1781,8 @@ def warn_precedence_bitwise_rel : Warning< InGroup; def note_precedence_bitwise_first : Note< "place parentheses around the %0 expression to evaluate it first">; +def note_precedence_bitwise_silence : Note< + "place parentheses around the %0 expression to silence this warning">; def err_sizeof_nonfragile_interface : Error< "invalid application of '%select{alignof|sizeof}1' to interface %0 in " @@ -2227,6 +2229,8 @@ def warn_condition_is_idiomatic_assignment : Warning<"using the result " InGroup>, DefaultIgnore; def note_condition_assign_to_comparison : Note< "use '==' to turn this assignment into an equality comparison">; +def note_condition_assign_silence : Note< + "place parentheses around the assignment to silence this warning">; def warn_value_always_zero : Warning< "%0 is always %select{zero|false|NULL}1 in this context">; diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 589ba472b7..d10ea6d4ab 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6268,33 +6268,37 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, /// ParenRange in parentheses. static void SuggestParentheses(Sema &Self, SourceLocation Loc, const PartialDiagnostic &PD, - SourceRange ParenRange, - const PartialDiagnostic &SecondPD, + const PartialDiagnostic &FirstNote, + SourceRange FirstParenRange, + const PartialDiagnostic &SecondNote, SourceRange SecondParenRange) { - SourceLocation EndLoc = Self.PP.getLocForEndOfToken(ParenRange.getEnd()); - if (!ParenRange.getEnd().isFileID() || EndLoc.isInvalid()) { - // We can't display the parentheses, so just dig the - // warning/error and return. - Self.Diag(Loc, PD); + Self.Diag(Loc, PD); + + if (!FirstNote.getDiagID()) + return; + + SourceLocation EndLoc = Self.PP.getLocForEndOfToken(FirstParenRange.getEnd()); + if (!FirstParenRange.getEnd().isFileID() || EndLoc.isInvalid()) { + // We can't display the parentheses, so just return. return; } - Self.Diag(Loc, PD) - << FixItHint::CreateInsertion(ParenRange.getBegin(), "(") + Self.Diag(Loc, FirstNote) + << FixItHint::CreateInsertion(FirstParenRange.getBegin(), "(") << FixItHint::CreateInsertion(EndLoc, ")"); - if (!SecondPD.getDiagID()) + if (!SecondNote.getDiagID()) return; EndLoc = Self.PP.getLocForEndOfToken(SecondParenRange.getEnd()); if (!SecondParenRange.getEnd().isFileID() || EndLoc.isInvalid()) { // We can't display the parentheses, so just dig the // warning/error and return. - Self.Diag(Loc, SecondPD); + Self.Diag(Loc, SecondNote); return; } - Self.Diag(Loc, SecondPD) + Self.Diag(Loc, SecondNote) << FixItHint::CreateInsertion(SecondParenRange.getBegin(), "(") << FixItHint::CreateInsertion(EndLoc, ")"); } @@ -6328,19 +6332,23 @@ static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperator::Opcode Opc, Self.PDiag(diag::warn_precedence_bitwise_rel) << SourceRange(lhs->getLocStart(), OpLoc) << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(lhsopc), - lhs->getSourceRange(), Self.PDiag(diag::note_precedence_bitwise_first) << BinOp::getOpcodeStr(Opc), - SourceRange(cast(lhs)->getRHS()->getLocStart(), rhs->getLocEnd())); + SourceRange(cast(lhs)->getRHS()->getLocStart(), rhs->getLocEnd()), + Self.PDiag(diag::note_precedence_bitwise_silence) + << BinOp::getOpcodeStr(lhsopc), + lhs->getSourceRange()); else if (BinOp::isComparisonOp(rhsopc)) SuggestParentheses(Self, OpLoc, Self.PDiag(diag::warn_precedence_bitwise_rel) << SourceRange(OpLoc, rhs->getLocEnd()) << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(rhsopc), - rhs->getSourceRange(), Self.PDiag(diag::note_precedence_bitwise_first) << BinOp::getOpcodeStr(Opc), - SourceRange(lhs->getLocEnd(), cast(rhs)->getLHS()->getLocStart())); + SourceRange(lhs->getLocEnd(), cast(rhs)->getLHS()->getLocStart()), + Self.PDiag(diag::note_precedence_bitwise_silence) + << BinOp::getOpcodeStr(rhsopc), + rhs->getSourceRange()); } /// DiagnoseBinOpPrecedence - Emit warnings for expressions with tricky @@ -7434,12 +7442,12 @@ void Sema::DiagnoseAssignmentAsCondition(Expr *E) { SourceLocation Open = E->getSourceRange().getBegin(); SourceLocation Close = PP.getLocForEndOfToken(E->getSourceRange().getEnd()); - Diag(Loc, diagnostic) - << E->getSourceRange() - << FixItHint::CreateInsertion(Open, "(") - << FixItHint::CreateInsertion(Close, ")"); + Diag(Loc, diagnostic) << E->getSourceRange(); Diag(Loc, diag::note_condition_assign_to_comparison) << FixItHint::CreateReplacement(Loc, "=="); + Diag(Loc, diag::note_condition_assign_silence) + << FixItHint::CreateInsertion(Open, "(") + << FixItHint::CreateInsertion(Close, ")"); } bool Sema::CheckBooleanCondition(Expr *&E, SourceLocation Loc) { diff --git a/test/Misc/tabstop.c b/test/Misc/tabstop.c index 66685c62d1..49c4d7bebb 100644 --- a/test/Misc/tabstop.c +++ b/test/Misc/tabstop.c @@ -39,12 +39,9 @@ void f(void) // CHECK-3: {{^ }}if (0 & 1 == 1) // CHECK-3: {{^ }} ( ) -// CHECK-3: {{^ }} ( ) // CHECK-4: {{^ }}if (0 & 1 == 1) // CHECK-4: {{^ }} ( ) -// CHECK-4: {{^ }} ( ) // CHECK-5: {{^ }}if (0 & 1 == 1) // CHECK-5: {{^ }} ( ) -// CHECK-5: {{^ }} ( ) diff --git a/test/Sema/parentheses.c b/test/Sema/parentheses.c index 69c91cb15f..e53f0eb99b 100644 --- a/test/Sema/parentheses.c +++ b/test/Sema/parentheses.c @@ -5,17 +5,21 @@ void if_assign(void) { int i; if (i = 4) {} // expected-warning {{assignment as a condition}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} if ((i = 4)) {} } void bitwise_rel(unsigned i) { (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ - // expected-note{{place parentheses around the & expression to evaluate it first}} + // expected-note{{place parentheses around the & expression to evaluate it first}} \ + // expected-note{{place parentheses around the == expression to silence this warning}} (void)(0 == i & 0x2); // expected-warning {{& has lower precedence than ==}} \ - // expected-note{{place parentheses around the & expression to evaluate it first}} + // expected-note{{place parentheses around the & expression to evaluate it first}} \ + // expected-note{{place parentheses around the == expression to silence this warning}} (void)(i & 0xff < 30); // expected-warning {{& has lower precedence than <}} \ - // expected-note{{place parentheses around the & expression to evaluate it first}} + // expected-note{{place parentheses around the & expression to evaluate it first}} \ + // expected-note{{place parentheses around the < expression to silence this warning}} (void)((i & 0x2) == 0); (void)(i & (0x2 == 0)); // Eager logical op diff --git a/test/SemaCXX/warn-assignment-condition.cpp b/test/SemaCXX/warn-assignment-condition.cpp index ce16a68a77..e5a3425804 100644 --- a/test/SemaCXX/warn-assignment-condition.cpp +++ b/test/SemaCXX/warn-assignment-condition.cpp @@ -12,33 +12,42 @@ void test() { // With scalars. if (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} if ((x = 7)) {} do { } while (x = 7); // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} do { } while ((x = 7)); while (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} + while ((x = 7)) {} for (; x = 7; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} for (; (x = 7); ) {} if (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} if ((p = p)) {} do { } while (p = p); // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} do { } while ((p = p)); while (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} while ((p = p)) {} for (; p = p; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} for (; (p = p); ) {} // Initializing variables (shouldn't warn). @@ -49,33 +58,41 @@ void test() { // With temporaries. if (x = (b+b).foo()) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} if ((x = (b+b).foo())) {} do { } while (x = (b+b).foo()); // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} do { } while ((x = (b+b).foo())); while (x = (b+b).foo()) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} while ((x = (b+b).foo())) {} for (; x = (b+b).foo(); ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} for (; (x = (b+b).foo()); ) {} // With a user-defined operator. if (a = b + b) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} if ((a = b + b)) {} do { } while (a = b + b); // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} do { } while ((a = b + b)); while (a = b + b) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // expected-note{{use '==' to turn this assignment into an equality comparison}} \ + // expected-note{{place parentheses around the assignment to silence this warning}} while ((a = b + b)) {} for (; a = b + b; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} \ - // expected-note{{use '==' to turn this assignment into an equality comparison}} + // 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); ) {} }