On Jan 8, 2009, at 1:41 PM, Douglas Gregor wrote:
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.
I doubt it, but it doesn't hurt:-)
- 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().
Fixed.
// 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.
Great. When I move the property stuff "up", this will make a big difference (since ObjCContainerDecl will have multiple iterators).
/// -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 :)
Correct. Will do.
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.
Yes, that's why I added the FIXME. Will do.
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?
The primary user visible difference is naming. Like C++, ObjC methods have an implicit 'self' argument. ObjC also has an implicit '_cmd' argument (but that's more of an implementation detail, though code can access it). Offhand, I don't have a strong opinion on this (I can look into it later).
Thanks for cleaning this up!
No problem. Thanks for your help/review. snaroff
- Doug
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
