[cfe-commits] r160768 - in /cfe/trunk: include/clang/StaticAnalyzer/Core/PathSensitive/Calls.h include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h lib/StaticAnalyzer/Core/Calls.cpp lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp lib/StaticAnalyzer/Core/ExprEngineObjC.cpp test/Analysis/inlining/ test/Analysis/inlining/InlineObjCClassMethod.m
Anna Zaks
ganna at apple.com
Fri Jul 27 12:05:55 PDT 2012
On Jul 26, 2012, at 9:29 AM, jahanian wrote:
> 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.
>
I'll add the assert. Thanks.
>> + 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?
>
The analyzer probably does not care, since this is now used for inlining and we will not have the root class implementations. Though, it is correct to look up the instance method in the root. I am going to refactor this to rely on a lookup method in the AST, so the checking will be performed.
> - Fariborz
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120727/5afd13a4/attachment.html>
More information about the cfe-commits
mailing list