[cfe-commits] r92975 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/Sema/parentheses.c test/SemaCXX/warn-assignment-condition.cpp

Douglas Gregor dgregor at apple.com
Thu Jan 7 16:20:24 PST 2010


Author: dgregor
Date: Thu Jan  7 18:20:23 2010
New Revision: 92975

URL: http://llvm.org/viewvc/llvm-project?rev=92975&view=rev
Log:
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()) {
        ^
        ==



Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Sema/parentheses.c
    cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=92975&r1=92974&r2=92975&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Thu Jan  7 18:20:23 2010
@@ -1563,7 +1563,9 @@
 def warn_precedence_bitwise_rel : Warning<
   "%0 has lower precedence than %1; %1 will be evaluated first">,
   InGroup<Parentheses>;
-
+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_idiomatic_assignment : Warning<"using the result "
   "of an assignment as a condition without parentheses">,
   InGroup<DiagGroup<"idiomatic-parentheses">>, 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">;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=92975&r1=92974&r2=92975&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Jan  7 18:20:23 2010
@@ -6150,8 +6150,9 @@
 /// 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 @@
   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 @@
       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<BinOp>(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<BinOp>(rhs)->getLHS()->getLocStart()));
 }
 
@@ -7251,6 +7273,8 @@
     << 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) {

Modified: cfe/trunk/test/Sema/parentheses.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/parentheses.c?rev=92975&r1=92974&r2=92975&view=diff

==============================================================================
--- cfe/trunk/test/Sema/parentheses.c (original)
+++ cfe/trunk/test/Sema/parentheses.c Thu Jan  7 18:20:23 2010
@@ -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

Modified: cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp?rev=92975&r1=92974&r2=92975&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp Thu Jan  7 18:20:23 2010
@@ -11,26 +11,34 @@
   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 @@
   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); ) {}
 }





More information about the cfe-commits mailing list