[cfe-commits] r61929 - in /cfe/trunk: Driver/RewriteObjC.cpp include/clang/AST/DeclBase.h include/clang/AST/DeclObjC.h lib/AST/DeclBase.cpp lib/AST/DeclCXX.cpp lib/AST/DeclObjC.cpp lib/CodeGen/CGObjCGNU.cpp lib/Sema/IdentifierResolver.cpp lib/Sema/Sema.cpp lib/Sema/Sema.h lib/Sema/SemaCXXScopeSpec.cpp lib/Sema/SemaDecl.cpp lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaDeclObjC.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp

Douglas Gregor dgregor at apple.com
Thu Jan 8 10:41:05 PST 2009


On Jan 8, 2009, at 9:28 AM, Steve Naroff wrote:

> Author: snaroff
> Date: Thu Jan  8 11:28:14 2009
> New Revision: 61929
>
> URL: http://llvm.org/viewvc/llvm-project?rev=61929&view=rev
> Log:
> This is a large/messy diff that unifies the ObjC AST's with  
> DeclContext.

Excellent!

> - ObjCContainerDecl's (ObjCInterfaceDecl/ObjCCategoryDecl/ 
> ObjCProtocolDecl), ObjCCategoryImpl, & ObjCImplementation are all  
> DeclContexts.
> - ObjCMethodDecl is now a ScopedDecl (so it can play nicely with  
> DeclContext).
> - ObjCContainerDecl now does iteration/lookup using DeclContext  
> infrastructure (no more linear search:-)

I wonder if we'll actually see a performance improvement from this?  
Nonetheless, it's great from a software-engineering standpoint.

> - Removed ASTContext argument to DeclContext::lookup(). It wasn't  
> being used and complicated it's use from an ObjC AST perspective.

Good.

> - Added Sema::ProcessPropertyDecl() and removed  
> Sema::diagnosePropertySetterGetterMismatch().
> - Simplified Sema::ActOnAtEnd() considerably. Still more work to do.
> - Fixed an incorrect casting assumption in  
> Sema::getCurFunctionOrMethodDecl(), now that ObjCMethodDecl is a  
> ScopedDecl.
> - Removed addPropertyMethods from ObjCInterfaceDecl/ObjCCategoryDecl/ 
> ObjCProtocolDecl.
>
> This passes all the tests on my machine. Since many of the changes  
> are central to the way ObjC finds it's methods, I expect some  
> fallout (and there are still a handful of FIXME's). Nevertheless,  
> this should be a step in the right direction.
>
> Modified:
>    cfe/trunk/Driver/RewriteObjC.cpp
>    cfe/trunk/include/clang/AST/DeclBase.h
>    cfe/trunk/include/clang/AST/DeclObjC.h
>    cfe/trunk/lib/AST/DeclBase.cpp
>    cfe/trunk/lib/AST/DeclCXX.cpp
>    cfe/trunk/lib/AST/DeclObjC.cpp
>    cfe/trunk/lib/CodeGen/CGObjCGNU.cpp
>    cfe/trunk/lib/Sema/IdentifierResolver.cpp
>    cfe/trunk/lib/Sema/Sema.cpp
>    cfe/trunk/lib/Sema/Sema.h
>    cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp
>    cfe/trunk/lib/Sema/SemaDecl.cpp
>    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>    cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>    cfe/trunk/lib/Sema/SemaExprCXX.cpp
>    cfe/trunk/lib/Sema/SemaOverload.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=61929&r1=61928&r2=61929&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
> +++ cfe/trunk/include/clang/AST/DeclObjC.h Thu Jan  8 11:28:14 2009
> @@ -93,7 +93,7 @@
> /// A selector represents a unique name for a method. The selector  
> names for
> /// the above methods are setMenu:, menu, replaceSubview:with:, and  
> defaultMenu.
> ///
> -class ObjCMethodDecl : public NamedDecl, public DeclContext {
> +class ObjCMethodDecl : public ScopedDecl, public DeclContext {
> public:
>   enum ImplementationControl { None, Required, Optional };
> private:
> @@ -115,7 +115,7 @@
>   unsigned objcDeclQualifier : 6;
>
>   // Context this method is declared in.
> -  NamedDecl *MethodContext;
> +  DeclContext *MethodContext;

Isn't MethodContext redundant now? ScopedDecls have getDeclContext().

>   // Type of this method.
>   QualType MethodDeclType;
> @@ -140,17 +140,17 @@
>
>   ObjCMethodDecl(SourceLocation beginLoc, SourceLocation endLoc,
>                  Selector SelInfo, QualType T,
> -                 Decl *contextDecl,
> +                 DeclContext *contextDecl,
>                  bool isInstance = true,
>                  bool isVariadic = false,
>                  bool isSynthesized = false,
>                  ImplementationControl impControl = None)
> -  : NamedDecl(ObjCMethod, beginLoc, SelInfo),
> +  : ScopedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo, 0),
>     DeclContext(ObjCMethod),
>     IsInstance(isInstance), IsVariadic(isVariadic),
>     IsSynthesized(isSynthesized),
>     DeclImplementation(impControl), objcDeclQualifier(OBJC_TQ_None),
> -    MethodContext(static_cast<NamedDecl*>(contextDecl)),
> +    MethodContext(contextDecl),
>     MethodDeclType(T),
>     ParamInfo(0), NumMethodParams(0),
>     EndLoc(endLoc), Body(0), SelfDecl(0), CmdDecl(0) {}
> @@ -165,7 +165,7 @@
>   static ObjCMethodDecl *Create(ASTContext &C,
>                                 SourceLocation beginLoc,
>                                 SourceLocation endLoc, Selector  
> SelInfo,
> -                                QualType T, Decl *contextDecl,
> +                                QualType T, DeclContext *contextDecl,
>                                 bool isInstance = true,
>                                 bool isVariadic = false,
>                                 bool isSynthesized = false,
> @@ -183,7 +183,7 @@
>     return SourceRange(getLocation(), EndLoc);
>   }
>
> -  NamedDecl *getMethodContext() const { return MethodContext; }
> +  DeclContext *getMethodContext() const { return MethodContext; }
>
>   ObjCInterfaceDecl *getClassInterface();
>   const ObjCInterfaceDecl *getClassInterface() const {
> @@ -252,68 +252,177 @@
>
> /// ObjCContainerDecl - Represents a container for method  
> declarations.
> /// Current sub-classes are ObjCInterfaceDecl, ObjCCategoryDecl, and
> -/// ObjCProtocolDecl. FIXME: Use for ObjC implementation decls.
> -/// STILL UNDER CONSTRUCTION...
> +/// ObjCProtocolDecl.
> +/// FIXME: Use for ObjC implementation decls.
> +/// FIXME: Consider properties.
> +/// FIXME: It would be nice to reduce amount of "boilerplate"  
> iterator code
> +/// below. For now, the iterators are modeled after  
> RecordDecl::field_iterator().
> +/// If DeclContext ends up providing some support for creating more  
> strongly
> +/// typed iterators, the code below should be reduced considerably.

I'll build an iterator adaptor to make this stuff easier in the future.

> ///
> -class ObjCContainerDecl : public NamedDecl {
> -  /// instance methods
> -  ObjCMethodDecl **InstanceMethods;  // Null if not defined
> -  unsigned NumInstanceMethods;  // 0 if none.
> -
> -  /// class methods
> -  ObjCMethodDecl **ClassMethods;  // Null if not defined
> -  unsigned NumClassMethods;  // 0 if none
> -

Yay! :)

> +class ObjCContainerDecl : public NamedDecl, public DeclContext {
>   SourceLocation AtEndLoc; // marks the end of the method container.
> public:
>
>   ObjCContainerDecl(Kind DK, SourceLocation L, IdentifierInfo *Id)
> -    : NamedDecl(DK, L, Id),
> -      InstanceMethods(0), NumInstanceMethods(0),
> -      ClassMethods(0), NumClassMethods(0) {}
> +    : NamedDecl(DK, L, Id), DeclContext(DK) {}
>
>   virtual ~ObjCContainerDecl();
>
> -  typedef ObjCMethodDecl * const * instmeth_iterator;
> -  instmeth_iterator instmeth_begin() const { return  
> InstanceMethods; }
> -  instmeth_iterator instmeth_end() const {
> -    return InstanceMethods+NumInstanceMethods;
> +  // Iterator access to instance/class methods.
> +  class method_iterator {
> +  protected:
> +    /// Current - Current position within the sequence of  
> declarations
> +    /// in this record.
> +    DeclContext::decl_iterator Current;
> +
> +    /// End - Last position in the sequence of declarations in this
> +    /// record.
> +    DeclContext::decl_iterator End;
> +
> +    /// IsInstance - If true, we are iterating through instance  
> methods.
> +    /// If false, we are iteratring through class methods.
> +    bool IsInstance;
> +
> +    /// SkipToNextMethod - Advances the current position up to the  
> next
> +    /// ObjCMethodDecl.
> +    void SkipToNextMethod() {
> +      while (Current != End) {
> +        ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(*Current);
> +        if (M &&
> +            (IsInstance && M->isInstance() || !IsInstance && !M- 
> >isInstance()))
> +          return;
> +        ++Current;
> +      }
> +    }
> +
> +  public:
> +    typedef ObjCMethodDecl const *    value_type;
> +    typedef ObjCMethodDecl const *    reference;
> +    typedef ObjCMethodDecl const *    pointer;
> +    typedef std::ptrdiff_t            difference_type;
> +    typedef std::forward_iterator_tag iterator_category;
> +
> +    method_iterator() : Current(), End(), IsInstance(true) { }
> +
> +    method_iterator(DeclContext::decl_iterator C,
> +                    DeclContext::decl_iterator E, bool I)
> +      : Current(C), End(E), IsInstance(I) {
> +      SkipToNextMethod();
> +    }
> +
> +    reference operator*() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    pointer operator->() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    method_iterator& operator++() {
> +      ++Current;
> +      SkipToNextMethod();
> +      return *this;
> +    }
> +
> +    method_iterator operator++(int) {
> +      method_iterator tmp(*this);
> +      ++(*this);
> +      return tmp;
> +    }
> +
> +    friend bool
> +    operator==(const method_iterator& x, const method_iterator& y) {
> +      return x.Current == y.Current;
> +    }
> +
> +    friend bool
> +    operator!=(const method_iterator& x, const method_iterator& y) {
> +      return x.Current != y.Current;
> +    }
> +  };
> +
> +  class instmeth_iterator : public method_iterator {
> +  public:
> +    typedef ObjCMethodDecl*           value_type;
> +    typedef ObjCMethodDecl*           reference;
> +    typedef ObjCMethodDecl*           pointer;
> +
> +    instmeth_iterator() : method_iterator() { }
> +
> +    instmeth_iterator(DeclContext::decl_iterator C,  
> DeclContext::decl_iterator E)
> +      : method_iterator(C, E, true) { }
> +
> +    reference operator*() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    pointer operator->() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    instmeth_iterator& operator++() {
> +      ++Current;
> +      SkipToNextMethod();
> +      return *this;
> +    }
> +
> +    instmeth_iterator operator++(int) {
> +      instmeth_iterator tmp(*this);
> +      ++(*this);
> +      return tmp;
> +    }
> +  };
> +
> +  instmeth_iterator instmeth_begin() const {
> +    return instmeth_iterator(decls_begin(), decls_end());
>   }
> -
> -  typedef ObjCMethodDecl * const * classmeth_iterator;
> -  classmeth_iterator classmeth_begin() const { return ClassMethods; }
> -  classmeth_iterator classmeth_end() const {
> -    return ClassMethods+NumClassMethods;
> +  instmeth_iterator instmeth_end() const {
> +    return instmeth_iterator(decls_end(), decls_end());
>   }
>
> -  // Get the local instance method declared in this interface.
> -  ObjCMethodDecl *getInstanceMethod(Selector Sel) const {
> -    for (instmeth_iterator I = instmeth_begin(), E = instmeth_end();
> -         I != E; ++I) {
> -      if ((*I)->getSelector() == Sel)
> -        return *I;
> +  class classmeth_iterator : public method_iterator {
> +  public:
> +    typedef ObjCMethodDecl*           value_type;
> +    typedef ObjCMethodDecl*           reference;
> +    typedef ObjCMethodDecl*           pointer;
> +
> +    classmeth_iterator() : method_iterator() { }
> +
> +    classmeth_iterator(DeclContext::decl_iterator C,  
> DeclContext::decl_iterator E)
> +      : method_iterator(C, E, false) { }
> +
> +    reference operator*() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    pointer operator->() const { return cast<ObjCMethodDecl> 
> (*Current); }
> +
> +    classmeth_iterator& operator++() {
> +      ++Current;
> +      SkipToNextMethod();
> +      return *this;
>     }
> -    return 0;
> -  }
> -  // Get the local class method declared in this interface.
> -  ObjCMethodDecl *getClassMethod(Selector Sel) const {
> -    for (classmeth_iterator I = classmeth_begin(), E = classmeth_end 
> ();
> -         I != E; ++I) {
> -      if ((*I)->getSelector() == Sel)
> -        return *I;
> +
> +    classmeth_iterator operator++(int) {
> +      classmeth_iterator tmp(*this);
> +      ++(*this);
> +      return tmp;
>     }
> -    return 0;
> +  };
> +  classmeth_iterator classmeth_begin() const {
> +    return classmeth_iterator(decls_begin(), decls_end());
>   }
> +  classmeth_iterator classmeth_end() const {
> +    return classmeth_iterator(decls_end(), decls_end());
> +  }
> +
> +  // Get the local instance/class method declared in this interface.
> +  ObjCMethodDecl *getInstanceMethod(Selector Sel) const;
> +  ObjCMethodDecl *getClassMethod(Selector Sel) const;
>
> -  void addMethods(ObjCMethodDecl **insMethods, unsigned  
> numInsMembers,
> -                  ObjCMethodDecl **clsMethods, unsigned  
> numClsMembers,
> -                  SourceLocation AtEndLoc);
> -
> -  unsigned getNumInstanceMethods() const { return  
> NumInstanceMethods; }
> -  unsigned getNumClassMethods() const { return NumClassMethods; }
> +  // Get the number of instance/class methods.
> +  unsigned getNumInstanceMethods() const;
> +  unsigned getNumClassMethods() const;

These getNum*Methods operations are O(n), right? It makes sense to  
document that, so that we have something to point to when someone  
tries to use it in a loop :)

> Modified: cfe/trunk/lib/AST/DeclObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=61929&r1=61928&r2=61929&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/AST/DeclObjC.cpp (original)
> +++ cfe/trunk/lib/AST/DeclObjC.cpp Thu Jan  8 11:28:14 2009
> @@ -24,7 +24,7 @@
>                                        SourceLocation beginLoc,
>                                        SourceLocation endLoc,
>                                        Selector SelInfo, QualType T,
> -                                       Decl *contextDecl,
> +                                       DeclContext *contextDecl,
>                                        bool isInstance,
>                                        bool isVariadic,
>                                        bool isSynthesized,
> @@ -61,8 +61,6 @@
> }
>
> ObjCContainerDecl::~ObjCContainerDecl() {
> -  delete [] InstanceMethods;
> -  delete [] ClassMethods;
> }
>
> ObjCInterfaceDecl::~ObjCInterfaceDecl() {
> @@ -362,7 +360,7 @@
>   assert(RecordForDecl && "lookupFieldDeclForIvar no storage for  
> class");
>   DeclarationName Member = ivar->getDeclName();
>   DeclContext::lookup_result Lookup = (const_cast< RecordDecl *> 
> (RecordForDecl))
> -                                        ->lookup(Context, Member);
> +                                        ->lookup(Member);
>   assert((Lookup.first != Lookup.second) && "field decl not found");
>   FieldDecl *MemberDecl = dyn_cast<FieldDecl>(*Lookup.first);
>   assert(MemberDecl && "field decl not found");
> @@ -382,27 +380,6 @@
>   }
> }
>
> -/// addMethods - Insert instance and methods declarations into
> -/// ObjCInterfaceDecl's InsMethods and ClsMethods fields.
> -///
> -void ObjCContainerDecl::addMethods(ObjCMethodDecl **insMethods,
> -                                   unsigned numInsMembers,
> -                                   ObjCMethodDecl **clsMethods,
> -                                   unsigned numClsMembers,
> -                                   SourceLocation endLoc) {
> -  NumInstanceMethods = numInsMembers;
> -  if (numInsMembers) {
> -    InstanceMethods = new ObjCMethodDecl*[numInsMembers];
> -    memcpy(InstanceMethods, insMethods, numInsMembers*sizeof 
> (ObjCMethodDecl*));
> -  }
> -  NumClassMethods = numClsMembers;
> -  if (numClsMembers) {
> -    ClassMethods = new ObjCMethodDecl*[numClsMembers];
> -    memcpy(ClassMethods, clsMethods, numClsMembers*sizeof 
> (ObjCMethodDecl*));
> -  }
> -  AtEndLoc = endLoc;
> -}
> -
> /// addProperties - Insert property declaration AST nodes into
> /// ObjCInterfaceDecl's PropertyDecl field.
> ///
> @@ -440,18 +417,45 @@
>   }
> }
>
> -static void
> -addPropertyMethods(Decl *D,
> -                   ASTContext &Context,
> -                   ObjCPropertyDecl *property,
> -                   llvm::SmallVector<ObjCMethodDecl*, 32>  
> &insMethods,
> -                   llvm::DenseMap<Selector, const ObjCMethodDecl*>  
> &InsMap) {
> -  ObjCMethodDecl *GetterDecl, *SetterDecl = 0;
> -
> -  GetterDecl = const_cast<ObjCMethodDecl*>(InsMap[property- 
> >getGetterName()]);
> -  if (!property->isReadOnly())
> -    SetterDecl = const_cast<ObjCMethodDecl*>(InsMap[property- 
> >getSetterName()]);
> -
> +// Get the local instance method declared in this interface.
> +// FIXME: handle overloading, instance & class methods can have the  
> same name.
> +ObjCMethodDecl *ObjCContainerDecl::getInstanceMethod(Selector Sel)  
> const {
> +  lookup_const_result MethodResult = lookup(Sel);
> +  if (MethodResult.first)
> +    return const_cast<ObjCMethodDecl*>(
> +             dyn_cast<ObjCMethodDecl>(*MethodResult.first));
> +  return 0;
> +}
> +
> +// Get the local class method declared in this interface.
> +ObjCMethodDecl *ObjCContainerDecl::getClassMethod(Selector Sel)  
> const {
> +  lookup_const_result MethodResult = lookup(Sel);
> +  if (MethodResult.first)
> +    return const_cast<ObjCMethodDecl*>(
> +             dyn_cast<ObjCMethodDecl>(*MethodResult.first));
> +  return 0;
> +}

If we have both an instance method and a class method with the same  
selector, one of getInstanceMethod or getClassMethod will do the wrong  
thing because MethodResult will contain two decls. Each of these  
routines should look through [MethodResult.first, MethodResult.second)  
to see if there is an ObjCMethodDecl for which isInstance has the  
right value.

>
> Modified: cfe/trunk/lib/Sema/Sema.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=61929&r1=61928&r2=61929&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- cfe/trunk/lib/Sema/Sema.cpp (original)
> +++ cfe/trunk/lib/Sema/Sema.cpp Thu Jan  8 11:28:14 2009
> @@ -223,7 +223,7 @@
>   while (isa<BlockDecl>(DC))
>     DC = DC->getParent();
>   if (isa<ObjCMethodDecl>(DC) || isa<FunctionDecl>(DC))
> -    return cast<NamedDecl>(DC);
> +    return cast<ScopedDecl>(DC);

This makes me wonder... should an ObjCMethodDecl eventually be a  
subclass of FunctionDecl, or are the notions too different?

Thanks for cleaning this up!

	- Doug

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


More information about the cfe-commits mailing list