LGTM, I have couple of comments below. - Fariborz
On Jul 25, 2012, at 5:27 PM, Anna Zaks wrote: > Author: zaks > Date: Wed Jul 25 19:27:51 2012 > New Revision: 160768 > > URL: http://llvm.org/viewvc/llvm-project?rev=160768&view=rev > Log: > [analyzer] Inline ObjC class methods. > > - Some cleanup(the TODOs) will be done after ObjC method inlining is > complete. > - Simplified CallEvent::getDefinition not to require ISDynamicDispatch > parameter. > - Also addressed Jordan's comments from r160530. > > Added: > cfe/trunk/test/Analysis/inlining/ > cfe/trunk/test/Analysis/inlining/InlineObjCClassMethod.m > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp > cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineObjC.cpp > > Modified: cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h?rev=160768&r1=160767&r2=160768&view=diff > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h Wed Jul > 25 19:27:51 2012 > @@ -121,15 +121,9 @@ > const Decl *getDecl() const; > > /// \brief Returns the definition of the function or method that will be > - /// called. May be null. > - /// > - /// This is used when deciding how to inline the call. > - /// > - /// \param IsDynamicDispatch True if the definition returned may not be the > - /// definition that is actually invoked at runtime. Note that if we have > - /// sufficient type information to devirtualize a dynamic method call, > - /// we will (and \p IsDynamicDispatch will be set to \c false). > - const Decl *getDefinition(bool &IsDynamicDispatch) const; > + /// called. Returns NULL if the definition cannot be found; ex: due to > + /// dynamic dispatch in ObjC methods. > + const Decl *getRuntimeDefinition() const; > > /// \brief Returns the expression whose value will be the result of this > call. > /// May be null. > @@ -289,8 +283,7 @@ > return cast_or_null<FunctionDecl>(CallEvent::getDecl()); > } > > - const Decl *getDefinition(bool &IsDynamicDispatch) const { > - IsDynamicDispatch = false; > + const Decl *getRuntimeDefinition() const { > const FunctionDecl *FD = getDecl(); > // Note that hasBody() will fill FD with the definition FunctionDecl. > if (FD && FD->hasBody(FD)) > @@ -371,7 +364,7 @@ > : SimpleCall(CE, St, LCtx, K) {} > > public: > - const Decl *getDefinition(bool &IsDynamicDispatch) const; > + const Decl *getRuntimeDefinition() const; > > static bool classof(const CallEvent *CA) { > return CA->getKind() >= CE_BEG_CXX_INSTANCE_CALLS && > @@ -457,8 +450,7 @@ > return BR->getDecl(); > } > > - const Decl *getDefinition(bool &IsDynamicDispatch) const { > - IsDynamicDispatch = false; > + const Decl *getRuntimeDefinition() const { > return getBlockDecl(); > } > > @@ -551,7 +543,7 @@ > unsigned getNumArgs() const { return 0; } > > SVal getCXXThisVal() const; > - const Decl *getDefinition(bool &IsDynamicDispatch) const; > + const Decl *getRuntimeDefinition() const; > > static bool classof(const CallEvent *CA) { > return CA->getKind() == CE_CXXDestructor; > @@ -620,6 +612,8 @@ > void getExtraInvalidatedRegions(RegionList &Regions) const; > > QualType getDeclaredResultType() const; > + ObjCMethodDecl *LookupClassMethodDefinition(Selector Sel, > + ObjCInterfaceDecl *ClassDecl) > const; > > public: > ObjCMethodCall(const ObjCMessageExpr *Msg, ProgramStateRef St, > @@ -678,18 +672,23 @@ > llvm_unreachable("Unknown message kind"); > } > > - const Decl *getDefinition(bool &IsDynamicDispatch) const { > - IsDynamicDispatch = true; > - > - const ObjCMethodDecl *MD = getDecl(); > - if (!MD) > - return 0; > + // TODO: We might want to only compute this once (or change the API for > + // getting the parameters). Currently, this gets called 3 times during > + // inlining. > + const Decl *getRuntimeDefinition() const { > > - for (Decl::redecl_iterator I = MD->redecls_begin(), E = > MD->redecls_end(); > - I != E; ++I) { > - if (cast<ObjCMethodDecl>(*I)->isThisDeclarationADefinition()) > - return *I; > + const ObjCMessageExpr *E = getOriginExpr(); getOriginExpr may return null. You should check for that. > + if (E->isInstanceMessage()) { > + return 0; > + } else { > + // This is a calss method. > + // If we have type info for the receiver class, we are calling via > + // class name. > + if (ObjCInterfaceDecl *IDecl = E->getReceiverInterface()) { > + return LookupClassMethodDefinition(E->getSelector(), IDecl); > + } > } > + > return 0; > } > > @@ -770,8 +769,8 @@ > DISPATCH(getDecl); > } > > -inline const Decl *CallEvent::getDefinition(bool &IsDynamicDispatch) const { > - DISPATCH_ARG(getDefinition, IsDynamicDispatch); > +inline const Decl *CallEvent::getRuntimeDefinition() const { > + DISPATCH(getRuntimeDefinition); > } > > inline unsigned CallEvent::getNumArgs() const { > > Modified: > cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h?rev=160768&r1=160767&r2=160768&view=diff > ============================================================================== > --- cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > (original) > +++ cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h > Wed Jul 25 19:27:51 2012 > @@ -471,7 +471,7 @@ > void evalCall(ExplodedNodeSet &Dst, ExplodedNode *Pred, > const SimpleCall &Call); > > - /// \bried Default implementation of call evaluation. > + /// \brief Default implementation of call evaluation. > void defaultEvalCall(NodeBuilder &B, ExplodedNode *Pred, > const CallEvent &Call); > private: > > Modified: cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp?rev=160768&r1=160767&r2=160768&view=diff > ============================================================================== > --- cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp (original) > +++ cfe/trunk/lib/StaticAnalyzer/Core/Calls.cpp Wed Jul 25 19:27:51 2012 > @@ -201,8 +201,7 @@ > > CallEvent::param_iterator > AnyFunctionCall::param_begin(bool UseDefinitionParams) const { > - bool IgnoredDynamicDispatch; > - const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch) > + const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > : getDecl(); > if (!D) > return 0; > @@ -212,8 +211,7 @@ > > CallEvent::param_iterator > AnyFunctionCall::param_end(bool UseDefinitionParams) const { > - bool IgnoredDynamicDispatch; > - const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch) > + const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > : getDecl(); > if (!D) > return 0; > @@ -354,8 +352,8 @@ > } > > > -const Decl *CXXInstanceCall::getDefinition(bool &IsDynamicDispatch) const { > - const Decl *D = SimpleCall::getDefinition(IsDynamicDispatch); > +const Decl *CXXInstanceCall::getRuntimeDefinition() const { > + const Decl *D = SimpleCall::getRuntimeDefinition(); > if (!D) > return 0; > > @@ -368,8 +366,7 @@ > if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal())) > return Devirtualized; > > - IsDynamicDispatch = true; > - return MD; > + return 0; > } > > > @@ -458,8 +455,8 @@ > Regions.push_back(static_cast<const MemRegion *>(Data)); > } > > -const Decl *CXXDestructorCall::getDefinition(bool &IsDynamicDispatch) const { > - const Decl *D = AnyFunctionCall::getDefinition(IsDynamicDispatch); > +const Decl *CXXDestructorCall::getRuntimeDefinition() const { > + const Decl *D = AnyFunctionCall::getRuntimeDefinition(); > if (!D) > return 0; > > @@ -472,15 +469,13 @@ > if (const CXXMethodDecl *Devirtualized = devirtualize(MD, getCXXThisVal())) > return Devirtualized; > > - IsDynamicDispatch = true; > - return MD; > + return 0; > } > > > CallEvent::param_iterator > ObjCMethodCall::param_begin(bool UseDefinitionParams) const { > - bool IgnoredDynamicDispatch; > - const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch) > + const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > : getDecl(); > if (!D) > return 0; > @@ -490,8 +485,7 @@ > > CallEvent::param_iterator > ObjCMethodCall::param_end(bool UseDefinitionParams) const { > - bool IgnoredDynamicDispatch; > - const Decl *D = UseDefinitionParams ? getDefinition(IgnoredDynamicDispatch) > + const Decl *D = UseDefinitionParams ? getRuntimeDefinition() > : getDecl(); > if (!D) > return 0; > @@ -593,3 +587,34 @@ > return OCM_Message; > return static_cast<ObjCMessageKind>(Info.getInt()); > } > + > +// TODO: This implementation is copied from SemaExprObjC.cpp, needs to be > +// factored into the ObjCInterfaceDecl. > +ObjCMethodDecl *ObjCMethodCall::LookupClassMethodDefinition(Selector Sel, > + ObjCInterfaceDecl *ClassDecl) > const { > + ObjCMethodDecl *Method = 0; > + // Lookup in class and all superclasses. > + while (ClassDecl && !Method) { > + if (ObjCImplementationDecl *ImpDecl = ClassDecl->getImplementation()) > + Method = ImpDecl->getClassMethod(Sel); > + > + // Look through local category implementations associated with the class. > + if (!Method) > + Method = ClassDecl->getCategoryClassMethod(Sel); > + > + // Before we give up, check if the selector is an instance method. > + // But only in the root. This matches gcc's behavior and what the > + // runtime expects. > + if (!Method && !ClassDecl->getSuperClass()) { > + Method = ClassDecl->lookupInstanceMethod(Sel); You don't want this, do you? - Fariborz
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
