From ec8deba768e7ba93ad9974763dc3902896924a3c Mon Sep 17 00:00:00 2001 From: Fariborz Jahanian Date: Thu, 28 Mar 2013 19:50:55 +0000 Subject: [PATCH] Objective-C: Provide fixit suggestions when class object is accessed via accessing 'isa' ivar to use object_getClass/object_setClass apis. // rdar://13503456 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@178282 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/AST/ExprObjC.h | 16 +++++++++++++-- lib/Sema/SemaExpr.cpp | 30 +++++++++++++++++++++++++---- lib/Sema/SemaExprMember.cpp | 1 + lib/Sema/TreeTransform.h | 6 ++++-- lib/Serialization/ASTReaderStmt.cpp | 1 + lib/Serialization/ASTWriterStmt.cpp | 1 + test/FixIt/auto-isa-fixit.m | 19 ++++++++++++++++++ 7 files changed, 66 insertions(+), 8 deletions(-) create mode 100644 test/FixIt/auto-isa-fixit.m diff --git a/include/clang/AST/ExprObjC.h b/include/clang/AST/ExprObjC.h index 1910af082e..3f17f21cb8 100644 --- a/include/clang/AST/ExprObjC.h +++ b/include/clang/AST/ExprObjC.h @@ -1380,16 +1380,20 @@ class ObjCIsaExpr : public Expr { /// IsaMemberLoc - This is the location of the 'isa'. SourceLocation IsaMemberLoc; + + /// OpLoc - This is the location of '.' or '->' + SourceLocation OpLoc; /// IsArrow - True if this is "X->F", false if this is "X.F". bool IsArrow; public: - ObjCIsaExpr(Expr *base, bool isarrow, SourceLocation l, QualType ty) + ObjCIsaExpr(Expr *base, bool isarrow, SourceLocation l, SourceLocation oploc, + QualType ty) : Expr(ObjCIsaExprClass, ty, VK_LValue, OK_Ordinary, /*TypeDependent=*/false, base->isValueDependent(), base->isInstantiationDependent(), /*ContainsUnexpandedParameterPack=*/false), - Base(base), IsaMemberLoc(l), IsArrow(isarrow) {} + Base(base), IsaMemberLoc(l), OpLoc(oploc), IsArrow(isarrow) {} /// \brief Build an empty expression. explicit ObjCIsaExpr(EmptyShell Empty) : Expr(ObjCIsaExprClass, Empty) { } @@ -1404,10 +1408,18 @@ public: /// location of 'F'. SourceLocation getIsaMemberLoc() const { return IsaMemberLoc; } void setIsaMemberLoc(SourceLocation L) { IsaMemberLoc = L; } + + SourceLocation getOpLoc() const { return OpLoc; } + void setOpLoc(SourceLocation L) { OpLoc = L; } SourceLocation getLocStart() const LLVM_READONLY { return getBase()->getLocStart(); } + + SourceLocation getBaseLocEnd() const LLVM_READONLY { + return getBase()->getLocEnd(); + } + SourceLocation getLocEnd() const LLVM_READONLY { return IsaMemberLoc; } SourceLocation getExprLoc() const LLVM_READONLY { return IsaMemberLoc; } diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp index 54422e81eb..238d833c38 100644 --- a/lib/Sema/SemaExpr.cpp +++ b/lib/Sema/SemaExpr.cpp @@ -491,8 +491,18 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) { } CheckForNullPointerDereference(*this, E); - if (isa(E->IgnoreParens())) - Diag(E->getExprLoc(), diag::warn_objc_isa_use); + if (const ObjCIsaExpr *OISA = dyn_cast(E->IgnoreParenCasts())) { + NamedDecl *ObjectGetClass = LookupSingleName(TUScope, + &Context.Idents.get("object_getClass"), + SourceLocation(), LookupOrdinaryName); + if (ObjectGetClass) + Diag(E->getExprLoc(), diag::warn_objc_isa_use) << + FixItHint::CreateInsertion(OISA->getLocStart(), "object_getClass(") << + FixItHint::CreateReplacement( + SourceRange(OISA->getOpLoc(), OISA->getIsaMemberLoc()), ")"); + else + Diag(E->getExprLoc(), diag::warn_objc_isa_use); + } // C++ [conv.lval]p1: // [...] If T is a non-class type, the type of the prvalue is the @@ -8537,8 +8547,20 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, CheckArrayAccess(LHS.get()); CheckArrayAccess(RHS.get()); - if (isa(LHS.get()->IgnoreParens())) - Diag(LHS.get()->getExprLoc(), diag::warn_objc_isa_assign); + if (const ObjCIsaExpr *OISA = dyn_cast(LHS.get()->IgnoreParenCasts())) { + NamedDecl *ObjectSetClass = LookupSingleName(TUScope, + &Context.Idents.get("object_setClass"), + SourceLocation(), LookupOrdinaryName); + if (ObjectSetClass && isa(LHS.get())) { + SourceLocation RHSLocEnd = PP.getLocForEndOfToken(RHS.get()->getLocEnd()); + Diag(LHS.get()->getExprLoc(), diag::warn_objc_isa_assign) << + FixItHint::CreateInsertion(LHS.get()->getLocStart(), "object_setClass(") << + FixItHint::CreateReplacement(SourceRange(OISA->getOpLoc(), OpLoc), ",") << + FixItHint::CreateInsertion(RHSLocEnd, ")"); + } + else + Diag(LHS.get()->getExprLoc(), diag::warn_objc_isa_assign); + } if (CompResultTy.isNull()) return Owned(new (Context) BinaryOperator(LHS.take(), RHS.take(), Opc, diff --git a/lib/Sema/SemaExprMember.cpp b/lib/Sema/SemaExprMember.cpp index e41a2e9145..481c22135e 100644 --- a/lib/Sema/SemaExprMember.cpp +++ b/lib/Sema/SemaExprMember.cpp @@ -1132,6 +1132,7 @@ Sema::LookupMemberExpr(LookupResult &R, ExprResult &BaseExpr, // apparently. if (OTy->isObjCId() && Member->isStr("isa")) return Owned(new (Context) ObjCIsaExpr(BaseExpr.take(), IsArrow, MemberLoc, + OpLoc, Context.getObjCClassType())); if (ShouldTryAgainWithRedefinitionType(*this, BaseExpr)) return LookupMemberExpr(R, BaseExpr, IsArrow, OpLoc, SS, diff --git a/lib/Sema/TreeTransform.h b/lib/Sema/TreeTransform.h index 1e424b6076..e228d7a0f2 100644 --- a/lib/Sema/TreeTransform.h +++ b/lib/Sema/TreeTransform.h @@ -2389,13 +2389,14 @@ public: /// By default, performs semantic analysis to build the new expression. /// Subclasses may override this routine to provide different behavior. ExprResult RebuildObjCIsaExpr(Expr *BaseArg, SourceLocation IsaLoc, + SourceLocation OpLoc, bool IsArrow) { CXXScopeSpec SS; ExprResult Base = getSema().Owned(BaseArg); LookupResult R(getSema(), &getSema().Context.Idents.get("isa"), IsaLoc, Sema::LookupMemberName); ExprResult Result = getSema().LookupMemberExpr(R, Base, IsArrow, - /*FIME:*/IsaLoc, + OpLoc, SS, 0, false); if (Result.isInvalid() || Base.isInvalid()) return ExprError(); @@ -2404,7 +2405,7 @@ public: return Result; return getSema().BuildMemberReferenceExpr(Base.get(), Base.get()->getType(), - /*FIXME:*/IsaLoc, IsArrow, + OpLoc, IsArrow, SS, SourceLocation(), /*FirstQualifierInScope=*/0, R, @@ -8788,6 +8789,7 @@ TreeTransform::TransformObjCIsaExpr(ObjCIsaExpr *E) { return SemaRef.Owned(E); return getDerived().RebuildObjCIsaExpr(Base.get(), E->getIsaMemberLoc(), + E->getOpLoc(), E->isArrow()); } diff --git a/lib/Serialization/ASTReaderStmt.cpp b/lib/Serialization/ASTReaderStmt.cpp index 9c99d6e46f..6159916c82 100644 --- a/lib/Serialization/ASTReaderStmt.cpp +++ b/lib/Serialization/ASTReaderStmt.cpp @@ -528,6 +528,7 @@ void ASTStmtReader::VisitObjCIsaExpr(ObjCIsaExpr *E) { VisitExpr(E); E->setBase(Reader.ReadSubExpr()); E->setIsaMemberLoc(ReadSourceLocation(Record, Idx)); + E->setOpLoc(ReadSourceLocation(Record, Idx)); E->setArrow(Record[Idx++]); } diff --git a/lib/Serialization/ASTWriterStmt.cpp b/lib/Serialization/ASTWriterStmt.cpp index ee9735c163..bb8afb8652 100644 --- a/lib/Serialization/ASTWriterStmt.cpp +++ b/lib/Serialization/ASTWriterStmt.cpp @@ -498,6 +498,7 @@ void ASTStmtWriter::VisitObjCIsaExpr(ObjCIsaExpr *E) { VisitExpr(E); Writer.AddStmt(E->getBase()); Writer.AddSourceLocation(E->getIsaMemberLoc(), Record); + Writer.AddSourceLocation(E->getOpLoc(), Record); Record.push_back(E->isArrow()); Code = serialization::EXPR_OBJC_ISA; } diff --git a/test/FixIt/auto-isa-fixit.m b/test/FixIt/auto-isa-fixit.m new file mode 100644 index 0000000000..ec0d96851c --- /dev/null +++ b/test/FixIt/auto-isa-fixit.m @@ -0,0 +1,19 @@ +// RUN: cp %s %t +// RUN: %clang_cc1 -x objective-c -fixit %t +// RUN: %clang_cc1 -x objective-c -Werror %t +// rdar://13503456 + +void object_setClass(id, id); +Class object_getClass(id); + +id rhs(); + +Class pr6302(id x123) { + x123->isa = 0; + x123->isa = rhs(); + x123->isa = (id)(x123->isa); + x123->isa = (id)x123->isa; + x123->isa = (x123->isa); + x123->isa = (id)(x123->isa); + return x123->isa; +} -- 2.40.0