[cfe-commits] r124668 - in /cfe/trunk: include/clang/Basic/DiagnosticSemaKinds.td include/clang/Sema/Sema.h lib/Sema/SemaExpr.cpp test/Analysis/self-init.m test/SemaCXX/warn-assignment-condition.cpp

[Argyrios Kyrtzidis]

Author: akirtzidis
Date: Tue Feb  1 12:24:22 2011
New Revision: 124668

URL: http://llvm.org/viewvc/llvm-project?rev=124668&view=rev
Log:
Warn for "if ((a == b))" where the equality expression is needlessly wrapped inside parentheses.
It's highly likely that the user intended an assignment used as condition.

Addresses rdar://8848646.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/include/clang/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/Analysis/self-init.m
    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=124668&r1=124667&r2=124668&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Feb  1 12:24:22 2011
@@ -2853,6 +2853,13 @@
 def note_condition_assign_silence : Note<
   "place parentheses around the assignment to silence this warning">;
 
+def warn_equality_with_extra_parens : Warning<"equality comparison with "
+  "extraneous parentheses">, InGroup<Parentheses>;
+def note_equality_comparison_to_assign : Note<
+  "use '=' to turn this equality comparison into an assignment">;
+def note_equality_comparison_silence : Note<
+  "remove extraneous parentheses around the comparison to silence this warning">;
+
 def warn_synthesized_ivar_access : Warning<
   "direct access of synthesized ivar by using property access %0">,
   InGroup<NonfragileAbi2>, DefaultIgnore;

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=124668&r1=124667&r2=124668&view=diff
==============================================================================
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Tue Feb  1 12:24:22 2011
@@ -4734,6 +4734,10 @@
   /// being used as a boolean condition, warn if it's an assignment.
   void DiagnoseAssignmentAsCondition(Expr *E);
 
+  /// \brief Redundant parentheses over an equality comparison can indicate
+  /// that the user intended an assignment used as condition.
+  void DiagnoseEqualityWithExtraParens(ParenExpr *parenE);
+
   /// CheckCXXBooleanCondition - Returns true if conversion to bool is invalid.
   bool CheckCXXBooleanCondition(Expr *&CondExpr);
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=124668&r1=124667&r2=124668&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Feb  1 12:24:22 2011
@@ -9225,8 +9225,30 @@
     << FixItHint::CreateInsertion(Close, ")");
 }
 
+/// \brief Redundant parentheses over an equality comparison can indicate
+/// that the user intended an assignment used as condition.
+void Sema::DiagnoseEqualityWithExtraParens(ParenExpr *parenE) {
+  Expr *E = parenE->IgnoreParens();
+
+  if (BinaryOperator *opE = dyn_cast<BinaryOperator>(E))
+    if (opE->getOpcode() == BO_EQ) {
+      SourceLocation Loc = opE->getOperatorLoc();
+
+      Diag(Loc, diag::warn_equality_with_extra_parens) << E->getSourceRange();
+
+      Diag(Loc, diag::note_equality_comparison_to_assign)
+        << FixItHint::CreateReplacement(Loc, "=");
+
+      Diag(Loc, diag::note_equality_comparison_silence)
+        << FixItHint::CreateRemoval(parenE->getSourceRange().getBegin())
+        << FixItHint::CreateRemoval(parenE->getSourceRange().getEnd());
+    }
+}
+
 bool Sema::CheckBooleanCondition(Expr *&E, SourceLocation Loc) {
   DiagnoseAssignmentAsCondition(E);
+  if (ParenExpr *parenE = dyn_cast<ParenExpr>(E))
+    DiagnoseEqualityWithExtraParens(parenE);
 
   if (!E->isTypeDependent()) {
     if (E->isBoundMemberFunction(Context))

Modified: cfe/trunk/test/Analysis/self-init.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/self-init.m?rev=124668&r1=124667&r2=124668&view=diff
==============================================================================
--- cfe/trunk/test/Analysis/self-init.m (original)
+++ cfe/trunk/test/Analysis/self-init.m Tue Feb  1 12:24:22 2011
@@ -135,7 +135,7 @@
 }
 
 -(id)init13 {
-	if ((self == [super init])) {
+	if (self == [super init]) {
 	  myivar = 0; // expected-warning {{Instance variable used}}
 	}
 	return self; // expected-warning {{Returning 'self'}}

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=124668&r1=124667&r2=124668&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-assignment-condition.cpp Tue Feb  1 12:24:22 2011
@@ -105,4 +105,8 @@
   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}}
+
+  if ((x == 5)) {} // expected-warning {{equality comparison with extraneous parentheses}} \
+                   // expected-note {{use '=' to turn this equality comparison into an assignment}} \
+                   // expected-note {{remove extraneous parentheses around the comparison to silence this warning}}
 }