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
diff --git a/lib/AST/DeclObjC.cpp b/lib/AST/DeclObjC.cpp
index 5f5ba52..08c59b5 100644
--- a/lib/AST/DeclObjC.cpp
+++ b/lib/AST/DeclObjC.cpp
@@ -959,52 +959,6 @@
}
}
-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 @@
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 a6f8bf7..0e0672c 100644
--- a/lib/Sema/SemaDeclObjC.cpp
+++ b/lib/Sema/SemaDeclObjC.cpp
@@ -2809,10 +2809,46 @@
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())
diff --git a/test/SemaObjC/arc-repeated-weak.mm b/test/SemaObjC/arc-repeated-weak.mm
index eb818a8..b5d9002 100644
--- a/test/SemaObjC/arc-repeated-weak.mm
+++ b/test/SemaObjC/arc-repeated-weak.mm
@@ -345,8 +345,12 @@
@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]);
}
