Apparently the following idiom is specifically encouraged:

  if (self = [super init])
Recognize it and only warn if -Wparentheses is explicitly enabled.

llvm-svn: 86790

4 files changed, 30 insertions, 2 deletions

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index b8cca04f81c..93c655b5050 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -40,6 +40,7 @@ def FormatExtraArgs : DiagGroup<"format-extra-args">;
 def FormatZeroLength : DiagGroup<"format-zero-length">;
 
 def FourByteMultiChar : DiagGroup<"four-char-constants">;
+def : DiagGroup<"idiomatic-parentheses">;
 def : DiagGroup<"import">;
 def : DiagGroup<"init-self">;
 def : DiagGroup<"inline">;
@@ -63,7 +64,6 @@ def : DiagGroup<"old-style-definition">;
 def : DiagGroup<"overflow">;
 def : DiagGroup<"overloaded-virtual">;
 def : DiagGroup<"packed">;
-def Parentheses : DiagGroup<"parentheses">;
 def PointerArith : DiagGroup<"pointer-arith">;
 def : DiagGroup<"pointer-to-int-cast">;
 def : DiagGroup<"redundant-decls">;
@@ -117,6 +117,10 @@ def CharSubscript : DiagGroup<"char-subscripts">;
 
 // Aggregation warning settings.
 
+// -Widiomatic-parentheses contains warnings about 'idiomatic'
+// missing parentheses;  it is off by default.
+def Parentheses : DiagGroup<"parentheses", [DiagGroup<"idiomatic-parentheses">]>;
+
 // -Wconversion has its own warnings, but we split this one out for
 // legacy reasons.
 def Conversion : DiagGroup<"conversion",
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 285ae8f70db..e4ebb5ecc7c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -1806,6 +1806,9 @@ def err_incomplete_object_call : Error<
 def warn_condition_is_assignment : Warning<"using the result of an "
   "assignment as a condition without parentheses">,
   InGroup<Parentheses>;
+def warn_condition_is_self_assignment : Warning<"using the result of an "
+  "assignment to 'self' as a condition without parentheses">,
+  InGroup<DiagGroup<"idiomatic-parentheses">>, DefaultIgnore;
 
 def warn_value_always_zero : Warning<
   "%0 is always %select{zero|false|NULL}1 in this context">;
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index c29cfb7c5d2..6bdcb0ed902 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -6510,11 +6510,22 @@ bool Sema::CheckCallReturnType(QualType ReturnType, SourceLocation Loc,
 void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
   SourceLocation Loc;
 
+  unsigned diagnostic = diag::warn_condition_is_assignment;
+
   if (isa<BinaryOperator>(E)) {
     BinaryOperator *Op = cast<BinaryOperator>(E);
     if (Op->getOpcode() != BinaryOperator::Assign)
       return;
 
+    // Greylist the following Cocoa ObjC idiom by putting it into a
+    // warning subcategory which defaults off:
+    //   if (self = [super init])
+    // The selector can vary, and it's possible that the base might,
+    // too, so we just recognize any message call.
+    if (isSelfExpr(Op->getLHS()) &&
+        isa<ObjCMessageExpr>(Op->getRHS()->IgnoreParenCasts()))
+      diagnostic = diag::warn_condition_is_self_assignment;
+
     Loc = Op->getOperatorLoc();
   } else if (isa<CXXOperatorCallExpr>(E)) {
     CXXOperatorCallExpr *Op = cast<CXXOperatorCallExpr>(E);
@@ -6530,7 +6541,7 @@ void Sema::DiagnoseAssignmentAsCondition(Expr *E) {
   SourceLocation Open = E->getSourceRange().getBegin();
   SourceLocation Close = PP.getLocForEndOfToken(E->getSourceRange().getEnd());
   
-  Diag(Loc, diag::warn_condition_is_assignment)
+  Diag(Loc, diagnostic)
     << E->getSourceRange()
     << CodeModificationHint::CreateInsertion(Open, "(")
     << CodeModificationHint::CreateInsertion(Close, ")");
diff --git a/clang/test/SemaObjC/call-super-2.m b/clang/test/SemaObjC/call-super-2.m
index a481cffd288..afd35a8e263 100644
--- a/clang/test/SemaObjC/call-super-2.m
+++ b/clang/test/SemaObjC/call-super-2.m
@@ -6,6 +6,7 @@ typedef struct objc_object *id;
 id objc_getClass(const char *s);
 
 @interface Object 
+- (id) initWithInt: (int) i;
 @end
 
 @protocol Func
@@ -28,6 +29,7 @@ id objc_getClass(const char *s);
 - (int) instance_func5;
 - (int) instance_func6;
 - (int) instance_func7;
+- (id) initWithInt: (int) i;
 @end
 
 @implementation Derived
@@ -94,5 +96,13 @@ id objc_getClass(const char *s);
 {
    return [objc_getClass("Derived") class_func1];
 }
+- (id) initWithInt: (int) i
+{
+   // Don't warn about parentheses here.
+   if (self = [super initWithInt: i]) {
+     [self instance_func1];
+   }
+   return self;
+}
 @end