Skip to content

[NFC Incremental] Small changes to fine-grained unit test framework #29722

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 1 commit into from
Feb 9, 2020
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
5 changes: 4 additions & 1 deletion include/swift/AST/FineGrainedDependencies.h
Original file line number Diff line number Diff line change
Expand Up @@ -721,7 +721,8 @@ class SourceFileDepGraphNode : public DepGraphNode {
}

std::string humanReadableName() const {
return DepGraphNode::humanReadableName("here");
return DepGraphNode::humanReadableName(getIsProvides() ? "here"
: "somewhere else");
}

bool verify() const {
Expand Down Expand Up @@ -883,6 +884,8 @@ class SourceFileDepGraph {

bool verifySequenceNumber() const;

void emitDotFile(StringRef outputPath, DiagnosticEngine &diags);

private:
void addNode(SourceFileDepGraphNode *n) {
n->setSequenceNumber(allNodes.size());
Expand Down
57 changes: 30 additions & 27 deletions include/swift/Driver/FineGrainedDependencyDriverGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ class ModuleDepGraphNode : public DepGraphNode {
return DepGraphNode::humanReadableName(where);
}

void dump(raw_ostream &) const;

SWIFT_DEBUG_DUMP;
};

Expand Down Expand Up @@ -312,9 +314,11 @@ class ModuleDepGraph {
}

/// For unit tests.
ModuleDepGraph(const bool EnableTypeFingerprints)
: ModuleDepGraph(true, false, EnableTypeFingerprints, false, nullptr) {}

ModuleDepGraph(const bool EnableTypeFingerprints,
const bool EmitDotFilesForDebugging = false)
: ModuleDepGraph(
true, /*emitFineGrainedDependencyDotFileAfterEveryImport=*/
EmitDotFilesForDebugging, EnableTypeFingerprints, false, nullptr) {}

//============================================================================
// MARK: ModuleDepGraph - updating from a switdeps file
Expand All @@ -328,26 +332,27 @@ class ModuleDepGraph {
/// compensates.
Changes loadFromPath(const driver::Job *, StringRef, DiagnosticEngine &);


Changes loadFromSourceFileDepGraph(const driver::Job *cmd,
const SourceFileDepGraph &);
const SourceFileDepGraph &,
DiagnosticEngine &);

/// Also for unit tests
Changes
simulateLoad(const driver::Job *cmd,
llvm::StringMap<std::vector<std::string>> simpleNames,
llvm::StringMap<std::vector<std::pair<std::string, std::string>>>
compoundNames = {},
const bool includePrivateDeps = false,
const bool hadCompilationError = false);
/// Also for unit tests
Changes
simulateLoad(const driver::Job *cmd,
llvm::StringMap<std::vector<std::string>> simpleNames,
llvm::StringMap<std::vector<std::pair<std::string, std::string>>>
compoundNames = {},
const bool includePrivateDeps = false,
const bool hadCompilationError = false);


private:
/// Read a SourceFileDepGraph belonging to \p job from \p buffer
/// and integrate it into the ModuleDepGraph.
/// Used both the first time, and to reload the SourceFileDepGraph.
/// If any changes were observed, indicate same in the return vale.
Changes loadFromBuffer(const driver::Job *, llvm::MemoryBuffer &);
Changes loadFromBuffer(const driver::Job *, llvm::MemoryBuffer &,
DiagnosticEngine &);

/// Integrate a SourceFileDepGraph into the receiver.
/// Integration happens when the driver needs to read SourceFileDepGraph.
Expand All @@ -365,8 +370,9 @@ class ModuleDepGraph {
const SourceFileDepGraphNode *integrand) const;

/// Integrate the \p integrand into the receiver.
/// Return the changed node if any..
NullablePtr<ModuleDepGraphNode>
/// If an illegal value was found, return \c None, otherwise
/// return the changed node if any..
Optional<NullablePtr<ModuleDepGraphNode>>
integrateSourceFileDepGraphNode(const SourceFileDepGraph &g,
const SourceFileDepGraphNode *integrand,
const PreexistingNodeIfAny preexistingMatch,
Expand All @@ -392,6 +398,12 @@ class ModuleDepGraph {
/// After importing a provides node from the frontend, record its
/// dependencies.
/// Return true if moduleUseNode picks up a new external-dependency
///
/// \param g The source file graph being integrated into the module graph
/// \param sourceFileUseNode The source file node just integrated, which may
/// also be a use (i.e. a "depends", a declaration used by something else)
/// \param moduleUseNode The module file node corresponding to the \c
/// sourceFileUseNode
bool recordWhatUseDependsUpon(const SourceFileDepGraph &g,
const SourceFileDepGraphNode *sourceFileUseNode,
ModuleDepGraphNode *moduleUseNode);
Expand Down Expand Up @@ -486,17 +498,8 @@ class ModuleDepGraph {

bool haveAnyNodesBeenTraversedIn(const driver::Job *) const;

/// Given a "cascading" job, that is a job whose dependents must be recompiled
/// when this job is recompiled, Compute two sets of jobs:
/// 1. Return value (via visited) is the set of jobs needing recompilation
/// after this one, and
/// 2. Jobs not previously known to need dependencies reexamined after they
/// are recompiled.
///
/// Returns jobs to be run because of changes to any/ever node in the
/// argument. Only return jobs marked that were previously unmarked, assuming
/// previously marked jobs are already scheduled.
/// TODO: rewrite above comment
/// Find all jobs (possibly including the argument) requiring recompilation
/// assuming that every entity in \p jobToBeRecompiled has changed.
std::vector<const driver::Job *>
findJobsToRecompileWhenWholeJobChanges(const driver::Job *jobToBeRecompiled);

Expand Down
53 changes: 53 additions & 0 deletions lib/AST/FineGrainedDependencies.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,15 @@
#include "swift/AST/FineGrainedDependencies.h"

// may not all be needed
#include "swift/AST/DiagnosticEngine.h"
#include "swift/AST/DiagnosticsCommon.h"
#include "swift/AST/DiagnosticsFrontend.h"
#include "swift/AST/FileSystem.h"
#include "swift/Basic/FileSystem.h"
#include "swift/Basic/LLVM.h"
#include "swift/Demangling/Demangle.h"
#include "swift/Frontend/FrontendOptions.h"

#include "llvm/ADT/MapVector.h"
#include "llvm/ADT/SetVector.h"
#include "llvm/ADT/SmallVector.h"
Expand All @@ -34,6 +39,28 @@
using namespace swift;
using namespace fine_grained_dependencies;

//==============================================================================
// MARK: Emitting and reading SourceFileDepGraph
//==============================================================================

Optional<SourceFileDepGraph> SourceFileDepGraph::loadFromPath(StringRef path) {
auto bufferOrError = llvm::MemoryBuffer::getFile(path);
if (!bufferOrError)
return None;
return loadFromBuffer(*bufferOrError.get());
}

Optional<SourceFileDepGraph>
SourceFileDepGraph::loadFromBuffer(llvm::MemoryBuffer &buffer) {
SourceFileDepGraph fg;
llvm::yaml::Input yamlReader(llvm::MemoryBufferRef(buffer), nullptr);
yamlReader >> fg;
if (yamlReader.error())
return None;
// return fg; compiles for Mac but not Linux, because it cannot be copied.
return Optional<SourceFileDepGraph>(std::move(fg));
}

//==============================================================================
// MARK: SourceFileDepGraph access
//==============================================================================
Expand Down Expand Up @@ -229,6 +256,23 @@ raw_ostream &fine_grained_dependencies::operator<<(raw_ostream &out,
bool DependencyKey::verify() const {
assert((getKind() != NodeKind::externalDepend || isInterface()) &&
"All external dependencies must be interfaces.");
switch (getKind()) {
case NodeKind::topLevel:
case NodeKind::dynamicLookup:
case NodeKind::externalDepend:
case NodeKind::sourceFileProvide:
assert(context.empty() && !name.empty() && "Must only have a name");
break;
case NodeKind::nominal:
case NodeKind::potentialMember:
assert(!context.empty() && name.empty() && "Must only have a context");
break;
case NodeKind::member:
assert(!context.empty() && !name.empty() && "Must have both");
break;
case NodeKind::kindCount:
llvm_unreachable("impossible");
}
return true;
}

Expand Down Expand Up @@ -300,6 +344,15 @@ void SourceFileDepGraph::verifySame(const SourceFileDepGraph &other) const {
#endif
}

void SourceFileDepGraph::emitDotFile(StringRef outputPath,
DiagnosticEngine &diags) {
std::string dotFileName = outputPath.str() + ".dot";
withOutputFile(diags, dotFileName, [&](llvm::raw_pwrite_stream &out) {
DotFileEmitter<SourceFileDepGraph>(out, *this, false, false).emit();
return false;
});
}

//==============================================================================
// MARK: SourceFileDepGraph YAML reading & writing
//==============================================================================
Expand Down
79 changes: 29 additions & 50 deletions lib/AST/SourceFileDepGraphConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,32 +46,6 @@
using namespace swift;
using namespace fine_grained_dependencies;

//==============================================================================
// MARK: Emitting and reading SourceFileDepGraph
//==============================================================================

Optional<SourceFileDepGraph> SourceFileDepGraph::loadFromPath(StringRef path) {
auto bufferOrError = llvm::MemoryBuffer::getFile(path);
if (!bufferOrError)
return None;
return loadFromBuffer(*bufferOrError.get());
}

Optional<SourceFileDepGraph>
SourceFileDepGraph::loadFromBuffer(llvm::MemoryBuffer &buffer) {
SourceFileDepGraph fg;
llvm::yaml::Input yamlReader(llvm::MemoryBufferRef(buffer), nullptr);
yamlReader >> fg;
if (yamlReader.error())
return None;
// return fg; compiles for Mac but not Linux, because it cannot be copied.
return Optional<SourceFileDepGraph>(std::move(fg));
}

//==============================================================================
// MARK: Start of SourceFileDepGraph building, specific to status quo
//==============================================================================

//==============================================================================
// MARK: Helpers for key construction that must be in frontend
//==============================================================================
Expand Down Expand Up @@ -501,7 +475,7 @@ class SourceFileDepGraphConstructor {
/// a flag indicating if the member is private to its enclosing file, and
/// a flag indicating if the dependency cascades.
const std::vector<std::pair<std::tuple<std::string, std::string, bool>, bool>>
memberDepends;
dependsWithContexts;

/// The base name of a class member depended-upon for dynamic lookup, and a
/// cascades flag.
Expand Down Expand Up @@ -534,7 +508,8 @@ class SourceFileDepGraphConstructor {
bool hadCompilationError,
const std::string &interfaceHash,
ArrayRef<std::pair<std::string, bool>> topLevelDepends,
ArrayRef<std::pair<std::tuple<std::string, std::string, bool>, bool>> memberDepends,
ArrayRef<std::pair<std::tuple<std::string, std::string, bool>, bool>>
dependsWithContexts,
ArrayRef<std::pair<std::string, bool>> dynamicLookupDepends,
ArrayRef<std::string> externalDependencies,

Expand All @@ -554,7 +529,7 @@ class SourceFileDepGraphConstructor {

interfaceHash(interfaceHash),
topLevelDepends(topLevelDepends),
memberDepends(memberDepends),
dependsWithContexts(dependsWithContexts),
dynamicLookupDepends(dynamicLookupDepends),
externalDependencies(externalDependencies),

Expand All @@ -569,11 +544,15 @@ class SourceFileDepGraphConstructor {
classMembers(classMembers)
{}

SourceFileDepGraphConstructor static forSourceFile(SourceFile *SF,
const DependencyTracker &depTracker,
StringRef swiftDeps,
const bool includePrivateDeps,
const bool hadCompilationError) {
// clang-format off
static SourceFileDepGraphConstructor
forSourceFile(
SourceFile *SF,
const DependencyTracker &depTracker,
StringRef swiftDeps,
const bool includePrivateDeps,
const bool hadCompilationError) {
// clang-format on

SourceFileDeclFinder declFinder(SF, includePrivateDeps);
std::vector<std::pair<std::string, bool>> topLevelDepends;
Expand All @@ -584,11 +563,11 @@ class SourceFileDepGraphConstructor {
for (const auto &p: SF->getReferencedNameTracker()->getDynamicLookupNames())
dynamicLookupDepends.push_back(std::make_pair(p.getFirst().userFacingName(), p.getSecond()));

std::vector<std::pair<std::tuple<std::string, std::string, bool>, bool>> memberDepends;
std::vector<std::pair<std::tuple<std::string, std::string, bool>, bool>> dependsWithContexts;
for (const auto &p: SF->getReferencedNameTracker()->getUsedMembers()) {
const auto &member = p.getFirst().second;
StringRef emptyOrUserFacingName = member.empty() ? "" : member.userFacingName();
memberDepends.push_back(
dependsWithContexts.push_back(
std::make_pair(
std::make_tuple(
mangleTypeAsContext(p.getFirst().first),
Expand All @@ -604,7 +583,7 @@ class SourceFileDepGraphConstructor {

getInterfaceHash(SF),
topLevelDepends,
memberDepends,
dependsWithContexts,
dynamicLookupDepends,
depTracker.getDependencies(),

Expand Down Expand Up @@ -782,7 +761,7 @@ void SourceFileDepGraphConstructor::addDependencyArcsToGraph() {
// TODO: express the multiple provides and depends streams with variadic
// templates
addAllDependenciesFrom<NodeKind::topLevel>(topLevelDepends);
addAllDependenciesFrom(memberDepends);
addAllDependenciesFrom(dependsWithContexts);
addAllDependenciesFrom<NodeKind::dynamicLookup>(dynamicLookupDepends);
addAllDependenciesFrom(externalDependencies);
}
Expand All @@ -798,7 +777,7 @@ void SourceFileDepGraphConstructor::recordThatThisWholeFileDependsOn(
// Entry point from the Frontend to this whole system
//==============================================================================

bool swift::fine_grained_dependencies::emitReferenceDependencies(
bool fine_grained_dependencies::emitReferenceDependencies(
DiagnosticEngine &diags, SourceFile *const SF,
const DependencyTracker &depTracker, StringRef outputPath,
const bool alsoEmitDotFile) {
Expand All @@ -814,8 +793,9 @@ bool swift::fine_grained_dependencies::emitReferenceDependencies(
// we force the inclusion of private declarations when fingerprints
// are enabled.
const bool includeIntrafileDeps =
SF->getASTContext().LangOpts.FineGrainedDependenciesIncludeIntrafileOnes ||
SF->getASTContext().LangOpts.EnableTypeFingerprints;
SF->getASTContext()
.LangOpts.FineGrainedDependenciesIncludeIntrafileOnes ||
SF->getASTContext().LangOpts.EnableTypeFingerprints;
const bool hadCompilationError = SF->getASTContext().hadError();
auto gc = SourceFileDepGraphConstructor::forSourceFile(
SF, depTracker, outputPath, includeIntrafileDeps, hadCompilationError);
Expand All @@ -832,13 +812,9 @@ bool swift::fine_grained_dependencies::emitReferenceDependencies(
// If path is stdout, cannot read it back, so check for "-"
assert(outputPath == "-" || g.verifyReadsWhatIsWritten(outputPath));

if (alsoEmitDotFile) {
std::string dotFileName = outputPath.str() + ".dot";
withOutputFile(diags, dotFileName, [&](llvm::raw_pwrite_stream &out) {
DotFileEmitter<SourceFileDepGraph>(out, g, false, false).emit();
return false;
});
}
if (alsoEmitDotFile)
g.emitDotFile(outputPath, diags);

return hadError;
}

Expand Down Expand Up @@ -952,7 +928,10 @@ SourceFileDepGraph SourceFileDepGraph::simulateLoad(

// clang-format off
SourceFileDepGraphConstructor c(
swiftDepsFilename, includePrivateDeps, hadCompilationError, interfaceHash,
swiftDepsFilename,
includePrivateDeps,
hadCompilationError,
interfaceHash,
getSimpleDepends(simpleNamesByRDK[dependsTopLevel]),
getCompoundDepends(simpleNamesByRDK[dependsNominal],
compoundNamesByRDK[dependsMember]),
Expand All @@ -961,7 +940,7 @@ SourceFileDepGraph SourceFileDepGraph::simulateLoad(
{}, // precedence groups
{}, // memberOperatorDecls
{}, // operators
getMangledHolderProvides(simpleNamesByRDK[providesNominal]), // topNominals
{}, // topNominals
getBaseNameProvides(simpleNamesByRDK[providesTopLevel]), // topValues
getMangledHolderProvides(simpleNamesByRDK[providesNominal]), // allNominals
getMangledHolderProvides(simpleNamesByRDK[providesNominal]), // potentialMemberHolders
Expand Down
Loading