Skip to content

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

Merged
merged 7 commits into from
Feb 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions include/swift/AST/AbstractSourceFileDepGraphFactory.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ class AbstractSourceFileDepGraphFactory {
for (const auto &declOrPair : contentsVec) {
auto fp =
AbstractSourceFileDepGraphFactory::getFingerprintIfAny(declOrPair);
addADefinedDecl(
DependencyKey::createForProvidedEntityInterface<kind>(declOrPair),
fp);
auto key = DependencyKey::Builder{kind, DeclAspect::interface}
.withContext(declOrPair)
.withName(declOrPair)
.build();
addADefinedDecl(key, fp);
}
}

Expand Down
18 changes: 9 additions & 9 deletions include/swift/AST/DependencyCollector.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

namespace swift {

class NominalTypeDecl;
class DeclContext;

namespace evaluator {

Expand All @@ -50,31 +50,31 @@ struct DependencyCollector {
Dynamic,
} kind;

NominalTypeDecl *subject;
DeclContext *subject;
DeclBaseName name;

private:
Reference(Kind kind, NominalTypeDecl *subject, DeclBaseName name)
Reference(Kind kind, DeclContext *subject, DeclBaseName name)
: kind(kind), subject(subject), name(name) {}

public:
static Reference empty() {
return {Kind::Empty, llvm::DenseMapInfo<NominalTypeDecl *>::getEmptyKey(),
return {Kind::Empty, llvm::DenseMapInfo<DeclContext *>::getEmptyKey(),
llvm::DenseMapInfo<DeclBaseName>::getEmptyKey()};
}

static Reference tombstone() {
return {Kind::Tombstone,
llvm::DenseMapInfo<NominalTypeDecl *>::getTombstoneKey(),
llvm::DenseMapInfo<DeclContext *>::getTombstoneKey(),
llvm::DenseMapInfo<DeclBaseName>::getTombstoneKey()};
}

public:
static Reference usedMember(NominalTypeDecl *subject, DeclBaseName name) {
static Reference usedMember(DeclContext *subject, DeclBaseName name) {
return {Kind::UsedMember, subject, name};
}

static Reference potentialMember(NominalTypeDecl *subject) {
static Reference potentialMember(DeclContext *subject) {
return {Kind::PotentialMember, subject, DeclBaseName()};
}

Expand Down Expand Up @@ -119,7 +119,7 @@ struct DependencyCollector {
/// up front. A used member dependency causes the file to be rebuilt if the
/// definition of that member changes in any way - via
/// deletion, addition, or mutation of a member with that same name.
void addUsedMember(NominalTypeDecl *subject, DeclBaseName name);
void addUsedMember(DeclContext *subject, DeclBaseName name);
/// Registers a reference from the current dependency scope to a
/// "potential member" of the given \p subject type.
///
Expand All @@ -133,7 +133,7 @@ struct DependencyCollector {
///
/// These dependencies are most appropriate for protocol conformances,
/// superclass constraints, and other requirements involving entire types.
void addPotentialMember(NominalTypeDecl *subject);
void addPotentialMember(DeclContext *subject);
/// Registers a reference from the current dependency scope to a given
/// top-level \p name.
///
Expand Down
9 changes: 2 additions & 7 deletions include/swift/AST/FineGrainedDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -426,14 +426,14 @@ class DependencyKey {
private:
const NodeKind kind;
const DeclAspect aspect;
const NominalTypeDecl *context;
const DeclContext *context;
StringRef name;

private:
// A private copy constructor so our clients are forced to use the
// move-only builder interface.
explicit Builder(NodeKind kind, DeclAspect aspect,
const NominalTypeDecl *context, StringRef name)
const DeclContext *context, StringRef name)
: kind(kind), aspect(aspect), context(context), name(name) {}

public:
Expand Down Expand Up @@ -535,11 +535,6 @@ class DependencyKey {
}
bool isInterface() const { return getAspect() == DeclAspect::interface; }

/// Create just the interface half of the keys for a provided Decl or Decl
/// pair
template <NodeKind kind, typename Entity>
static DependencyKey createForProvidedEntityInterface(Entity);

DependencyKey correspondingImplementation() const {
return withAspect(DeclAspect::implementation);
}
Expand Down
8 changes: 5 additions & 3 deletions lib/AST/Evaluator.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
//
//===----------------------------------------------------------------------===//
#include "swift/AST/Evaluator.h"
#include "swift/AST/DeclContext.h"
#include "swift/AST/DiagnosticEngine.h"
#include "swift/Basic/LangOptions.h"
#include "swift/Basic/Range.h"
Expand Down Expand Up @@ -112,13 +113,14 @@ evaluator::DependencyCollector::~DependencyCollector() {
#endif
}

void evaluator::DependencyCollector::addUsedMember(NominalTypeDecl *subject,
void evaluator::DependencyCollector::addUsedMember(DeclContext *subject,
DeclBaseName name) {
assert(subject->isTypeContext());
return parent.recordDependency(Reference::usedMember(subject, name));
}

void evaluator::DependencyCollector::addPotentialMember(
NominalTypeDecl *subject) {
void evaluator::DependencyCollector::addPotentialMember(DeclContext *subject) {
assert(subject->isTypeContext());
return parent.recordDependency(Reference::potentialMember(subject));
}

Expand Down
58 changes: 39 additions & 19 deletions lib/AST/FrontendSourceFileDepGraphFactory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()
};
}
Expand All @@ -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);
Copy link
Contributor

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 the D parameter is the wrong kind of Decl. Perhaps the declaration of withContext in the header file would benefit from a comment stating the required types of that parameter?

}
return Builder{kind, aspect, context, name};
}
case NodeKind::topLevel:
case NodeKind::dynamicLookup:
case NodeKind::externalDepend:
Expand Down Expand Up @@ -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>(
Expand Down Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment needs another sentence or two explaining the "why".

Copy link
Contributor

Choose a reason for hiding this comment

The 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);
}
});
}

Expand Down Expand Up @@ -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>(
Expand Down
7 changes: 7 additions & 0 deletions lib/AST/NameLookupRequests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The 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());
}

Expand Down
6 changes: 3 additions & 3 deletions lib/Frontend/DependencyVerifier.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ struct Obligation {
}
static bool isEqual(const Obligation::Key &LHS,
const Obligation::Key &RHS) {
return LHS.Name == RHS.Name && LHS.Kind == RHS.Kind;
return LHS.Name.equals(RHS.Name) && LHS.Kind == RHS.Kind;
}
};
};
Expand Down Expand Up @@ -395,12 +395,12 @@ bool DependencyVerifier::constructObligations(const SourceFile *SF,
llvm_unreachable("Cannot enumerate dead dependency!");

case NodeKind::PotentialMember: {
auto key = copyQualifiedTypeName(Ctx, reference.subject);
auto key = copyQualifiedTypeName(Ctx, reference.subject->getSelfNominalTypeDecl());
Obligations.insert({Obligation::Key::forPotentialMember(key),
{"", Expectation::Kind::PotentialMember}});
} break;
case NodeKind::UsedMember: {
auto demContext = copyQualifiedTypeName(Ctx, reference.subject);
auto demContext = copyQualifiedTypeName(Ctx, reference.subject->getSelfNominalTypeDecl());
auto name = reference.name.userFacingName();
auto key = Ctx.AllocateCopy((demContext + "." + name).str());
Obligations.insert(
Expand Down
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()
Loading