From e7a77727804c12750cb39e8732e26f2a26e4ce0f Mon Sep 17 00:00:00 2001 From: Argyrios Kyrtzidis Date: Wed, 17 Apr 2013 00:09:08 +0000 Subject: [PATCH] Use the extra info in global method pool to speed up looking for ObjC overridden methods. When we are in a implementation, we check the global method pool whether there were category methods with the same selector. If there were none (common case) we don't need to do lookups for overridden methods again. Note that for an interface method (if we don't encounter its implementation), it is considered that it overrides methods that were declared before it, not for category methods introduced after it. This is tradeoff in favor of performance, since it is expensive to do lookups in case there was a category, and moving the global method pool to ASTContext (so we can check it) would increase complexity. rdar://13508196 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@179654 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/AST/DeclObjC.cpp | 51 +----------------------------- lib/Sema/SemaDeclObjC.cpp | 44 +++++++++++++++++++++++--- test/SemaObjC/arc-repeated-weak.mm | 8 +++-- 3 files changed, 47 insertions(+), 56 deletions(-) diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp index 5f5ba52947..08c59b567d 100644 --- a/lib/AST/DeclObjC.cpp +++ b/lib/AST/DeclObjC.cpp @@ -959,52 +959,6 @@ static void collectOverriddenMethodsSlow(const ObjCMethodDecl *Method, } } -static void collectOnCategoriesAfterLocation(SourceLocation Loc, - const ObjCInterfaceDecl *Class, - SourceManager &SM, - const ObjCMethodDecl *Method, - SmallVectorImpl &Methods) { - if (!Class) - return; - - for (ObjCInterfaceDecl::known_categories_iterator - Cat = Class->known_categories_begin(), - CatEnd = Class->known_categories_end(); - Cat != CatEnd; ++Cat) { - if (SM.isBeforeInTranslationUnit(Loc, Cat->getLocation())) - CollectOverriddenMethodsRecurse(*Cat, Method, Methods, true); - } - - collectOnCategoriesAfterLocation(Loc, Class->getSuperClass(), SM, - Method, Methods); -} - -/// \brief Faster collection that is enabled when ObjCMethodDecl::isOverriding() -/// returns false. -/// You'd think that in that case there are no overrides but categories can -/// "introduce" new overridden methods that are missed by Sema because the -/// overrides lookup that it does for methods, inside implementations, will -/// stop at the interface level (if there is a method there) and not look -/// further in super classes. -/// Methods in an implementation can overide methods in super class's category -/// but not in current class's category. But, such methods -static void collectOverriddenMethodsFast(SourceManager &SM, - const ObjCMethodDecl *Method, - SmallVectorImpl &Methods) { - assert(!Method->isOverriding()); - - const ObjCContainerDecl * - ContD = cast(Method->getDeclContext()); - if (isa(ContD) || isa(ContD)) - return; - const ObjCInterfaceDecl *Class = Method->getClassInterface(); - if (!Class) - return; - - collectOnCategoriesAfterLocation(Class->getLocation(), Class->getSuperClass(), - SM, Method, Methods); -} - void ObjCMethodDecl::getOverriddenMethods( SmallVectorImpl &Overridden) const { const ObjCMethodDecl *Method = this; @@ -1014,10 +968,7 @@ void ObjCMethodDecl::getOverriddenMethods( getMethod(Method->getSelector(), Method->isInstanceMethod()); } - if (!Method->isOverriding()) { - collectOverriddenMethodsFast(getASTContext().getSourceManager(), - Method, Overridden); - } else { + if (Method->isOverriding()) { collectOverriddenMethodsSlow(Method, Overridden); assert(!Overridden.empty() && "ObjCMethodDecl's overriding bit is not as expected"); diff --git a/lib/Sema/SemaDeclObjC.cpp b/lib/Sema/SemaDeclObjC.cpp index a6f8bf7f57..0e0672c2c3 100644 --- a/lib/Sema/SemaDeclObjC.cpp +++ b/lib/Sema/SemaDeclObjC.cpp @@ -2809,10 +2809,46 @@ void Sema::CheckObjCMethodOverrides(ObjCMethodDecl *ObjCMethod, i = overrides.begin(), e = overrides.end(); i != e; ++i) { ObjCMethodDecl *overridden = *i; - if (isa(overridden->getDeclContext()) || - CurrentClass != overridden->getClassInterface() || - overridden->isOverriding()) - hasOverriddenMethodsInBaseOrProtocol = true; + if (!hasOverriddenMethodsInBaseOrProtocol) { + if (isa(overridden->getDeclContext()) || + CurrentClass != overridden->getClassInterface() || + overridden->isOverriding()) { + hasOverriddenMethodsInBaseOrProtocol = true; + + } else if (isa(ObjCMethod->getDeclContext())) { + // OverrideSearch will return as "overridden" the same method in the + // interface. For hasOverriddenMethodsInBaseOrProtocol, we need to + // check whether a category of a base class introduced a method with the + // same selector, after the interface method declaration. + // To avoid unnecessary lookups in the majority of cases, we use the + // extra info bits in GlobalMethodPool to check whether there were any + // category methods with this selector. + GlobalMethodPool::iterator It = + MethodPool.find(ObjCMethod->getSelector()); + if (It != MethodPool.end()) { + ObjCMethodList &List = + ObjCMethod->isInstanceMethod()? It->second.first: It->second.second; + unsigned CategCount = List.getBits(); + if (CategCount > 0) { + // If the method is in a category we'll do lookup if there were at + // least 2 category methods recorded, otherwise only one will do. + if (CategCount > 1 || + !isa(overridden->getDeclContext())) { + OverrideSearch overrides(*this, overridden); + for (OverrideSearch::iterator + OI= overrides.begin(), OE= overrides.end(); OI!=OE; ++OI) { + ObjCMethodDecl *SuperOverridden = *OI; + if (CurrentClass != SuperOverridden->getClassInterface()) { + hasOverriddenMethodsInBaseOrProtocol = true; + overridden->setOverriding(true); + break; + } + } + } + } + } + } + } // Propagate down the 'related result type' bit from overridden methods. if (RTC != Sema::RTC_Incompatible && overridden->hasRelatedResultType()) diff --git a/test/SemaObjC/arc-repeated-weak.mm b/test/SemaObjC/arc-repeated-weak.mm index eb818a8737..b5d9002130 100644 --- a/test/SemaObjC/arc-repeated-weak.mm +++ b/test/SemaObjC/arc-repeated-weak.mm @@ -345,8 +345,12 @@ void test1(Sub1 *s) { @end void test2(Sub1 *s) { - use([s prop]); // expected-warning{{weak property 'prop' is accessed multiple times}} - use([s prop]); // expected-note{{also accessed here}} + // This does not warn because the "prop" in "Base1(cat)" was introduced + // after the method declaration and we don't find it as overridden. + // Always looking for overridden methods after the method declaration is expensive + // and it's not clear it is worth it currently. + use([s prop]); + use([s prop]); } -- 2.40.0