From 8b6ec6870f65f4a4d4c88b2a9715706ba34ee6d9 Mon Sep 17 00:00:00 2001
From: Argyrios Kyrtzidis <akyrtzi@gmail.com>
Date: Tue, 1 Feb 2011 18:24:22 +0000
Subject: [PATCH] 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.

llvm-svn: 124668
---
 .../clang/Basic/DiagnosticSemaKinds.td        |  7 ++++++
 clang/include/clang/Sema/Sema.h               |  4 ++++
 clang/lib/Sema/SemaExpr.cpp                   | 22 +++++++++++++++++++
 clang/test/Analysis/self-init.m               |  2 +-
 .../SemaCXX/warn-assignment-condition.cpp     |  4 ++++
 5 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 316db752153d..1a9d694805b3 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/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/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index f8cabf9c5f80..908d1a1ca687 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/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/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 21bf4203e3e6..82771298e0ae 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/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/clang/test/Analysis/self-init.m b/clang/test/Analysis/self-init.m
index 1e16e41d7a9a..1dc5aa92f9c3 100644
--- a/clang/test/Analysis/self-init.m
+++ b/clang/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/clang/test/SemaCXX/warn-assignment-condition.cpp b/clang/test/SemaCXX/warn-assignment-condition.cpp
index 9dcffbfe84e8..48cc137dac73 100644
--- a/clang/test/SemaCXX/warn-assignment-condition.cpp
+++ b/clang/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}}
 }
-- 
GitLab