From f12ff4df48e74ac553bf138633afdc95444e4c0b Mon Sep 17 00:00:00 2001 From: Fariborz Jahanian Date: Tue, 2 Apr 2013 18:57:54 +0000 Subject: [PATCH] Objective-C: Provide fixit hints when warning about 'isa' ivar being explicitely accessed when base is a user class object reference. // rdar://13503456 llvm-svn: 178562 --- clang/include/clang/AST/ExprObjC.h | 12 +++++- clang/lib/CodeGen/CGObjC.cpp | 3 +- clang/lib/Sema/SemaExpr.cpp | 40 ++++++++++++++++--- clang/lib/Sema/SemaExprMember.cpp | 2 +- clang/lib/Sema/SemaObjCProperty.cpp | 2 + clang/lib/Serialization/ASTReaderStmt.cpp | 1 + clang/lib/Serialization/ASTWriterStmt.cpp | 1 + clang/test/FixIt/auto-isa-fixit.m | 47 +++++++++++++++++++++++ 8 files changed, 98 insertions(+), 10 deletions(-) diff --git a/clang/include/clang/AST/ExprObjC.h b/clang/include/clang/AST/ExprObjC.h index 3f17f21cb88b..5211171541b5 100644 --- a/clang/include/clang/AST/ExprObjC.h +++ b/clang/include/clang/AST/ExprObjC.h @@ -461,18 +461,23 @@ class ObjCIvarRefExpr : public Expr { ObjCIvarDecl *D; Stmt *Base; SourceLocation Loc; + /// OpLoc - This is the location of '.' or '->' + SourceLocation OpLoc; + bool IsArrow:1; // True if this is "X->F", false if this is "X.F". bool IsFreeIvar:1; // True if ivar reference has no base (self assumed). public: ObjCIvarRefExpr(ObjCIvarDecl *d, QualType t, - SourceLocation l, Expr *base, + SourceLocation l, SourceLocation oploc, + Expr *base, bool arrow = false, bool freeIvar = false) : Expr(ObjCIvarRefExprClass, t, VK_LValue, OK_Ordinary, /*TypeDependent=*/false, base->isValueDependent(), base->isInstantiationDependent(), base->containsUnexpandedParameterPack()), - D(d), Base(base), Loc(l), IsArrow(arrow), IsFreeIvar(freeIvar) {} + D(d), Base(base), Loc(l), OpLoc(oploc), + IsArrow(arrow), IsFreeIvar(freeIvar) {} explicit ObjCIvarRefExpr(EmptyShell Empty) : Expr(ObjCIvarRefExprClass, Empty) {} @@ -497,6 +502,9 @@ public: return isFreeIvar() ? Loc : getBase()->getLocStart(); } SourceLocation getLocEnd() const LLVM_READONLY { return Loc; } + + SourceLocation getOpLoc() const { return OpLoc; } + void setOpLoc(SourceLocation L) { OpLoc = L; } static bool classof(const Stmt *T) { return T->getStmtClass() == ObjCIvarRefExprClass; diff --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp index 9ee3c8b60e0d..e8378becf125 100644 --- a/clang/lib/CodeGen/CGObjC.cpp +++ b/clang/lib/CodeGen/CGObjC.cpp @@ -1189,7 +1189,8 @@ CodeGenFunction::generateObjCSetterBody(const ObjCImplementationDecl *classImpl, selfDecl->getType(), CK_LValueToRValue, &self, VK_RValue); ObjCIvarRefExpr ivarRef(ivar, ivar->getType().getNonReferenceType(), - SourceLocation(), &selfLoad, true, true); + SourceLocation(), SourceLocation(), + &selfLoad, true, true); ParmVarDecl *argDecl = *setterMethod->param_begin(); QualType argType = argDecl->getType().getNonReferenceType(); diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp index c1d86550a53a..5459d74016f2 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -456,7 +456,8 @@ static void CheckForNullPointerDereference(Sema &S, Expr *E) { } static void DiagnoseDirectIsaAccess(Sema &S, const ObjCIvarRefExpr *OIRE, - bool IsAssign) { + SourceLocation AssignLoc, + const Expr* RHS) { const ObjCIvarDecl *IV = OIRE->getDecl(); if (!IV) return; @@ -476,8 +477,35 @@ static void DiagnoseDirectIsaAccess(Sema &S, const ObjCIvarRefExpr *OIRE, ObjCIvarDecl *IV = IDecl->lookupInstanceVariable(Member, ClassDeclared); if (!ClassDeclared->getSuperClass() && (*ClassDeclared->ivar_begin()) == IV) { - S.Diag(OIRE->getLocation(), IsAssign ? diag::warn_objc_isa_assign - : diag::warn_objc_isa_use); + if (RHS) { + NamedDecl *ObjectSetClass = + S.LookupSingleName(S.TUScope, + &S.Context.Idents.get("object_setClass"), + SourceLocation(), S.LookupOrdinaryName); + if (ObjectSetClass) { + SourceLocation RHSLocEnd = S.PP.getLocForEndOfToken(RHS->getLocEnd()); + S.Diag(OIRE->getExprLoc(), diag::warn_objc_isa_assign) << + FixItHint::CreateInsertion(OIRE->getLocStart(), "object_setClass(") << + FixItHint::CreateReplacement(SourceRange(OIRE->getOpLoc(), + AssignLoc), ",") << + FixItHint::CreateInsertion(RHSLocEnd, ")"); + } + else + S.Diag(OIRE->getLocation(), diag::warn_objc_isa_assign); + } else { + NamedDecl *ObjectGetClass = + S.LookupSingleName(S.TUScope, + &S.Context.Idents.get("object_getClass"), + SourceLocation(), S.LookupOrdinaryName); + if (ObjectGetClass) + S.Diag(OIRE->getExprLoc(), diag::warn_objc_isa_use) << + FixItHint::CreateInsertion(OIRE->getLocStart(), "object_getClass(") << + FixItHint::CreateReplacement( + SourceRange(OIRE->getOpLoc(), + OIRE->getLocEnd()), ")"); + else + S.Diag(OIRE->getLocation(), diag::warn_objc_isa_use); + } S.Diag(IV->getLocation(), diag::note_ivar_decl); } } @@ -538,7 +566,7 @@ ExprResult Sema::DefaultLvalueConversion(Expr *E) { } else if (const ObjCIvarRefExpr *OIRE = dyn_cast(E->IgnoreParenCasts())) - DiagnoseDirectIsaAccess(*this, OIRE, false); + DiagnoseDirectIsaAccess(*this, OIRE, SourceLocation(), /* Expr*/0); // C++ [conv.lval]p1: // [...] If T is a non-class type, the type of the prvalue is the @@ -2104,7 +2132,7 @@ Sema::LookupInObjCMethod(LookupResult &Lookup, Scope *S, Diag(Loc, diag::warn_direct_ivar_access) << IV->getDeclName(); ObjCIvarRefExpr *Result = new (Context) ObjCIvarRefExpr(IV, IV->getType(), - Loc, + Loc, IV->getLocation(), SelfExpr.take(), true, true); @@ -8628,7 +8656,7 @@ ExprResult Sema::CreateBuiltinBinOp(SourceLocation OpLoc, } else if (const ObjCIvarRefExpr *OIRE = dyn_cast(LHS.get()->IgnoreParenCasts())) - DiagnoseDirectIsaAccess(*this, OIRE, true); + DiagnoseDirectIsaAccess(*this, OIRE, OpLoc, RHS.get()); if (CompResultTy.isNull()) return Owned(new (Context) BinaryOperator(LHS.take(), RHS.take(), Opc, diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp index 54190edf37d9..847db24632b9 100644 --- a/clang/lib/Sema/SemaExprMember.cpp +++ b/clang/lib/Sema/SemaExprMember.cpp @@ -1252,7 +1252,7 @@ Sema::LookupMemberExpr(LookupResult &R, ExprResult &BaseExpr, } ObjCIvarRefExpr *Result = new (Context) ObjCIvarRefExpr(IV, IV->getType(), - MemberLoc, + MemberLoc, OpLoc, BaseExpr.take(), IsArrow); diff --git a/clang/lib/Sema/SemaObjCProperty.cpp b/clang/lib/Sema/SemaObjCProperty.cpp index c507ab7007bc..7ad8bc722d50 100644 --- a/clang/lib/Sema/SemaObjCProperty.cpp +++ b/clang/lib/Sema/SemaObjCProperty.cpp @@ -1136,6 +1136,7 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, MarkDeclRefReferenced(SelfExpr); Expr *IvarRefExpr = new (Context) ObjCIvarRefExpr(Ivar, Ivar->getType(), PropertyDiagLoc, + Ivar->getLocation(), SelfExpr, true, true); ExprResult Res = PerformCopyInitialization(InitializedEntity::InitializeResult( @@ -1171,6 +1172,7 @@ Decl *Sema::ActOnPropertyImplDecl(Scope *S, MarkDeclRefReferenced(SelfExpr); Expr *lhs = new (Context) ObjCIvarRefExpr(Ivar, Ivar->getType(), PropertyDiagLoc, + Ivar->getLocation(), SelfExpr, true, true); ObjCMethodDecl::param_iterator P = setterMethod->param_begin(); ParmVarDecl *Param = (*P); diff --git a/clang/lib/Serialization/ASTReaderStmt.cpp b/clang/lib/Serialization/ASTReaderStmt.cpp index 6159916c8253..078ecb7a06d6 100644 --- a/clang/lib/Serialization/ASTReaderStmt.cpp +++ b/clang/lib/Serialization/ASTReaderStmt.cpp @@ -893,6 +893,7 @@ void ASTStmtReader::VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) { VisitExpr(E); E->setDecl(ReadDeclAs(Record, Idx)); E->setLocation(ReadSourceLocation(Record, Idx)); + E->setOpLoc(ReadSourceLocation(Record, Idx)); E->setBase(Reader.ReadSubExpr()); E->setIsArrow(Record[Idx++]); E->setIsFreeIvar(Record[Idx++]); diff --git a/clang/lib/Serialization/ASTWriterStmt.cpp b/clang/lib/Serialization/ASTWriterStmt.cpp index bb8afb8652bb..b6f1d54d4079 100644 --- a/clang/lib/Serialization/ASTWriterStmt.cpp +++ b/clang/lib/Serialization/ASTWriterStmt.cpp @@ -857,6 +857,7 @@ void ASTStmtWriter::VisitObjCIvarRefExpr(ObjCIvarRefExpr *E) { VisitExpr(E); Writer.AddDeclRef(E->getDecl(), Record); Writer.AddSourceLocation(E->getLocation(), Record); + Writer.AddSourceLocation(E->getOpLoc(), Record); Writer.AddStmt(E->getBase()); Record.push_back(E->isArrow()); Record.push_back(E->isFreeIvar()); diff --git a/clang/test/FixIt/auto-isa-fixit.m b/clang/test/FixIt/auto-isa-fixit.m index ec0d96851c50..3f22c1838ac0 100644 --- a/clang/test/FixIt/auto-isa-fixit.m +++ b/clang/test/FixIt/auto-isa-fixit.m @@ -17,3 +17,50 @@ Class pr6302(id x123) { x123->isa = (id)(x123->isa); return x123->isa; } + + +@interface BaseClass { +@public + Class isa; // expected-note 3 {{instance variable is declared here}} +} +@end + +@interface OtherClass { +@public + id firstIvar; + Class isa; // note, not first ivar; +} +@end + +@interface Subclass : BaseClass @end + +@interface SiblingClass : BaseClass @end + +@interface Root @end + +@interface hasIsa : Root { +@public + Class isa; // note, isa is not in root class +} +@end + +@implementation Subclass +-(void)method { + hasIsa *u; + id v; + BaseClass *w; + Subclass *x; + SiblingClass *y; + OtherClass *z; + (void)v->isa; + (void)w->isa; + (void)x->isa; + (void)y->isa; + (void)z->isa; + (void)u->isa; + y->isa = 0; + y->isa = w->isa; + x->isa = rhs(); +} +@end +