-
Notifications
You must be signed in to change notification settings - Fork 10.5k
Register Dependency Arcs for Extension Members #36099
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
59000e6
d467346
d488f31
ef7dfad
49c89e2
db8e6cc
a99d07a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -56,28 +56,29 @@ using namespace fine_grained_dependencies; | |
// MARK: Helpers for key construction that must be in frontend | ||
//============================================================================== | ||
|
||
static std::string mangleTypeAsContext(const NominalTypeDecl *NTD) { | ||
static std::string identifierForContext(const DeclContext *DC) { | ||
if (!DC) return ""; | ||
|
||
Mangle::ASTMangler Mangler; | ||
return !NTD ? "" : Mangler.mangleTypeAsContextUSR(NTD); | ||
if (const auto *context = dyn_cast<NominalTypeDecl>(DC)) { | ||
return Mangler.mangleTypeAsContextUSR(context); | ||
} | ||
|
||
const auto *ext = cast<ExtensionDecl>(DC); | ||
auto fp = ext->getBodyFingerprint().getValueOr(Fingerprint::ZERO()); | ||
auto typeStr = Mangler.mangleTypeAsContextUSR(ext->getExtendedNominal()); | ||
return (typeStr + "@" + fp.getRawValue()).str(); | ||
} | ||
|
||
//============================================================================== | ||
// MARK: DependencyKey::Builder | ||
//============================================================================== | ||
|
||
template <NodeKind kindArg, typename Entity> | ||
DependencyKey DependencyKey::createForProvidedEntityInterface(Entity entity) { | ||
return DependencyKey::Builder{kindArg, DeclAspect::interface} | ||
.withContext(entity) | ||
.withName(entity) | ||
.build(); | ||
} | ||
|
||
DependencyKey DependencyKey::Builder::build() && { | ||
return DependencyKey{ | ||
kind, | ||
aspect, | ||
context ? mangleTypeAsContext(context) : "", | ||
identifierForContext(context), | ||
name.str() | ||
}; | ||
} | ||
|
@@ -92,24 +93,29 @@ DependencyKey::Builder DependencyKey::Builder::fromReference( | |
case Kind::Tombstone: | ||
llvm_unreachable("Cannot enumerate dead reference!"); | ||
case Kind::PotentialMember: | ||
return Builder{kind, aspect}.withContext(ref.subject); | ||
return Builder{kind, aspect}.withContext(ref.subject->getAsDecl()); | ||
case Kind::TopLevel: | ||
case Kind::Dynamic: | ||
return Builder{kind, aspect, nullptr, ref.name.userFacingName()}; | ||
case Kind::UsedMember: | ||
return Builder{kind, aspect, nullptr, ref.name.userFacingName()} | ||
.withContext(ref.subject); | ||
.withContext(ref.subject->getAsDecl()); | ||
} | ||
} | ||
|
||
DependencyKey::Builder DependencyKey::Builder::withContext(const Decl *D) && { | ||
switch (kind) { | ||
case NodeKind::nominal: | ||
case NodeKind::potentialMember: | ||
case NodeKind::member: | ||
case NodeKind::member: { | ||
/// nominal and potential member dependencies are created from a Decl and | ||
/// use the context field. | ||
return Builder{kind, aspect, cast<NominalTypeDecl>(D), name}; | ||
const DeclContext *context = dyn_cast<NominalTypeDecl>(D); | ||
if (!context) { | ||
context = cast<ExtensionDecl>(D); | ||
} | ||
return Builder{kind, aspect, context, name}; | ||
} | ||
case NodeKind::topLevel: | ||
case NodeKind::dynamicLookup: | ||
case NodeKind::externalDepend: | ||
|
@@ -411,6 +417,7 @@ void FrontendSourceFileDepGraphFactory::addAllDefinedDecls() { | |
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topNominals); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topValues); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.allNominals); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.extensions); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::potentialMember>( | ||
declFinder.potentialMemberHolders); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::member>( | ||
|
@@ -463,16 +470,28 @@ class UsedDeclEnumerator { | |
void enumerateNominalUses(UseEnumerator enumerator) { | ||
auto &Ctx = SF->getASTContext(); | ||
Ctx.evaluator.enumerateReferencesInFile(SF, [&](const auto &ref) { | ||
const NominalTypeDecl *subject = ref.subject; | ||
const DeclContext *subject = ref.subject; | ||
if (!subject) { | ||
return; | ||
} | ||
|
||
auto key = | ||
auto nominalKey = | ||
DependencyKey::Builder(NodeKind::nominal, DeclAspect::interface) | ||
.withContext(subject) | ||
.withContext(subject->getSelfNominalTypeDecl()) | ||
.build(); | ||
enumerateUse(key, enumerator); | ||
enumerateUse(nominalKey, enumerator); | ||
|
||
auto declKey = | ||
DependencyKey::Builder(NodeKind::nominal, DeclAspect::interface) | ||
.withContext(subject->getAsDecl()) | ||
.build(); | ||
|
||
// If the subject of this dependency is not the nominal type itself, | ||
// record another arc for the extension this member came from. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This comment needs another sentence or two explaining the "why". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, it ought to explain why just the interface and not the implementation for the sake of future maintainers. |
||
if (nominalKey != declKey) { | ||
assert(isa<ExtensionDecl>(subject)); | ||
enumerateUse(declKey, enumerator); | ||
} | ||
}); | ||
} | ||
|
||
|
@@ -579,6 +598,7 @@ void ModuleDepGraphFactory::addAllDefinedDecls() { | |
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topNominals); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::topLevel>(declFinder.topValues); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.allNominals); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::nominal>(declFinder.extensions); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::potentialMember>( | ||
declFinder.potentialMemberHolders); | ||
addAllDefinedDeclsOfAGivenType<NodeKind::member>( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -305,6 +305,13 @@ void DirectLookupRequest::writeDependencySink( | |
evaluator::DependencyCollector &tracker, | ||
const TinyPtrVector<ValueDecl *> &result) const { | ||
auto &desc = std::get<0>(getStorage()); | ||
// Add used members from the perspective of | ||
// 1) The decl context they are found in | ||
// 2) The decl context of the request | ||
// This gets us a dependency not just on `Foo.bar` but on `extension Foo.bar`. | ||
Comment on lines
+308
to
+311
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent! |
||
for (const auto *member : result) { | ||
tracker.addUsedMember(member->getDeclContext(), desc.Name.getBaseName()); | ||
} | ||
tracker.addUsedMember(desc.DC, desc.Name.getBaseName()); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
public struct S { | ||
private | ||
static func foo(_ i: Int) {print("1: other:2 commented out")} | ||
} | ||
extension S { | ||
// private // commented out to ensure we see a change to the fingerprint | ||
static func foo2(_ i: Int) {print("2: other:6 commented out")} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
public struct S { | ||
private // commenting out this line works | ||
static func foo(_ i: Int) {print("1: other:2 commented out")} | ||
} | ||
extension S { | ||
private // commenting out this line fails | ||
static func foo2(_ i: Int) {print("2: other:6 commented out")} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
extension S { | ||
static func foo<I: SignedInteger>(_ si: I) { | ||
print("1: other:2 not commented out") | ||
} | ||
static func foo2<I: SignedInteger>(_ si: I) { | ||
print("2: other:6 not commented out") | ||
} | ||
} | ||
|
||
S.foo(3) | ||
S.foo2(3) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
{ | ||
"main.swift": { | ||
"object": "./main.o", | ||
"swift-dependencies": "./main.swiftdeps" | ||
}, | ||
"definesS.swift": { | ||
"object": "./definesS.o", | ||
"swift-dependencies": "./definesS.swiftdeps" | ||
}, | ||
"": { | ||
"swift-dependencies": "./main~buildrecord.swiftdeps" | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
public protocol P { | ||
func foo() | ||
} | ||
|
||
public protocol Q: P { | ||
func bar() | ||
} | ||
|
||
public struct S {} | ||
|
||
extension S: P { | ||
public func foo() {} | ||
} | ||
|
||
extension S: Q { | ||
public func bar() {} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
public protocol P { | ||
func foo() | ||
} | ||
|
||
public protocol Q: P { | ||
func bar() | ||
} | ||
|
||
public struct S {} | ||
|
||
extension S: P { | ||
public func foo() {} | ||
public func bar() {} | ||
} | ||
|
||
extension S: Q { | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
extension S { | ||
public func foo(_ parameter: Int = 42) {} | ||
public func bar(_ parameter: Int = 42) {} | ||
} | ||
|
||
S().foo() | ||
S().bar() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not new with this PR, and a nit: When I saw the "cast" I realized that
withContext
will fail an assert when theD
parameter is the wrong kind ofDecl
. Perhaps the declaration ofwithContext
in the header file would benefit from a comment stating the required types of that parameter?