[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
steve naroff
snaroff at apple.com
Thu Jan 8 14:17:53 PST 2009
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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090108/c0cb7550/attachment.html>
More information about the cfe-commits
mailing list