<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><br><div><div>On Jan 8, 2009, at 9:28 AM, Steve Naroff wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>Author: snaroff<br>Date: Thu Jan 8 11:28:14 2009<br>New Revision: 61929<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=61929&view=rev">http://llvm.org/viewvc/llvm-project?rev=61929&view=rev</a><br>Log:<br>This is a large/messy diff that unifies the ObjC AST's with DeclContext.<br></div></blockquote><div><br></div><div>Excellent!</div><br><blockquote type="cite"><div>- ObjCContainerDecl's (ObjCInterfaceDecl/ObjCCategoryDecl/ObjCProtocolDecl), ObjCCategoryImpl, & ObjCImplementation are all DeclContexts.<br>- ObjCMethodDecl is now a ScopedDecl (so it can play nicely with DeclContext).<br>- ObjCContainerDecl now does iteration/lookup using DeclContext infrastructure (no more linear search:-)<br></div></blockquote><div><br></div><div>I wonder if we'll actually see a performance improvement from this? Nonetheless, it's great from a software-engineering standpoint.</div><br><blockquote type="cite"><div>- Removed ASTContext argument to DeclContext::lookup(). It wasn't being used and complicated it's use from an ObjC AST perspective.<br></div></blockquote><div><br></div><div>Good.</div><br><blockquote type="cite"><div>- Added Sema::ProcessPropertyDecl() and removed Sema::diagnosePropertySetterGetterMismatch().<br>- Simplified Sema::ActOnAtEnd() considerably. Still more work to do.<br>- Fixed an incorrect casting assumption in Sema::getCurFunctionOrMethodDecl(), now that ObjCMethodDecl is a ScopedDecl.<br>- Removed addPropertyMethods from ObjCInterfaceDecl/ObjCCategoryDecl/ObjCProtocolDecl.<br><br>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.<br><br>Modified:<br> cfe/trunk/Driver/RewriteObjC.cpp<br> cfe/trunk/include/clang/AST/DeclBase.h<br> cfe/trunk/include/clang/AST/DeclObjC.h<br> cfe/trunk/lib/AST/DeclBase.cpp<br> cfe/trunk/lib/AST/DeclCXX.cpp<br> cfe/trunk/lib/AST/DeclObjC.cpp<br> cfe/trunk/lib/CodeGen/CGObjCGNU.cpp<br> cfe/trunk/lib/Sema/IdentifierResolver.cpp<br> cfe/trunk/lib/Sema/Sema.cpp<br> cfe/trunk/lib/Sema/Sema.h<br> cfe/trunk/lib/Sema/SemaCXXScopeSpec.cpp<br> cfe/trunk/lib/Sema/SemaDecl.cpp<br> cfe/trunk/lib/Sema/SemaDeclCXX.cpp<br> cfe/trunk/lib/Sema/SemaDeclObjC.cpp<br> cfe/trunk/lib/Sema/SemaExprCXX.cpp<br> cfe/trunk/lib/Sema/SemaOverload.cpp<br><br>Modified: cfe/trunk/include/clang/AST/DeclObjC.h<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=61929&r1=61928&r2=61929&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=61929&r1=61928&r2=61929&view=diff</a><br><br>==============================================================================<br>--- cfe/trunk/include/clang/AST/DeclObjC.h (original)<br>+++ cfe/trunk/include/clang/AST/DeclObjC.h Thu Jan 8 11:28:14 2009<br>@@ -93,7 +93,7 @@<br> /// A selector represents a unique name for a method. The selector names for<br> /// the above methods are setMenu:, menu, replaceSubview:with:, and defaultMenu.<br> ///<br>-class ObjCMethodDecl : public NamedDecl, public DeclContext {<br>+class ObjCMethodDecl : public ScopedDecl, public DeclContext {<br> public:<br> enum ImplementationControl { None, Required, Optional };<br> private:<br>@@ -115,7 +115,7 @@<br> unsigned objcDeclQualifier : 6;<br><br> // Context this method is declared in.<br>- NamedDecl *MethodContext;<br>+ DeclContext *MethodContext;<br></div></blockquote><div><br></div><div>Isn't MethodContext redundant now? ScopedDecls have getDeclContext().</div><br><blockquote type="cite"><div> // Type of this method.<br> QualType MethodDeclType;<br>@@ -140,17 +140,17 @@<br><br> ObjCMethodDecl(SourceLocation beginLoc, SourceLocation endLoc,<br> Selector SelInfo, QualType T,<br>- Decl *contextDecl,<br>+ DeclContext *contextDecl,<br> bool isInstance = true,<br> bool isVariadic = false,<br> bool isSynthesized = false,<br> ImplementationControl impControl = None)<br>- : NamedDecl(ObjCMethod, beginLoc, SelInfo),<br>+ : ScopedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo, 0),<br> DeclContext(ObjCMethod),<br> IsInstance(isInstance), IsVariadic(isVariadic),<br> IsSynthesized(isSynthesized),<br> DeclImplementation(impControl), objcDeclQualifier(OBJC_TQ_None),<br>- MethodContext(static_cast<NamedDecl*>(contextDecl)),<br>+ MethodContext(contextDecl),<br> MethodDeclType(T), <br> ParamInfo(0), NumMethodParams(0), <br> EndLoc(endLoc), Body(0), SelfDecl(0), CmdDecl(0) {}<br>@@ -165,7 +165,7 @@<br> static ObjCMethodDecl *Create(ASTContext &C,<br> SourceLocation beginLoc, <br> SourceLocation endLoc, Selector SelInfo,<br>- QualType T, Decl *contextDecl,<br>+ QualType T, DeclContext *contextDecl,<br> bool isInstance = true,<br> bool isVariadic = false,<br> bool isSynthesized = false,<br>@@ -183,7 +183,7 @@<br> return SourceRange(getLocation(), EndLoc); <br> }<br><br>- NamedDecl *getMethodContext() const { return MethodContext; }<br>+ DeclContext *getMethodContext() const { return MethodContext; }<br><br> ObjCInterfaceDecl *getClassInterface();<br> const ObjCInterfaceDecl *getClassInterface() const {<br>@@ -252,68 +252,177 @@<br><br> /// ObjCContainerDecl - Represents a container for method declarations.<br> /// Current sub-classes are ObjCInterfaceDecl, ObjCCategoryDecl, and<br>-/// ObjCProtocolDecl. FIXME: Use for ObjC implementation decls.<br>-/// STILL UNDER CONSTRUCTION...<br>+/// ObjCProtocolDecl. <br>+/// FIXME: Use for ObjC implementation decls.<br>+/// FIXME: Consider properties.<br>+/// FIXME: It would be nice to reduce amount of "boilerplate" iterator code<br>+/// below. For now, the iterators are modeled after RecordDecl::field_iterator().<br>+/// If DeclContext ends up providing some support for creating more strongly <br>+/// typed iterators, the code below should be reduced considerably.<br></div></blockquote><div><br></div><div>I'll build an iterator adaptor to make this stuff easier in the future.</div><br><blockquote type="cite"><div> ///<br>-class ObjCContainerDecl : public NamedDecl {<br>- /// instance methods<br>- ObjCMethodDecl **InstanceMethods; // Null if not defined<br>- unsigned NumInstanceMethods; // 0 if none.<br>- <br>- /// class methods<br>- ObjCMethodDecl **ClassMethods; // Null if not defined<br>- unsigned NumClassMethods; // 0 if none<br>- <br></div></blockquote><div><br></div>Yay! :)</div><div><br><blockquote type="cite"><div>+class ObjCContainerDecl : public NamedDecl, public DeclContext {<br> SourceLocation AtEndLoc; // marks the end of the method container.<br> public:<br><br> ObjCContainerDecl(Kind DK, SourceLocation L, IdentifierInfo *Id)<br>- : NamedDecl(DK, L, Id),<br>- InstanceMethods(0), NumInstanceMethods(0), <br>- ClassMethods(0), NumClassMethods(0) {}<br>+ : NamedDecl(DK, L, Id), DeclContext(DK) {}<br><br> virtual ~ObjCContainerDecl();<br><br>- typedef ObjCMethodDecl * const * instmeth_iterator;<br>- instmeth_iterator instmeth_begin() const { return InstanceMethods; }<br>- instmeth_iterator instmeth_end() const {<br>- return InstanceMethods+NumInstanceMethods;<br>+ // Iterator access to instance/class methods.<br>+ class method_iterator {<br>+ protected:<br>+ /// Current - Current position within the sequence of declarations<br>+ /// in this record. <br>+ DeclContext::decl_iterator Current;<br>+<br>+ /// End - Last position in the sequence of declarations in this<br>+ /// record.<br>+ DeclContext::decl_iterator End;<br>+<br>+ /// IsInstance - If true, we are iterating through instance methods.<br>+ /// If false, we are iteratring through class methods.<br>+ bool IsInstance;<br>+ <br>+ /// SkipToNextMethod - Advances the current position up to the next<br>+ /// ObjCMethodDecl.<br>+ void SkipToNextMethod() {<br>+ while (Current != End) {<br>+ ObjCMethodDecl *M = dyn_cast<ObjCMethodDecl>(*Current);<br>+ if (M && <br>+ (IsInstance && M->isInstance() || !IsInstance && !M->isInstance()))<br>+ return;<br>+ ++Current;<br>+ }<br>+ }<br>+<br>+ public:<br>+ typedef ObjCMethodDecl const * value_type;<br>+ typedef ObjCMethodDecl const * reference;<br>+ typedef ObjCMethodDecl const * pointer;<br>+ typedef std::ptrdiff_t difference_type;<br>+ typedef std::forward_iterator_tag iterator_category;<br>+<br>+ method_iterator() : Current(), End(), IsInstance(true) { }<br>+<br>+ method_iterator(DeclContext::decl_iterator C, <br>+ DeclContext::decl_iterator E, bool I)<br>+ : Current(C), End(E), IsInstance(I) {<br>+ SkipToNextMethod();<br>+ }<br>+<br>+ reference operator*() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ pointer operator->() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ method_iterator& operator++() {<br>+ ++Current;<br>+ SkipToNextMethod();<br>+ return *this;<br>+ }<br>+<br>+ method_iterator operator++(int) {<br>+ method_iterator tmp(*this);<br>+ ++(*this);<br>+ return tmp;<br>+ }<br>+<br>+ friend bool<br>+ operator==(const method_iterator& x, const method_iterator& y) {<br>+ return x.Current == y.Current;<br>+ }<br>+<br>+ friend bool <br>+ operator!=(const method_iterator& x, const method_iterator& y) {<br>+ return x.Current != y.Current;<br>+ }<br>+ };<br>+<br>+ class instmeth_iterator : public method_iterator {<br>+ public:<br>+ typedef ObjCMethodDecl* value_type;<br>+ typedef ObjCMethodDecl* reference;<br>+ typedef ObjCMethodDecl* pointer;<br>+<br>+ instmeth_iterator() : method_iterator() { }<br>+<br>+ instmeth_iterator(DeclContext::decl_iterator C, DeclContext::decl_iterator E)<br>+ : method_iterator(C, E, true) { } <br>+<br>+ reference operator*() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ pointer operator->() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ instmeth_iterator& operator++() {<br>+ ++Current;<br>+ SkipToNextMethod();<br>+ return *this;<br>+ }<br>+<br>+ instmeth_iterator operator++(int) {<br>+ instmeth_iterator tmp(*this);<br>+ ++(*this);<br>+ return tmp;<br>+ }<br>+ };<br>+<br>+ instmeth_iterator instmeth_begin() const {<br>+ return instmeth_iterator(decls_begin(), decls_end());<br> }<br>- <br>- typedef ObjCMethodDecl * const * classmeth_iterator;<br>- classmeth_iterator classmeth_begin() const { return ClassMethods; }<br>- classmeth_iterator classmeth_end() const {<br>- return ClassMethods+NumClassMethods;<br>+ instmeth_iterator instmeth_end() const {<br>+ return instmeth_iterator(decls_end(), decls_end());<br> }<br><br>- // Get the local instance method declared in this interface.<br>- ObjCMethodDecl *getInstanceMethod(Selector Sel) const {<br>- for (instmeth_iterator I = instmeth_begin(), E = instmeth_end(); <br>- I != E; ++I) {<br>- if ((*I)->getSelector() == Sel)<br>- return *I;<br>+ class classmeth_iterator : public method_iterator {<br>+ public:<br>+ typedef ObjCMethodDecl* value_type;<br>+ typedef ObjCMethodDecl* reference;<br>+ typedef ObjCMethodDecl* pointer;<br>+<br>+ classmeth_iterator() : method_iterator() { }<br>+<br>+ classmeth_iterator(DeclContext::decl_iterator C, DeclContext::decl_iterator E)<br>+ : method_iterator(C, E, false) { } <br>+<br>+ reference operator*() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ pointer operator->() const { return cast<ObjCMethodDecl>(*Current); }<br>+<br>+ classmeth_iterator& operator++() {<br>+ ++Current;<br>+ SkipToNextMethod();<br>+ return *this;<br> }<br>- return 0;<br>- }<br>- // Get the local class method declared in this interface.<br>- ObjCMethodDecl *getClassMethod(Selector Sel) const {<br>- for (classmeth_iterator I = classmeth_begin(), E = classmeth_end(); <br>- I != E; ++I) {<br>- if ((*I)->getSelector() == Sel)<br>- return *I;<br>+<br>+ classmeth_iterator operator++(int) {<br>+ classmeth_iterator tmp(*this);<br>+ ++(*this);<br>+ return tmp;<br> }<br>- return 0;<br>+ };<br>+ classmeth_iterator classmeth_begin() const {<br>+ return classmeth_iterator(decls_begin(), decls_end());<br> }<br>+ classmeth_iterator classmeth_end() const {<br>+ return classmeth_iterator(decls_end(), decls_end());<br>+ }<br>+<br>+ // Get the local instance/class method declared in this interface.<br>+ ObjCMethodDecl *getInstanceMethod(Selector Sel) const;<br>+ ObjCMethodDecl *getClassMethod(Selector Sel) const;<br><br>- void addMethods(ObjCMethodDecl **insMethods, unsigned numInsMembers,<br>- ObjCMethodDecl **clsMethods, unsigned numClsMembers,<br>- SourceLocation AtEndLoc);<br>- <br>- unsigned getNumInstanceMethods() const { return NumInstanceMethods; }<br>- unsigned getNumClassMethods() const { return NumClassMethods; }<br>+ // Get the number of instance/class methods.<br>+ unsigned getNumInstanceMethods() const;<br>+ unsigned getNumClassMethods() const;<br></div></blockquote><div><br></div><div>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 :)</div><br><blockquote type="cite"><div>Modified: cfe/trunk/lib/AST/DeclObjC.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=61929&r1=61928&r2=61929&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=61929&r1=61928&r2=61929&view=diff</a><br><br>==============================================================================<br>--- cfe/trunk/lib/AST/DeclObjC.cpp (original)<br>+++ cfe/trunk/lib/AST/DeclObjC.cpp Thu Jan 8 11:28:14 2009<br>@@ -24,7 +24,7 @@<br> SourceLocation beginLoc, <br> SourceLocation endLoc,<br> Selector SelInfo, QualType T,<br>- Decl *contextDecl,<br>+ DeclContext *contextDecl,<br> bool isInstance,<br> bool isVariadic,<br> bool isSynthesized,<br>@@ -61,8 +61,6 @@<br> }<br><br> ObjCContainerDecl::~ObjCContainerDecl() {<br>- delete [] InstanceMethods;<br>- delete [] ClassMethods;<br> }<br><br> ObjCInterfaceDecl::~ObjCInterfaceDecl() {<br>@@ -362,7 +360,7 @@<br> assert(RecordForDecl && "lookupFieldDeclForIvar no storage for class");<br> DeclarationName Member = ivar->getDeclName();<br> DeclContext::lookup_result Lookup = (const_cast< RecordDecl *>(RecordForDecl))<br>- ->lookup(Context, Member);<br>+ ->lookup(Member);<br> assert((Lookup.first != Lookup.second) && "field decl not found");<br> FieldDecl *MemberDecl = dyn_cast<FieldDecl>(*Lookup.first);<br> assert(MemberDecl && "field decl not found");<br>@@ -382,27 +380,6 @@<br> }<br> }<br><br>-/// addMethods - Insert instance and methods declarations into<br>-/// ObjCInterfaceDecl's InsMethods and ClsMethods fields.<br>-///<br>-void ObjCContainerDecl::addMethods(ObjCMethodDecl **insMethods, <br>- unsigned numInsMembers,<br>- ObjCMethodDecl **clsMethods,<br>- unsigned numClsMembers,<br>- SourceLocation endLoc) {<br>- NumInstanceMethods = numInsMembers;<br>- if (numInsMembers) {<br>- InstanceMethods = new ObjCMethodDecl*[numInsMembers];<br>- memcpy(InstanceMethods, insMethods, numInsMembers*sizeof(ObjCMethodDecl*));<br>- }<br>- NumClassMethods = numClsMembers;<br>- if (numClsMembers) {<br>- ClassMethods = new ObjCMethodDecl*[numClsMembers];<br>- memcpy(ClassMethods, clsMethods, numClsMembers*sizeof(ObjCMethodDecl*));<br>- }<br>- AtEndLoc = endLoc;<br>-}<br>-<br> /// addProperties - Insert property declaration AST nodes into<br> /// ObjCInterfaceDecl's PropertyDecl field.<br> ///<br>@@ -440,18 +417,45 @@<br> }<br> }<br><br>-static void <br>-addPropertyMethods(Decl *D,<br>- ASTContext &Context,<br>- ObjCPropertyDecl *property,<br>- llvm::SmallVector<ObjCMethodDecl*, 32> &insMethods,<br>- llvm::DenseMap<Selector, const ObjCMethodDecl*> &InsMap) {<br>- ObjCMethodDecl *GetterDecl, *SetterDecl = 0;<br>- <br>- GetterDecl = const_cast<ObjCMethodDecl*>(InsMap[property->getGetterName()]);<br>- if (!property->isReadOnly())<br>- SetterDecl = const_cast<ObjCMethodDecl*>(InsMap[property->getSetterName()]);<br>- <br>+// Get the local instance method declared in this interface.<br>+// FIXME: handle overloading, instance & class methods can have the same name.<br>+ObjCMethodDecl *ObjCContainerDecl::getInstanceMethod(Selector Sel) const {<br>+ lookup_const_result MethodResult = lookup(Sel);<br>+ if (MethodResult.first)<br>+ return const_cast<ObjCMethodDecl*>(<br>+ dyn_cast<ObjCMethodDecl>(*MethodResult.first));<br>+ return 0;<br>+}<br>+<br>+// Get the local class method declared in this interface.<br>+ObjCMethodDecl *ObjCContainerDecl::getClassMethod(Selector Sel) const {<br>+ lookup_const_result MethodResult = lookup(Sel);<br>+ if (MethodResult.first)<br>+ return const_cast<ObjCMethodDecl*>(<br>+ dyn_cast<ObjCMethodDecl>(*MethodResult.first));<br>+ return 0;<br>+}<br></div></blockquote><div><br></div><div>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.</div><div><br></div><blockquote type="cite"><div><font class="Apple-style-span" color="#000000"><br></font>Modified: cfe/trunk/lib/Sema/Sema.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=61929&r1=61928&r2=61929&view=diff">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=61929&r1=61928&r2=61929&view=diff</a><br><br>==============================================================================<br>--- cfe/trunk/lib/Sema/Sema.cpp (original)<br>+++ cfe/trunk/lib/Sema/Sema.cpp Thu Jan 8 11:28:14 2009<br>@@ -223,7 +223,7 @@<br> while (isa<BlockDecl>(DC))<br> DC = DC->getParent();<br> if (isa<ObjCMethodDecl>(DC) || isa<FunctionDecl>(DC))<br>- return cast<NamedDecl>(DC);<br>+ return cast<ScopedDecl>(DC);<br></div></blockquote><div><br></div><div>This makes me wonder... should an ObjCMethodDecl eventually be a subclass of FunctionDecl, or are the notions too different?</div><div><br></div><div>Thanks for cleaning this up!</div><div><br></div><div><span class="Apple-tab-span" style="white-space:pre"> </span>- Doug<br></div></div><br></body></html>