Stage In Flags To Fall Back To Manual Tracking

Request-based incremental dependencies are enabled by default. For the time being, add a flag that will turn them off and switch back to manual dependency tracking.

Author: CodaFi

include/swift/AST/SourceFile.h

@@ -480,8 +480,19 @@ class SourceFile final : public FileUnit {
     return RequestReferencedNames ? RequestReferencedNames.getPointer() : nullptr;
   }
 
+  /// Creates and installs the referenced name trackers in this source file.
+  ///
+  /// This entrypoint must be called before incremental compilation can proceed,
+  /// else reference dependencies will not be registered.
   void createReferencedNameTracker();
 
+  /// Retrieves the name tracker instance corresponding to
+  /// \c EnableRequestBasedIncrementalDependencies
+  ///
+  /// If incremental dependencies tracking is not enabled or \c createReferencedNameTracker()
+  /// has not been invoked on this source file, the result is \c nullptr.
+  const ReferencedNameTracker *getConfiguredReferencedNameTracker() const;
+
   /// The buffer ID for the file that was imported, or None if there
   /// is no associated buffer.
   Optional<unsigned> getBufferID() const {

include/swift/Basic/LangOptions.h

@@ -334,6 +334,9 @@ namespace swift {
     /// Enable verification when every SubstitutionMap is constructed.
     bool VerifyAllSubstitutionMaps = false;
 
+    /// If set to \c false, fall back to the legacy manual reference name tracking code.
+    bool EnableRequestBasedIncrementalDependencies = true;
+
     /// Sets the target we are building for and updates platform conditions
     /// to match.
     ///

include/swift/Frontend/DiagnosticVerifier.h

@@ -28,6 +28,7 @@ class SourceManager;
 class SourceFile;
 
 // MARK: - DependencyVerifier
+
 bool verifyDependencies(SourceManager &SM, const DependencyTracker &DT,
                         ArrayRef<FileUnit *> SFs);
 bool verifyDependencies(SourceManager &SM, const DependencyTracker &DT,
@@ -99,6 +100,7 @@ class DiagnosticVerifier : public DiagnosticConsumer {
 
   void printRemainingDiagnostics() const;
 };
-}
+
+} // end namespace swift
 
 #endif

include/swift/Option/Options.td

@@ -1033,6 +1033,16 @@ def disable_parser_lookup : Flag<["-"], "disable-parser-lookup">,
   Flags<[FrontendOption]>,
   HelpText<"Disable parser lookup & use ast scope lookup only (experimental)">;
 
+def enable_request_based_incremental_dependencies : Flag<["-"],
+  "enable-request-based-incremental-dependencies">,
+  Flags<[FrontendOption]>,
+  HelpText<"Enable request-based name tracking">;
+
+def disable_request_based_incremental_dependencies : Flag<["-"],
+  "disable-request-based-incremental-dependencies">,
+  Flags<[FrontendOption]>,
+  HelpText<"Disable request-based name tracking">;
+
 def index_file : Flag<["-"], "index-file">,
   HelpText<"Produce index data for a source file">, ModeOpt,
   Flags<[NoInteractiveOption, DoesNotAffectIncrementalBuild]>;

lib/AST/FrontendSourceFileDepGraphFactory.cpp

@@ -282,7 +282,8 @@ DependencyKey DependencyKey::createDependedUponKey(StringRef mangledHolderName,
 
 bool fine_grained_dependencies::emitReferenceDependencies(
     DiagnosticEngine &diags, SourceFile *const SF,
-    const DependencyTracker &depTracker, StringRef outputPath,
+    const DependencyTracker &depTracker,
+    StringRef outputPath,
     const bool alsoEmitDotFile) {
 
   // Before writing to the dependencies file path, preserve any previous file
@@ -292,7 +293,7 @@ bool fine_grained_dependencies::emitReferenceDependencies(
 
   SourceFileDepGraph g = FrontendSourceFileDepGraphFactory(
                              SF, outputPath, depTracker, alsoEmitDotFile)
-                             .construct();
+                              .construct();
 
   const bool hadError =
       withOutputFile(diags, outputPath, [&](llvm::raw_pwrite_stream &out) {
@@ -563,7 +564,7 @@ void FrontendSourceFileDepGraphFactory::addAllUsedDecls() {
       DependencyKey::createKeyForWholeSourceFile(DeclAspect::implementation,
                                                  swiftDeps);
 
-  SF->getReferencedNameTracker()->enumerateAllUses(
+  SF->getConfiguredReferencedNameTracker()->enumerateAllUses(
       includePrivateDeps, depTracker,
       [&](const fine_grained_dependencies::NodeKind kind, StringRef context,
           StringRef name, const bool isCascadingUse) {

lib/AST/Module.cpp

@@ -2565,7 +2565,18 @@ void SourceFile::setTypeRefinementContext(TypeRefinementContext *Root) {
 
 void SourceFile::createReferencedNameTracker() {
   assert(!ReferencedNames && "This file already has a name tracker.");
+  assert(!RequestReferencedNames && "This file already has a name tracker.");
   ReferencedNames.emplace(ReferencedNameTracker());
+  RequestReferencedNames.emplace(ReferencedNameTracker());
+}
+
+const ReferencedNameTracker *
+SourceFile::getConfiguredReferencedNameTracker() const {
+  if (getASTContext().LangOpts.EnableRequestBasedIncrementalDependencies) {
+    return getRequestBasedReferencedNameTracker();
+  } else {
+    return getReferencedNameTracker();
+  }
 }
 
 ArrayRef<OpaqueTypeDecl *> SourceFile::getOpaqueReturnTypeDecls() {

lib/Frontend/CompilerInvocation.cpp

@@ -402,6 +402,10 @@ static bool ParseLangArgs(LangOptions &Opts, ArgList &Args,
   }
 
   Opts.DisableParserLookup |= Args.hasArg(OPT_disable_parser_lookup);
+  Opts.EnableRequestBasedIncrementalDependencies =
+      Args.hasFlag(OPT_enable_request_based_incremental_dependencies,
+                   OPT_disable_request_based_incremental_dependencies,
+                   Opts.EnableRequestBasedIncrementalDependencies);
   Opts.EnableASTScopeLookup =
       Args.hasFlag(options::OPT_enable_astscope_lookup,
                    options::OPT_disable_astscope_lookup, Opts.EnableASTScopeLookup) ||

lib/Frontend/DependencyVerifier.cpp

@@ -427,7 +427,7 @@ bool DependencyVerifier::parseExpectations(
 
 bool DependencyVerifier::constructObligations(const SourceFile *SF,
                                               ObligationMap &Obligations) {
-  auto *tracker = SF->getReferencedNameTracker();
+  auto *tracker = SF->getConfiguredReferencedNameTracker();
   assert(tracker && "Constructed source file without referenced name tracker!");
 
   auto &Ctx = SF->getASTContext();
@@ -490,7 +490,7 @@ bool DependencyVerifier::constructObligations(const SourceFile *SF,
 bool DependencyVerifier::verifyObligations(
     const SourceFile *SF, const std::vector<Expectation> &ExpectedDependencies,
     ObligationMap &OM, llvm::StringMap<Expectation> &NegativeExpectations) {
-  auto *tracker = SF->getReferencedNameTracker();
+  auto *tracker = SF->getConfiguredReferencedNameTracker();
   assert(tracker && "Constructed source file without referenced name tracker!");
   auto &diags = SF->getASTContext().Diags;
   for (auto &expectation : ExpectedDependencies) {

lib/FrontendTool/FrontendTool.cpp

@@ -686,7 +686,7 @@ static void countStatsPostSema(UnifiedStatsReporter &Stats,
   }
 
   for (auto SF : Instance.getPrimarySourceFiles()) {
-    if (auto *R = SF->getReferencedNameTracker()) {
+    if (auto *R = SF->getConfiguredReferencedNameTracker()) {
       C.NumReferencedTopLevelNames += R->getTopLevelNames().size();
       C.NumReferencedDynamicNames += R->getDynamicLookupNames().size();
       C.NumReferencedMemberNames += R->getUsedMembers().size();
@@ -941,16 +941,18 @@ static void emitReferenceDependenciesForAllPrimaryInputsIfNeeded(
         Invocation.getReferenceDependenciesFilePathForPrimary(
             SF->getFilename());
     if (!referenceDependenciesFilePath.empty()) {
-      if (Invocation.getLangOptions().EnableFineGrainedDependencies)
+      auto LangOpts = Invocation.getLangOptions();
+      if (LangOpts.EnableFineGrainedDependencies) {
         (void)fine_grained_dependencies::emitReferenceDependencies(
             Instance.getASTContext().Diags, SF,
-            *Instance.getDependencyTracker(), referenceDependenciesFilePath,
-            Invocation.getLangOptions()
-                .EmitFineGrainedDependencySourcefileDotFiles);
-      else
+            *Instance.getDependencyTracker(),
+            referenceDependenciesFilePath,
+            LangOpts.EmitFineGrainedDependencySourcefileDotFiles);
+      } else {
         (void)emitReferenceDependencies(Instance.getASTContext().Diags, SF,
                                         *Instance.getDependencyTracker(),
                                         referenceDependenciesFilePath);
+      }
     }
   }
 }

lib/FrontendTool/ReferenceDependencies.cpp

@@ -61,7 +61,8 @@ class ReferenceDependenciesEmitter {
   ///
   /// \return true on error
   static bool emit(DiagnosticEngine &diags, SourceFile *SF,
-                   const DependencyTracker &depTracker, StringRef outputPath);
+                   const DependencyTracker &depTracker,
+                   StringRef outputPath);
 
   /// Emit the dependencies.
   static void emit(SourceFile *SF, const DependencyTracker &depTracker,
@@ -488,12 +489,12 @@ void DependsEmitter::emit(const SourceFile *SF,
 }
 
 void DependsEmitter::emit() const {
-  const ReferencedNameTracker *const tracker = SF->getReferencedNameTracker();
-  assert(tracker && "Cannot emit reference dependencies without a tracker");
+  const auto *nameTracker = SF->getConfiguredReferencedNameTracker();
+  assert(nameTracker && "Cannot emit reference dependencies without a tracker");
 
-  emitTopLevelNames(tracker);
+  emitTopLevelNames(nameTracker);
 
-  auto &memberLookupTable = tracker->getUsedMembers();
+  auto &memberLookupTable = nameTracker->getUsedMembers();
   std::vector<MemberTableEntryTy> sortedMembers{
     memberLookupTable.begin(), memberLookupTable.end()
   };
@@ -519,7 +520,7 @@ void DependsEmitter::emit() const {
 
   emitMembers(sortedMembers);
   emitNominalTypes(sortedMembers);
-  emitDynamicLookup(tracker);
+  emitDynamicLookup(nameTracker);
   emitExternal(depTracker);
 }
 

test/NameBinding/reference-dependencies-consistency-manual.swift

@@ -0,0 +1,29 @@
+// Some types, such as StringLiteralType, used to be cached in the TypeChecker.
+// Consequently, the second primary file (in batch mode) to use that type would
+// hit in the cache and no dependency would be recorded.
+// This test ensures that this bug stays fixed.
+//
+// RUN: %empty-directory(%t)
+// RUN: %empty-directory(%t/request-based)
+// RUN: %empty-directory(%t/manual)
+//
+// Create two identical inputs, each needing StringLiteralType:
+//
+// RUN: echo 'fileprivate var v: String { return "\(x)" }; fileprivate let x = "a"' >%t/1.swift
+// RUN: echo 'fileprivate var v: String { return "\(x)" }; fileprivate let x = "a"' >%t/2.swift
+//
+// RUN:  %target-swift-frontend -enable-fine-grained-dependencies -typecheck -primary-file %t/1.swift -primary-file %t/2.swift -emit-reference-dependencies-path %t/request-based/1.swiftdeps -emit-reference-dependencies-path %t/request-based/2.swiftdeps
+// RUN:  %target-swift-frontend -enable-fine-grained-dependencies -typecheck -primary-file %t/1.swift -primary-file %t/2.swift -emit-reference-dependencies-path %t/manual/1.swiftdeps -emit-reference-dependencies-path %t/manual/2.swiftdeps -disable-request-based-incremental-dependencies
+//
+// Sequence numbers can vary
+// RUN: sed -e 's/[0-9][0-9]*/N/g' -e 's/N, //g' -e '/^ *$/d' <%t/request-based/1.swiftdeps | sort >%t/request-based/1-processed.swiftdeps
+// RUN: sed -e 's/[0-9][0-9]*/N/g' -e 's/N, //g' -e '/^ *$/d' <%t/request-based/2.swiftdeps | sort >%t/request-based/2-processed.swiftdeps
+// RUN: sed -e 's/[0-9][0-9]*/N/g' -e 's/N, //g' -e '/^ *$/d' <%t/manual/1.swiftdeps | sort >%t/manual/1-processed.swiftdeps
+// RUN: sed -e 's/[0-9][0-9]*/N/g' -e 's/N, //g' -e '/^ *$/d' <%t/manual/2.swiftdeps | sort >%t/manual/2-processed.swiftdeps
+
+// Check that manual matches manual and request-based matches request-based.
+// Unfortunately, the remaining two cross-configuration tests will not pass
+// because request-based dependencies capture strictly more dependency edges
+// than the manual name tracker.
+// RUN: cmp -s %t/request-based/1-processed.swiftdeps %t/request-based/2-processed.swiftdeps
+// RUN: cmp -s %t/manual/1-processed.swiftdeps %t/manual/2-processed.swiftdeps