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

5 files changed, 73 insertions, 43 deletions

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<Parentheses>;
 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<DiagGroup<"idiomatic-parentheses">>, 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<BinOp>(lhs)->getRHS()->getLocStart(), rhs->getLocEnd()));
+      SourceRange(cast<BinOp>(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<BinOp>(rhs)->getLHS()->getLocStart()));
+      SourceRange(lhs->getLocEnd(), cast<BinOp>(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); ) {}
 }