[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

jahanian fjahanian at apple.com
Thu Jul 26 09:29:27 PDT 2012


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


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120726/58f79279/attachment.html>


More information about the cfe-commits mailing list