Downgrade the "direct comparison" error for ObjC literals to a warning.

Chris pointed out that while the comparison is certainly problematic
and does not have well-defined behavior, it isn't any worse than some
of the other abuses that we merely warn about and doesn't need to make
the compilation fail.

Revert the release notes change (r159766) now that this is just a new warning.

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

docs/ReleaseNotes.html

@@ -218,21 +218,7 @@ model can be used.
 <h3 id="objcchanges">Objective-C Language Changes in Clang</h3>
 <!-- = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = -->
 
-<ul>
-  <li>
-    <p>It is now an error to compare against the addresses of Objective-C
-       literals. This is usually a simple mistake (using <code>==</code> instead
-       of <code>-isEqual:</code>), and the result depends on the implementation
-       of the various literals, none of which are guaranteed to be uniqued or
-       always newly-allocated.</p>
-    <p>In the past, we allowed comparisons against literal strings
-       (<code>@"..."</code>), since they are currently uniqued across
-       translation units at link time. This is an implementation detail and
-       should not be relied upon. If you are using such code, please use global
-       string constants instead (<code>NSString * const MyConst = @"..."</code>)
-       or use <code>-isEqual:</code>.</p>
-  </li>
-</ul>
+<p>...</p>
 
 <!-- = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = = -->
 <h3 id="apichanges">Internal API Changes</h3>

include/clang/Basic/DiagnosticSemaKinds.td

@@ -1607,10 +1607,11 @@ def err_invalid_collection_element : Error<
 def err_box_literal_collection : Error<
   "%select{string|character|boolean|numeric}0 literal must be prefixed by '@' "
   "in a collection">;
-def err_objc_literal_comparison : Error<
+def warn_objc_literal_comparison : Warning<
   "direct comparison of %select{a string literal|an array literal|"
-  "a dictionary literal|a numeric literal|a boxed expression|}0 is not "
-  "allowed%select{|; use -isEqual: instead}1">;
+  "a dictionary literal|a numeric literal|a boxed expression|}0 has "
+  "undefined behavior%select{|; use -isEqual: instead}1">,
+  InGroup<DiagGroup<"objc-literal-compare">>;
 def err_attr_tlsmodel_arg : Error<"tls_model must be \"global-dynamic\", "
   "\"local-dynamic\", \"initial-exec\" or \"local-exec\"">;
 

lib/Sema/SemaExpr.cpp

@@ -6739,7 +6739,7 @@ static DiagnosticBuilder diagnoseObjCLiteralComparison(Sema &S,
     llvm_unreachable("Unknown Objective-C object literal kind");
   }
 
-  return S.Diag(Loc, diag::err_objc_literal_comparison)
+  return S.Diag(Loc, diag::warn_objc_literal_comparison)
            << LiteralKind << CanFix << Literal->getSourceRange();
 }
 
@@ -6747,6 +6747,14 @@ static ExprResult fixObjCLiteralComparison(Sema &S, SourceLocation OpLoc,
                                            ExprResult &LHS,
                                            ExprResult &RHS,
                                            BinaryOperatorKind Op) {
+  // Check early to see if the warning's on.
+  // If it's off, we should /not/ be auto-applying the accompanying fixit.
+  DiagnosticsEngine::Level Level =
+    S.getDiagnostics().getDiagnosticLevel(diag::warn_objc_literal_comparison,
+                                          OpLoc);
+  if (Level == DiagnosticsEngine::Ignored)
+    return ExprEmpty();
+
   assert((Op == BO_EQ || Op == BO_NE) && "Cannot fix other operations.");
 
   // Get the LHS object's interface type.
@@ -8228,12 +8236,14 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc,
     break;
   case BO_EQ:
   case BO_NE:
-    if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS)) {
-      ExprResult IsEqualCall = fixObjCLiteralComparison(*this, OpLoc,
-                                                        LHS, RHS, Opc);
-      if (IsEqualCall.isUsable())
-        return IsEqualCall;
-      // Otherwise, fall back to the normal diagnostic in CheckCompareOperands.
+    if (getLangOpts().ObjC1) {
+      if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS)) {
+        ExprResult IsEqualCall = fixObjCLiteralComparison(*this, OpLoc,
+                                                          LHS, RHS, Opc);
+        if (IsEqualCall.isUsable())
+          return IsEqualCall;
+        // Otherwise, fall back to the normal warning in CheckCompareOperands.
+      }
     }
     ResultTy = CheckCompareOperands(LHS, RHS, OpLoc, Opc, false);
     break;

test/FixIt/objc-literals.m

@@ -46,9 +46,9 @@ void fixes() {
 }
 
 void testComparisons(id obj) {
-  if (obj == @"abc") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
-  if (obj != @"def") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
-  if (@"ghi" == obj) return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
+  if (obj == @"abc") return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
+  if (obj != @"def") return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
+  if (@"ghi" == obj) return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
 
   // CHECK: void testComparisons(id obj) {
   // Make sure these three substitutions aren't matching the CHECK lines.
@@ -56,11 +56,11 @@ void testComparisons(id obj) {
   // CHECK-NEXT: if (![obj isEqual: @"def"]) return;
   // CHECK-NEXT: if ([@"ghi" isEqual: obj]) return;
 
-  if (@[] == obj) return; // expected-error{{direct comparison of an array literal is not allowed; use -isEqual: instead}}
-  if (@{} == obj) return; // expected-error{{direct comparison of a dictionary literal is not allowed; use -isEqual: instead}}
-  if (@12 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@1.0 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@__objc_yes == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@(1+1) == obj) return; // expected-error{{direct comparison of a boxed expression is not allowed; use -isEqual: instead}}
+  if (@[] == obj) return; // expected-warning{{direct comparison of an array literal has undefined behavior; use -isEqual: instead}}
+  if (@{} == obj) return; // expected-warning{{direct comparison of a dictionary literal has undefined behavior; use -isEqual: instead}}
+  if (@12 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@1.0 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@__objc_yes == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@(1+1) == obj) return; // expected-warning{{direct comparison of a boxed expression has undefined behavior; use -isEqual: instead}}
 }
 

test/SemaObjC/objc-literal-comparison.m

@@ -28,17 +28,17 @@ typedef unsigned char BOOL;
 @end
 
 void testComparisonsWithFixits(id obj) {
-  if (obj == @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
-  if (obj != @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
-  if (@"" == obj) return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
-  if (@"" == @"") return; // expected-error{{direct comparison of a string literal is not allowed; use -isEqual: instead}}
+  if (obj == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
+  if (obj != @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
+  if (@"" == obj) return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
+  if (@"" == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior; use -isEqual: instead}}
 
-  if (@[] == obj) return; // expected-error{{direct comparison of an array literal is not allowed; use -isEqual: instead}}
-  if (@{} == obj) return; // expected-error{{direct comparison of a dictionary literal is not allowed; use -isEqual: instead}}
-  if (@12 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@1.0 == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@__objc_yes == obj) return; // expected-error{{direct comparison of a numeric literal is not allowed; use -isEqual: instead}}
-  if (@(1+1) == obj) return; // expected-error{{direct comparison of a boxed expression is not allowed; use -isEqual: instead}}
+  if (@[] == obj) return; // expected-warning{{direct comparison of an array literal has undefined behavior; use -isEqual: instead}}
+  if (@{} == obj) return; // expected-warning{{direct comparison of a dictionary literal has undefined behavior; use -isEqual: instead}}
+  if (@12 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@1.0 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@__objc_yes == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior; use -isEqual: instead}}
+  if (@(1+1) == obj) return; // expected-warning{{direct comparison of a boxed expression has undefined behavior; use -isEqual: instead}}
 }
 
 
@@ -55,14 +55,14 @@ void testComparisonsWithoutFixits() {
   // All of these verifications use regex form to ensure we /don't/ append
   // "use -isEqual: instead" to any of these.
 
-  if ([BaseObject new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
+  if ([BaseObject new] == @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
 
-  if ([BadEqualReturnString new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
-  if ([BadEqualArgString new] == @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
+  if ([BadEqualReturnString new] == @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
+  if ([BadEqualArgString new] == @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
 
-  if (@"" < @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
-  if (@"" > @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
-  if (@"" <= @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
-  if (@"" >= @"") return; // expected-error-re{{direct comparison of a string literal is not allowed$}}
+  if (@"" < @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
+  if (@"" > @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
+  if (@"" <= @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
+  if (@"" >= @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
 }