Skip to content

Commit 2f33356

Browse files
committed
AST: Optimize construction of the AnyObject dispatch table
Instead of visiting all members of all types and extensions, bail out early if the type is not a class or protocol, or the extension is not extending a class. This means we don't visit structs, enums or protocol extensions at all, which will avoid delayed parsing. Also, we were evaluating isObjC() on each member, which is an expensive operation; if the member does not have an explicit @objc we would still have to check if it overrides an @objc method or witnesses an @objc protocol requirement. Since most members are not ever found by dynamic lookup, this is wasted work. Instead, let's rely on AnyObject lookup filtering non-@objc members at the call site, which it was already doing anyway.
1 parent 14d0bbf commit 2f33356

File tree

7 files changed

+78
-18
lines changed

7 files changed

+78
-18
lines changed

include/swift/AST/DeclContext.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,10 @@ class alignas(1 << DeclContextAlignInBits) DeclContext {
459459
/// are used.
460460
ResilienceExpansion getResilienceExpansion() const;
461461

462+
/// Returns true if this context may possibly contain members visible to
463+
/// AnyObject dynamic lookup.
464+
bool mayContainMembersAccessedByDynamicLookup() const;
465+
462466
/// Returns true if lookups within this context could affect downstream files.
463467
///
464468
/// \param functionsAreNonCascading If true, functions are considered non-
@@ -724,6 +728,7 @@ class IterableDeclContext {
724728
MemberCount = 0;
725729
HasOperatorDeclarations = 0;
726730
HasUnparsedMembers = 0;
731+
HasNestedClassDeclarations = 0;
727732
}
728733

729734
/// Determine the kind of iterable context we have.

lib/AST/Decl.cpp

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2672,26 +2672,14 @@ bool ValueDecl::canBeAccessedByDynamicLookup() const {
26722672
if (!hasName())
26732673
return false;
26742674

2675-
// Dynamic lookup can only find class and protocol members, or extensions of
2676-
// classes.
2677-
auto nominalDC = getDeclContext()->getSelfNominalTypeDecl();
2678-
if (!nominalDC ||
2679-
(!isa<ClassDecl>(nominalDC) && !isa<ProtocolDecl>(nominalDC)))
2680-
return false;
2681-
2682-
// Dynamic lookup cannot find results within a non-protocol generic context,
2683-
// because there is no sensible way to infer the generic arguments.
2684-
if (getDeclContext()->isGenericContext() && !isa<ProtocolDecl>(nominalDC))
2675+
auto *dc = getDeclContext();
2676+
if (!dc->mayContainMembersAccessedByDynamicLookup())
26852677
return false;
26862678

26872679
// Dynamic lookup can find functions, variables, and subscripts.
26882680
if (!isa<FuncDecl>(this) && !isa<VarDecl>(this) && !isa<SubscriptDecl>(this))
26892681
return false;
26902682

2691-
// Dynamic lookup can only find @objc members.
2692-
if (!isObjC())
2693-
return false;
2694-
26952683
return true;
26962684
}
26972685

lib/AST/DeclContext.cpp

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -475,6 +475,25 @@ unsigned DeclContext::getSemanticDepth() const {
475475
return 1 + getParent()->getSemanticDepth();
476476
}
477477

478+
bool DeclContext::mayContainMembersAccessedByDynamicLookup() const {
479+
// Dynamic lookup can only find class and protocol members, or extensions of
480+
// classes.
481+
if (auto *NTD = dyn_cast<NominalTypeDecl>(this)) {
482+
if (isa<ClassDecl>(NTD))
483+
if (!isGenericContext())
484+
return true;
485+
if (auto *PD = dyn_cast<ProtocolDecl>(NTD))
486+
if (PD->getAttrs().hasAttribute<ObjCAttr>())
487+
return true;
488+
} else if (auto *ED = dyn_cast<ExtensionDecl>(this)) {
489+
if (auto *CD = dyn_cast<ClassDecl>(ED->getExtendedNominal()))
490+
if (!CD->isGenericContext())
491+
return true;
492+
}
493+
494+
return false;
495+
}
496+
478497
bool DeclContext::walkContext(ASTWalker &Walker) {
479498
switch (getContextKind()) {
480499
case DeclContextKind::Module:

lib/AST/Module.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -214,9 +214,15 @@ void SourceLookupCache::doPopulateCache(Range decls,
214214
void SourceLookupCache::populateMemberCache(const SourceFile &SF) {
215215
for (const Decl *D : SF.Decls) {
216216
if (const auto *NTD = dyn_cast<NominalTypeDecl>(D)) {
217-
addToMemberCache(NTD->getMembers());
217+
if (!NTD->hasUnparsedMembers() ||
218+
NTD->maybeHasNestedClassDeclarations() ||
219+
NTD->mayContainMembersAccessedByDynamicLookup())
220+
addToMemberCache(NTD->getMembers());
218221
} else if (const auto *ED = dyn_cast<ExtensionDecl>(D)) {
219-
addToMemberCache(ED->getMembers());
222+
if (!ED->hasUnparsedMembers() ||
223+
ED->maybeHasNestedClassDeclarations() ||
224+
ED->mayContainMembersAccessedByDynamicLookup())
225+
addToMemberCache(ED->getMembers());
220226
}
221227
}
222228

@@ -232,7 +238,10 @@ void SourceLookupCache::addToMemberCache(DeclRange decls) {
232238
if (auto NTD = dyn_cast<NominalTypeDecl>(VD)) {
233239
assert(!VD->canBeAccessedByDynamicLookup() &&
234240
"inner types cannot be accessed by dynamic lookup");
235-
addToMemberCache(NTD->getMembers());
241+
if (!NTD->hasUnparsedMembers() ||
242+
NTD->maybeHasNestedClassDeclarations() ||
243+
NTD->mayContainMembersAccessedByDynamicLookup())
244+
addToMemberCache(NTD->getMembers());
236245
} else if (VD->canBeAccessedByDynamicLookup()) {
237246
ClassMembers.add(VD);
238247
}

validation-test/compiler_crashers_2_fixed/0177-rdar-33093935.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,5 @@ extension A {
1010
}
1111

1212
class B: A {
13-
@objc var foo = superclass()?.name // expected-error{{property 'foo' references itself}}
13+
@objc var foo = superclass()?.name
1414
}
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select Sema.IsObjCRequest -Xfrontend=-disable-objc-attr-requires-foundation-module %s
2+
// REQUIRES: OS=macosx
3+
// REQUIRES: asserts
4+
5+
class C${N} {
6+
// This is the member we actually have to validate; we only want there
7+
// to be one definition:
8+
% if int(N) == 1:
9+
@objc func isObjCMember() {}
10+
% end
11+
12+
// Other random @objc and non-@objc members; the test ensures that we're
13+
// not touching these:
14+
func isAnotherObjCMember() {}
15+
func isNonObjCMember() {}
16+
}
17+
18+
func f${N}(a: AnyObject) {
19+
a.isObjCMember!()
20+
}
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
// RUN: %scale-test --sum-multi --begin 5 --end 16 --step 5 --select NumIterableDeclContextParsed -Xfrontend=-disable-objc-attr-requires-foundation-module %s
2+
// REQUIRES: OS=macosx
3+
// REQUIRES: asserts
4+
5+
// Dynamic member lookup should not force delayed parsing of structs, enums or protocol
6+
// extensions.
7+
struct S${N} {}
8+
enum E${N} {}
9+
extension Sequence {}
10+
11+
% if int(N) == 1:
12+
class C {
13+
@objc func isObjCMember() {}
14+
}
15+
% end
16+
17+
func f${N}(a: AnyObject) {
18+
a.isObjCMember!()
19+
}

0 commit comments

Comments
 (0)