[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
Jean-Daniel Dupas
devlists at shadowlab.org
Mon Jul 9 10:06:54 PDT 2012
While this is just a warning now, I found the comment in the release note very helpful to understand why we should never do it.
Le 9 juil. 2012 à 18:54, Jordan Rose a écrit :
> 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$}}
> }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
-- Jean-Daniel
More information about the cfe-commits
mailing list