r179654 - Use the extra info in global method pool to speed up looking for ObjC overridden methods.
Argyrios Kyrtzidis
akyrtzi at gmail.com
Tue Apr 16 17:09:08 PDT 2013
Author: akirtzidis
Date: Tue Apr 16 19:09:08 2013
New Revision: 179654
URL: http://llvm.org/viewvc/llvm-project?rev=179654&view=rev
Log:
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
Modified:
cfe/trunk/lib/AST/DeclObjC.cpp
cfe/trunk/lib/Sema/SemaDeclObjC.cpp
cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
Modified: cfe/trunk/lib/AST/DeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=179654&r1=179653&r2=179654&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclObjC.cpp (original)
+++ cfe/trunk/lib/AST/DeclObjC.cpp Tue Apr 16 19:09:08 2013
@@ -959,52 +959,6 @@ static void collectOverriddenMethodsSlow
}
}
-static void collectOnCategoriesAfterLocation(SourceLocation Loc,
- const ObjCInterfaceDecl *Class,
- SourceManager &SM,
- const ObjCMethodDecl *Method,
- SmallVectorImpl<const ObjCMethodDecl *> &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<const ObjCMethodDecl *> &Methods) {
- assert(!Method->isOverriding());
-
- const ObjCContainerDecl *
- ContD = cast<ObjCContainerDecl>(Method->getDeclContext());
- if (isa<ObjCInterfaceDecl>(ContD) || isa<ObjCProtocolDecl>(ContD))
- return;
- const ObjCInterfaceDecl *Class = Method->getClassInterface();
- if (!Class)
- return;
-
- collectOnCategoriesAfterLocation(Class->getLocation(), Class->getSuperClass(),
- SM, Method, Methods);
-}
-
void ObjCMethodDecl::getOverriddenMethods(
SmallVectorImpl<const ObjCMethodDecl *> &Overridden) const {
const ObjCMethodDecl *Method = this;
@@ -1014,10 +968,7 @@ void ObjCMethodDecl::getOverriddenMethod
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");
Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=179654&r1=179653&r2=179654&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Tue Apr 16 19:09:08 2013
@@ -2809,10 +2809,46 @@ void Sema::CheckObjCMethodOverrides(ObjC
i = overrides.begin(), e = overrides.end(); i != e; ++i) {
ObjCMethodDecl *overridden = *i;
- if (isa<ObjCProtocolDecl>(overridden->getDeclContext()) ||
- CurrentClass != overridden->getClassInterface() ||
- overridden->isOverriding())
- hasOverriddenMethodsInBaseOrProtocol = true;
+ if (!hasOverriddenMethodsInBaseOrProtocol) {
+ if (isa<ObjCProtocolDecl>(overridden->getDeclContext()) ||
+ CurrentClass != overridden->getClassInterface() ||
+ overridden->isOverriding()) {
+ hasOverriddenMethodsInBaseOrProtocol = true;
+
+ } else if (isa<ObjCImplDecl>(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<ObjCCategoryImplDecl>(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())
Modified: cfe/trunk/test/SemaObjC/arc-repeated-weak.mm
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/arc-repeated-weak.mm?rev=179654&r1=179653&r2=179654&view=diff
==============================================================================
--- cfe/trunk/test/SemaObjC/arc-repeated-weak.mm (original)
+++ cfe/trunk/test/SemaObjC/arc-repeated-weak.mm Tue Apr 16 19:09:08 2013
@@ -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]);
}
More information about the cfe-commits
mailing list