Merge pull request #29722 from davidungar/unit-test-users-v2

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

Author: David Ungar

include/swift/AST/FineGrainedDependencies.h

@@ -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 {
@@ -883,6 +884,8 @@ class SourceFileDepGraph {
 
   bool verifySequenceNumber() const;
 
+  void emitDotFile(StringRef outputPath, DiagnosticEngine &diags);
+
 private:
   void addNode(SourceFileDepGraphNode *n) {
     n->setSequenceNumber(allNodes.size());

include/swift/Driver/FineGrainedDependencyDriverGraph.h

@@ -123,6 +123,8 @@ class ModuleDepGraphNode : public DepGraphNode {
     return DepGraphNode::humanReadableName(where);
   }
 
+  void dump(raw_ostream &) const;
+
   SWIFT_DEBUG_DUMP;
 };
 
@@ -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
@@ -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.
@@ -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,
@@ -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);
@@ -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);
 

lib/AST/FineGrainedDependencies.cpp

@@ -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"
@@ -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
 //==============================================================================
@@ -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;
 }
 
@@ -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
 //==============================================================================

lib/AST/SourceFileDepGraphConstructor.cpp

@@ -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
 //==============================================================================
@@ -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.
@@ -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,
 
@@ -554,7 +529,7 @@ class SourceFileDepGraphConstructor {
 
     interfaceHash(interfaceHash),
     topLevelDepends(topLevelDepends),
-    memberDepends(memberDepends),
+    dependsWithContexts(dependsWithContexts),
     dynamicLookupDepends(dynamicLookupDepends),
     externalDependencies(externalDependencies),
 
@@ -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;
@@ -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),
@@ -604,7 +583,7 @@ class SourceFileDepGraphConstructor {
 
       getInterfaceHash(SF),
       topLevelDepends,
-      memberDepends,
+      dependsWithContexts,
       dynamicLookupDepends,
       depTracker.getDependencies(),
 
@@ -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);
 }
@@ -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) {
@@ -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);
@@ -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;
 }
 
@@ -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]),
@@ -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