[cfe-commits] r160377 - in /cfe/trunk: 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
Tue Jul 17 10:46:40 PDT 2012
Author: jrose
Date: Tue Jul 17 12:46:40 2012
New Revision: 160377
URL: http://llvm.org/viewvc/llvm-project?rev=160377&view=rev
Log:
Now that -Wobjc-literal-compare is a warning, put the fixit on a note.
Recovering as if the user had actually called -isEqual: is a bit too far from
the semantics of the program as written, /even though/ it's probably what they
intended.
Modified:
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/include/clang/Basic/DiagnosticSemaKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=160377&r1=160376&r2=160377&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Jul 17 12:46:40 2012
@@ -1613,8 +1613,9 @@
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 has "
- "undefined behavior%select{|; use -isEqual: instead}1">,
- InGroup<DiagGroup<"objc-literal-compare">>;
+ "undefined behavior">, InGroup<DiagGroup<"objc-literal-compare">>;
+def note_objc_literal_comparison_isequal : Note<
+ "use 'isEqual:' instead">;
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=160377&r1=160376&r2=160377&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Jul 17 12:46:40 2012
@@ -6688,11 +6688,60 @@
}
}
-static DiagnosticBuilder diagnoseObjCLiteralComparison(Sema &S,
- SourceLocation Loc,
- ExprResult &LHS,
- ExprResult &RHS,
- bool CanFix = false) {
+static bool hasIsEqualMethod(Sema &S, const Expr *LHS, const Expr *RHS) {
+ // Get the LHS object's interface type.
+ QualType Type = LHS->getType();
+ QualType InterfaceType;
+ if (const ObjCObjectPointerType *PTy = Type->getAs<ObjCObjectPointerType>()) {
+ InterfaceType = PTy->getPointeeType();
+ if (const ObjCObjectType *iQFaceTy =
+ InterfaceType->getAsObjCQualifiedInterfaceType())
+ InterfaceType = iQFaceTy->getBaseType();
+ } else {
+ // If this is not actually an Objective-C object, bail out.
+ return false;
+ }
+
+ // If the RHS isn't an Objective-C object, bail out.
+ if (!RHS->getType()->isObjCObjectPointerType())
+ return false;
+
+ // Try to find the -isEqual: method.
+ Selector IsEqualSel = S.NSAPIObj->getIsEqualSelector();
+ ObjCMethodDecl *Method = S.LookupMethodInObjectType(IsEqualSel,
+ InterfaceType,
+ /*instance=*/true);
+ if (!Method) {
+ if (Type->isObjCIdType()) {
+ // For 'id', just check the global pool.
+ Method = S.LookupInstanceMethodInGlobalPool(IsEqualSel, SourceRange(),
+ /*receiverId=*/true,
+ /*warn=*/false);
+ } else {
+ // Check protocols.
+ Method = S.LookupMethodInQualifiedType(IsEqualSel,
+ cast<ObjCObjectPointerType>(Type),
+ /*instance=*/true);
+ }
+ }
+
+ if (!Method)
+ return false;
+
+ QualType T = Method->param_begin()[0]->getType();
+ if (!T->isObjCObjectPointerType())
+ return false;
+
+ QualType R = Method->getResultType();
+ if (!R->isScalarType())
+ return false;
+
+ return true;
+}
+
+static void diagnoseObjCLiteralComparison(Sema &S, SourceLocation Loc,
+ ExprResult &LHS, ExprResult &RHS,
+ BinaryOperator::Opcode Opc){
Expr *Literal = (isObjCObjectLiteral(LHS) ? LHS : RHS).get();
unsigned LiteralKind;
@@ -6740,94 +6789,20 @@
llvm_unreachable("Unknown Objective-C object literal kind");
}
- return S.Diag(Loc, diag::warn_objc_literal_comparison)
- << LiteralKind << CanFix << Literal->getSourceRange();
-}
-
-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.
- QualType Type = LHS.get()->getType();
- QualType InterfaceType;
- if (const ObjCObjectPointerType *PTy = Type->getAs<ObjCObjectPointerType>()) {
- InterfaceType = PTy->getPointeeType();
- if (const ObjCObjectType *iQFaceTy =
- InterfaceType->getAsObjCQualifiedInterfaceType())
- InterfaceType = iQFaceTy->getBaseType();
- } else {
- // If this is not actually an Objective-C object, bail out.
- return ExprEmpty();
- }
-
- // If the RHS isn't an Objective-C object, bail out.
- if (!RHS.get()->getType()->isObjCObjectPointerType())
- return ExprEmpty();
+ S.Diag(Loc, diag::warn_objc_literal_comparison)
+ << LiteralKind << Literal->getSourceRange();
- // Try to find the -isEqual: method.
- Selector IsEqualSel = S.NSAPIObj->getIsEqualSelector();
- ObjCMethodDecl *Method = S.LookupMethodInObjectType(IsEqualSel,
- InterfaceType,
- /*instance=*/true);
- bool ReceiverIsId = (Type->isObjCIdType() || Type->isObjCQualifiedIdType());
-
- if (!Method && ReceiverIsId) {
- Method = S.LookupInstanceMethodInGlobalPool(IsEqualSel, SourceRange(),
- /*receiverId=*/true,
- /*warn=*/false);
+ if (BinaryOperator::isEqualityOp(Opc) &&
+ hasIsEqualMethod(S, LHS.get(), RHS.get())) {
+ SourceLocation Start = LHS.get()->getLocStart();
+ SourceLocation End = S.PP.getLocForEndOfToken(RHS.get()->getLocEnd());
+ SourceRange OpRange(Loc, S.PP.getLocForEndOfToken(Loc));
+
+ S.Diag(Loc, diag::note_objc_literal_comparison_isequal)
+ << FixItHint::CreateInsertion(Start, Opc == BO_EQ ? "[" : "![")
+ << FixItHint::CreateReplacement(OpRange, "isEqual:")
+ << FixItHint::CreateInsertion(End, "]");
}
-
- if (!Method)
- return ExprEmpty();
-
- QualType T = Method->param_begin()[0]->getType();
- if (!T->isObjCObjectPointerType())
- return ExprEmpty();
-
- QualType R = Method->getResultType();
- if (!R->isScalarType())
- return ExprEmpty();
-
- // At this point we know we have a good -isEqual: method.
- // Emit the diagnostic and fixit.
- DiagnosticBuilder Diag = diagnoseObjCLiteralComparison(S, OpLoc,
- LHS, RHS, true);
-
- Expr *LHSExpr = LHS.take();
- Expr *RHSExpr = RHS.take();
-
- SourceLocation Start = LHSExpr->getLocStart();
- SourceLocation End = S.PP.getLocForEndOfToken(RHSExpr->getLocEnd());
- SourceRange OpRange(OpLoc, S.PP.getLocForEndOfToken(OpLoc));
-
- Diag << FixItHint::CreateInsertion(Start, Op == BO_EQ ? "[" : "![")
- << FixItHint::CreateReplacement(OpRange, "isEqual:")
- << FixItHint::CreateInsertion(End, "]");
-
- // Finally, build the call to -isEqual: (and possible logical not).
- ExprResult Call = S.BuildInstanceMessage(LHSExpr, LHSExpr->getType(),
- /*SuperLoc=*/SourceLocation(),
- IsEqualSel, Method,
- OpLoc, OpLoc, OpLoc,
- MultiExprArg(S, &RHSExpr, 1),
- /*isImplicit=*/false);
-
- ExprResult CallCond = S.CheckBooleanCondition(Call.get(), OpLoc);
-
- if (Op == BO_NE)
- return S.CreateBuiltinUnaryOp(OpLoc, UO_LNot, CallCond.get());
- return CallCond;
}
// C99 6.5.8, C++ [expr.rel]
@@ -7155,7 +7130,7 @@
diagnoseDistinctPointerComparison(*this, Loc, LHS, RHS,
/*isError*/false);
if (isObjCObjectLiteral(LHS) || isObjCObjectLiteral(RHS))
- diagnoseObjCLiteralComparison(*this, Loc, LHS, RHS);
+ diagnoseObjCLiteralComparison(*this, Loc, LHS, RHS, Opc);
if (LHSIsNull && !RHSIsNull)
LHS = ImpCastExprToType(LHS.take(), RHSType, CK_BitCast);
@@ -8237,15 +8212,6 @@
break;
case BO_EQ:
case BO_NE:
- 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;
case BO_And:
Modified: cfe/trunk/test/FixIt/objc-literals.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/FixIt/objc-literals.m?rev=160377&r1=160376&r2=160377&view=diff
==============================================================================
--- cfe/trunk/test/FixIt/objc-literals.m (original)
+++ cfe/trunk/test/FixIt/objc-literals.m Tue Jul 17 12:46:40 2012
@@ -2,12 +2,10 @@
// RUN: cp %s %t
// RUN: not %clang_cc1 -fsyntax-only -fixit -x objective-c %t
// RUN: %clang_cc1 -fsyntax-only -pedantic -Werror -x objective-c %t
-// RUN: FileCheck -input-file=%t %s
typedef unsigned char BOOL;
@interface NSObject
-- (BOOL)isEqual:(id)other;
@end
@interface NSNumber : NSObject
@@ -44,23 +42,3 @@
"blah" // expected-error{{string literal must be prefixed by '@'}}
];
}
-
-void testComparisons(id obj) {
- 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.
- // CHECK-NEXT: if ([obj isEqual: @"abc"]) return;
- // CHECK-NEXT: if (![obj isEqual: @"def"]) return;
- // CHECK-NEXT: if ([@"ghi" isEqual: obj]) return;
-
- 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=160377&r1=160376&r2=160377&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/objc-literal-comparison.m (original)
+++ cfe/trunk/test/SemaObjC/objc-literal-comparison.m Tue Jul 17 12:46:40 2012
@@ -28,17 +28,17 @@
@end
void testComparisonsWithFixits(id obj) {
- 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}}
+ if (obj == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (obj != @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@"" == obj) return; // expected-warning{{direct comparison of a string literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@"" == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+
+ if (@[] == obj) return; // expected-warning{{direct comparison of an array literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@{} == obj) return; // expected-warning{{direct comparison of a dictionary literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@12 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@1.0 == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@__objc_yes == obj) return; // expected-warning{{direct comparison of a numeric literal has undefined behavior}} expected-note{{use 'isEqual:' instead}}
+ if (@(1+1) == obj) return; // expected-warning{{direct comparison of a boxed expression has undefined behavior}} expected-note{{use 'isEqual:' instead}}
}
@@ -52,17 +52,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-warning{{direct comparison of a string literal has undefined behavior}}
- if ([BaseObject new] == @"") return; // expected-warning-re{{direct comparison of a string literal has undefined behavior$}}
+ if ([BadEqualReturnString new] == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
+ if ([BadEqualArgString new] == @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
- 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-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$}}
+ if (@"" < @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
+ if (@"" > @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
+ if (@"" <= @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
+ if (@"" >= @"") return; // expected-warning{{direct comparison of a string literal has undefined behavior}}
}
More information about the cfe-commits
mailing list