[cfe-commits] r159939 - in /cfe/trunk: docs/ReleaseNotes.html include/clang/Basic/DiagnosticSemaKinds.td lib/Sema/SemaExpr.cpp test/FixIt/objc-literals.m test/SemaObjC/objc-literal-comparison.m

Jordan Rose jordan_rose at apple.com
Mon Jul 9 09:54:45 PDT 2012


Author: jrose
Date: Mon Jul  9 11:54:44 2012
New Revision: 159939

URL: http://llvm.org/viewvc/llvm-project?rev=159939&view=rev
Log:
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.

Modified:
    cfe/trunk/docs/ReleaseNotes.html
    cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
    cfe/trunk/lib/Sema/SemaExpr.cpp
    cfe/trunk/test/FixIt/objc-literals.m
    cfe/trunk/test/SemaObjC/objc-literal-comparison.m

Modified: cfe/trunk/docs/ReleaseNotes.html
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ReleaseNotes.html?rev=159939&r1=159938&r2=159939&view=diff
==============================================================================
--- cfe/trunk/docs/ReleaseNotes.html (original)
+++ cfe/trunk/docs/ReleaseNotes.html Mon Jul  9 11:54:44 2012
@@ -218,21 +218,7 @@
 <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>

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=159939&r1=159938&r2=159939&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Jul  9 11:54:44 2012
@@ -1607,10 +1607,11 @@
 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\"">;
 

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=159939&r1=159938&r2=159939&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Mon Jul  9 11:54:44 2012
@@ -6739,7 +6739,7 @@
     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 @@
                                            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 @@
     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;

Modified: cfe/trunk/test/FixIt/objc-literals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/objc-literals.m?rev=159939&r1=159938&r2=159939&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/objc-literals.m (original)
+++ cfe/trunk/test/FixIt/objc-literals.m Mon Jul  9 11:54:44 2012
@@ -46,9 +46,9 @@
 }
 
 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 @@
   // 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}}
 }
 

Modified: cfe/trunk/test/SemaObjC/objc-literal-comparison.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/objc-literal-comparison.m?rev=159939&r1=159938&r2=159939&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/objc-literal-comparison.m (original)
+++ cfe/trunk/test/SemaObjC/objc-literal-comparison.m Mon Jul  9 11:54:44 2012
@@ -28,17 +28,17 @@
 @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 @@
   // 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$}}
 }
 





More information about the cfe-commits mailing list