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

diff --git a/docs/ReleaseNotes.html b/docs/ReleaseNotes.html
index 6c38c712e70eb9fa326199c62f4a697a8720e5ee..b820e8dc3fdf1c17744dc87d7069b498862e056c 100644 (file)
--- a/docs/ReleaseNotes.html
+++ b/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>

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
index 38c346595f4c56087f7926a82de7ec9153f504ee..324bbb8b6586de1cf16ac87ce893dea482d549fb 100644 (file)
--- a/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/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\"">;
 

diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 6dbf70476582e5a8f04fa741cfcfb484267e25f5..fbd70a8d26c06348c6f1a55be5e5685f12f98a50 100644 (file)
--- a/lib/Sema/SemaExpr.cpp
+++ b/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;

diff --git a/test/FixIt/objc-literals.m b/test/FixIt/objc-literals.m
index 356e26348e5270bc42a8ada521f666a041118ec7..b2cc3cd23962dcc9752a312a4710359bc67ca728 100644 (file)
--- a/test/FixIt/objc-literals.m
+++ b/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}}
 }
 

diff --git a/test/SemaObjC/objc-literal-comparison.m b/test/SemaObjC/objc-literal-comparison.m
index 23cb42bc625c595fddaeb1a790c4e1c82a3b8ba1..fea624f1a6670a2b02b723e18575f1885b3fd6e4 100644 (file)
--- a/test/SemaObjC/objc-literal-comparison.m
+++ b/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-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 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-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$}}
 }