From 827feec561c8a1f23c099da56c4ac98364ecfc09 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Fri, 8 Jan 2010 00:20:23 +0000 Subject: [PATCH] Improve the fix-its for -Wparentheses to ensure that the fix-it suggestions follow recovery. Additionally, add a note to these diagnostics which suggests a fix-it for changing the behavior to what the user probably meant. Examples: t.cpp:2:9: warning: & has lower precedence than ==; == will be evaluated first [-Wparentheses] if (i & j == k) { ^~~~~~~~ ( ) t.cpp:2:9: note: place parentheses around the & expression to evaluate it first if (i & j == k) { ^ ( ) t.cpp:14:9: warning: using the result of an assignment as a condition without parentheses [-Wparentheses] if (i = f()) { ~~^~~~~ ( ) t.cpp:14:9: note: use '==' to turn this assignment into an equality comparison if (i = f()) { ^ == git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@92975 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/DiagnosticSemaKinds.td | 6 ++- lib/Sema/SemaExpr.cpp | 28 ++++++++++++- test/Sema/parentheses.c | 12 ++++-- test/SemaCXX/warn-assignment-condition.cpp | 48 ++++++++++++++-------- 4 files changed, 71 insertions(+), 23 deletions(-) diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 8470ad9850..ac6d3ec866 100644 --- a/include/clang/Basic/DiagnosticSemaKinds.td +++ b/include/clang/Basic/DiagnosticSemaKinds.td @@ -1563,7 +1563,9 @@ def warn_shift_gt_typewidth : Warning< def warn_precedence_bitwise_rel : Warning< "%0 has lower precedence than %1; %1 will be evaluated first">, InGroup; - +def note_precedence_bitwise_first : Note< + "place parentheses around the %0 expression to evaluate it first">; + def err_sizeof_nonfragile_interface : Error< "invalid application of '%select{alignof|sizeof}1' to interface %0 in " "non-fragile ABI">; @@ -1968,6 +1970,8 @@ def warn_condition_is_assignment : Warning<"using the result of an " def warn_condition_is_idiomatic_assignment : Warning<"using the result " "of an assignment as a condition without parentheses">, InGroup>, DefaultIgnore; +def note_condition_assign_to_comparison : Note< + "use '==' to turn this assignment into an equality comparison">; 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 216e2e6098..f131109833 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -6150,8 +6150,9 @@ Action::OwningExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, /// ParenRange in parentheses. static void SuggestParentheses(Sema &Self, SourceLocation Loc, const PartialDiagnostic &PD, - SourceRange ParenRange) -{ + SourceRange ParenRange, + const PartialDiagnostic &SecondPD = PartialDiagnostic(0), + SourceRange SecondParenRange = SourceRange()) { SourceLocation EndLoc = Self.PP.getLocForEndOfToken(ParenRange.getEnd()); if (!ParenRange.getEnd().isFileID() || EndLoc.isInvalid()) { // We can't display the parentheses, so just dig the @@ -6163,6 +6164,21 @@ static void SuggestParentheses(Sema &Self, SourceLocation Loc, Self.Diag(Loc, PD) << CodeModificationHint::CreateInsertion(ParenRange.getBegin(), "(") << CodeModificationHint::CreateInsertion(EndLoc, ")"); + + if (!SecondPD.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); + return; + } + + Self.Diag(Loc, SecondPD) + << CodeModificationHint::CreateInsertion(SecondParenRange.getBegin(), "(") + << CodeModificationHint::CreateInsertion(EndLoc, ")"); } /// DiagnoseBitwisePrecedence - Emit a warning when bitwise and comparison @@ -6194,12 +6210,18 @@ static void DiagnoseBitwisePrecedence(Sema &Self, BinaryOperator::Opcode Opc, PDiag(diag::warn_precedence_bitwise_rel) << SourceRange(lhs->getLocStart(), OpLoc) << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(lhsopc), + lhs->getSourceRange(), + PDiag(diag::note_precedence_bitwise_first) + << BinOp::getOpcodeStr(Opc), SourceRange(cast(lhs)->getRHS()->getLocStart(), rhs->getLocEnd())); else if (BinOp::isComparisonOp(rhsopc)) SuggestParentheses(Self, OpLoc, PDiag(diag::warn_precedence_bitwise_rel) << SourceRange(OpLoc, rhs->getLocEnd()) << BinOp::getOpcodeStr(Opc) << BinOp::getOpcodeStr(rhsopc), + rhs->getSourceRange(), + PDiag(diag::note_precedence_bitwise_first) + << BinOp::getOpcodeStr(Opc), SourceRange(lhs->getLocEnd(), cast(rhs)->getLHS()->getLocStart())); } @@ -7251,6 +7273,8 @@ void Sema::DiagnoseAssignmentAsCondition(Expr *E) { << E->getSourceRange() << CodeModificationHint::CreateInsertion(Open, "(") << CodeModificationHint::CreateInsertion(Close, ")"); + Diag(Loc, diag::note_condition_assign_to_comparison) + << CodeModificationHint::CreateReplacement(Loc, "=="); } bool Sema::CheckBooleanCondition(Expr *&E, SourceLocation Loc) { diff --git a/test/Sema/parentheses.c b/test/Sema/parentheses.c index f7a7fbd37d..69c91cb15f 100644 --- a/test/Sema/parentheses.c +++ b/test/Sema/parentheses.c @@ -4,14 +4,18 @@ // Test the various warnings under -Wparentheses void if_assign(void) { int i; - if (i = 4) {} // expected-warning {{assignment as a condition}} + if (i = 4) {} // expected-warning {{assignment as a condition}} \ + // expected-note{{use '==' to turn this assignment into an equality comparison}} if ((i = 4)) {} } void bitwise_rel(unsigned i) { - (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} - (void)(0 == i & 0x2); // expected-warning {{& has lower precedence than ==}} - (void)(i & 0xff < 30); // expected-warning {{& has lower precedence than <}} + (void)(i & 0x2 == 0); // expected-warning {{& has lower precedence than ==}} \ + // expected-note{{place parentheses around the & expression to evaluate it first}} + (void)(0 == i & 0x2); // expected-warning {{& has lower precedence than ==}} \ + // expected-note{{place parentheses around the & expression to evaluate it first}} + (void)(i & 0xff < 30); // expected-warning {{& has lower precedence than <}} \ + // expected-note{{place parentheses around the & expression to evaluate it first}} (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 1df906dd7e..ce16a68a77 100644 --- a/test/SemaCXX/warn-assignment-condition.cpp +++ b/test/SemaCXX/warn-assignment-condition.cpp @@ -11,26 +11,34 @@ void test() { A a, b; // With scalars. - if (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} if ((x = 7)) {} do { - } while (x = 7); // expected-warning {{using the result of an assignment as a condition without parentheses}} + } 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}} do { } while ((x = 7)); - while (x = 7) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} while ((x = 7)) {} - for (; x = 7; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} for (; (x = 7); ) {} - if (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} if ((p = p)) {} do { - } while (p = p); // expected-warning {{using the result of an assignment as a condition without parentheses}} + } 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}} do { } while ((p = p)); - while (p = p) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} while ((p = p)) {} - for (; p = p; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} for (; (p = p); ) {} // Initializing variables (shouldn't warn). @@ -40,26 +48,34 @@ void test() { while (A y = a) {} // With temporaries. - if (x = (b+b).foo()) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} if ((x = (b+b).foo())) {} do { - } while (x = (b+b).foo()); // expected-warning {{using the result of an assignment as a condition without parentheses}} + } 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}} do { } while ((x = (b+b).foo())); - while (x = (b+b).foo()) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} while ((x = (b+b).foo())) {} - for (; x = (b+b).foo(); ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} 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}} + 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}} if ((a = b + b)) {} do { - } while (a = b + b); // expected-warning {{using the result of an assignment as a condition without parentheses}} + } 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}} do { } while ((a = b + b)); - while (a = b + b) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} while ((a = b + b)) {} - for (; a = b + b; ) {} // expected-warning {{using the result of an assignment as a condition without parentheses}} + 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}} for (; (a = b + b); ) {} }