From: Jordan Rose Date: Mon, 9 Jul 2012 16:54:44 +0000 (+0000) Subject: Downgrade the "direct comparison" error for ObjC literals to a warning. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=6deae7cc8de2fb7578ed244d064cd34af744aac5;p=clang 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 6c38c712e7..b820e8dc3f 100644 --- a/docs/ReleaseNotes.html +++ b/docs/ReleaseNotes.html @@ -218,21 +218,7 @@ model can be used.

Objective-C Language Changes in Clang

- +

...

Internal API Changes

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td index 38c346595f..324bbb8b65 100644 --- 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>; 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 6dbf704765..fbd70a8d26 100644 --- 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 356e26348e..b2cc3cd239 100644 --- 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 23cb42bc62..fea624f1a6 100644 --- 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$}} }