From: Argyrios Kyrtzidis <akyrtzi@gmail.com>
Date: Tue, 1 Feb 2011 18:24:22 +0000 (+0000)
Subject: Warn for "if ((a == b))" where the equality expression is needlessly wrapped inside... 
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=0e2dc3a1159806c8303b0979be1ce1526cc64ed3;p=clang

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.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@124668 91177308-0d34-0410-b5e6-96231b3b80d8
---

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 316db75215..1a9d694805 100644
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2853,6 +2853,13 @@ def note_condition_or_assign_to_comparison : Note<
 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;
diff --git a/include/clang/Sema/Sema.h b/include/clang/Sema/Sema.h
index f8cabf9c5f..908d1a1ca6 100644
--- a/include/clang/Sema/Sema.h
+++ b/include/clang/Sema/Sema.h
@@ -4734,6 +4734,10 @@ public:
   /// 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);
 
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 21bf4203e3..82771298e0 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -9225,8 +9225,30 @@ void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
     << 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))
diff --git a/test/Analysis/self-init.m b/test/Analysis/self-init.m
index 1e16e41d7a..1dc5aa92f9 100644
--- a/test/Analysis/self-init.m
+++ b/test/Analysis/self-init.m
@@ -135,7 +135,7 @@ extern void *somePtr;
 }
 
 -(id)init13 {
-	if ((self == [super init])) {
+	if (self == [super init]) {
 	  myivar = 0; // expected-warning {{Instance variable used}}
 	}
 	return self; // expected-warning {{Returning 'self'}}
diff --git a/test/SemaCXX/warn-assignment-condition.cpp b/test/SemaCXX/warn-assignment-condition.cpp
index 9dcffbfe84..48cc137dac 100644
--- a/test/SemaCXX/warn-assignment-condition.cpp
+++ b/test/SemaCXX/warn-assignment-condition.cpp
@@ -105,4 +105,8 @@ void test() {
   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}}
 }