Merge pull request swiftlang#32601 from hamishknight/not-yet-over-and-out

Author: hamishknight

include/swift/AST/ModuleLoader.h

@@ -60,13 +60,22 @@ enum class Bridgeability : unsigned {
   Full
 };
 
+/// Specifies which dependencies the intermodule dependency tracker records.
+enum class IntermoduleDepTrackingMode {
+  /// Records both system and non-system dependencies.
+  IncludeSystem,
+
+  /// Records only non-system dependencies.
+  ExcludeSystem,
+};
+
 /// Records dependencies on files outside of the current module;
 /// implemented in terms of a wrapped clang::DependencyCollector.
 class DependencyTracker {
   std::shared_ptr<clang::DependencyCollector> clangCollector;
 public:
   explicit DependencyTracker(
-      bool TrackSystemDeps,
+      IntermoduleDepTrackingMode Mode,
       std::shared_ptr<llvm::FileCollector> FileCollector = {});
 
   /// Adds a file as a dependency.

include/swift/ClangImporter/ClangImporter.h

@@ -175,7 +175,7 @@ class ClangImporter final : public ClangModuleLoader {
   /// Create a new clang::DependencyCollector customized to
   /// ClangImporter's specific uses.
   static std::shared_ptr<clang::DependencyCollector>
-  createDependencyCollector(bool TrackSystemDeps,
+  createDependencyCollector(IntermoduleDepTrackingMode Mode,
                             std::shared_ptr<llvm::FileCollector> FileCollector);
 
   /// Append visible module names to \p names. Note that names are possibly

include/swift/Frontend/Frontend.h

@@ -498,10 +498,6 @@ class CompilerInstance {
     Diagnostics.removeConsumer(*DC);
   }
 
-  void createDependencyTracker(bool TrackSystemDeps) {
-    assert(!Context && "must be called before setup()");
-    DepTracker = std::make_unique<DependencyTracker>(TrackSystemDeps);
-  }
   DependencyTracker *getDependencyTracker() { return DepTracker.get(); }
   const DependencyTracker *getDependencyTracker() const { return DepTracker.get(); }
 
@@ -582,6 +578,7 @@ class CompilerInstance {
   bool setUpASTContextIfNeeded();
   void setupStatsReporter();
   void setupDiagnosticVerifierIfNeeded();
+  void setupDependencyTrackerIfNeeded();
   Optional<unsigned> setUpCodeCompletionBuffer();
 
   /// Set up all state in the CompilerInstance to process the given input file.

include/swift/Frontend/FrontendOptions.h

@@ -27,7 +27,7 @@ namespace llvm {
 }
 
 namespace swift {
-
+enum class IntermoduleDepTrackingMode;
 
 /// Options for controlling the behavior of the frontend.
 class FrontendOptions {
@@ -237,9 +237,10 @@ class FrontendOptions {
   /// See the \ref SILOptions.EmitSortedSIL flag.
   bool EmitSortedSIL = false;
 
-  /// Indicates whether the dependency tracker should track system
-  /// dependencies as well.
-  bool TrackSystemDeps = false;
+  /// Specifies the collection mode for the intermodule dependency tracker.
+  /// Note that if set, the dependency tracker will be enabled even if no
+  /// output path is configured.
+  Optional<IntermoduleDepTrackingMode> IntermoduleDependencyTracking;
 
   /// Should we serialize the hashes of dependencies (vs. the modification
   /// times) when compiling a module interface?
@@ -315,6 +316,9 @@ class FrontendOptions {
     return ImplicitImportModuleNames;
   }
 
+  /// Whether we're configured to track system intermodule dependencies.
+  bool shouldTrackSystemDependencies() const;
+
 private:
   static bool canActionEmitDependencies(ActionType);
   static bool canActionEmitReferenceDependencies(ActionType);

lib/AST/ModuleLoader.cpp

@@ -30,14 +30,15 @@ class FileCollector;
 namespace swift {
 
 DependencyTracker::DependencyTracker(
-    bool TrackSystemDeps, std::shared_ptr<llvm::FileCollector> FileCollector)
+    IntermoduleDepTrackingMode Mode,
+    std::shared_ptr<llvm::FileCollector> FileCollector)
     // NB: The ClangImporter believes it's responsible for the construction of
     // this instance, and it static_cast<>s the instance pointer to its own
     // subclass based on that belief. If you change this to be some other
     // instance, you will need to change ClangImporter's code to handle the
     // difference.
-    : clangCollector(ClangImporter::createDependencyCollector(TrackSystemDeps,
-                                                              FileCollector)) {}
+    : clangCollector(
+          ClangImporter::createDependencyCollector(Mode, FileCollector)) {}
 
 void
 DependencyTracker::addDependency(StringRef File, bool IsSystem) {

lib/ClangImporter/ClangImporter.cpp

@@ -328,12 +328,13 @@ class ClangImporterDependencyCollector : public clang::DependencyCollector
   /// The FileCollector is used by LLDB to generate reproducers. It's not used
   /// by Swift to track dependencies.
   std::shared_ptr<llvm::FileCollector> FileCollector;
-  const bool TrackSystemDeps;
+  const IntermoduleDepTrackingMode Mode;
 
 public:
   ClangImporterDependencyCollector(
-      bool TrackSystemDeps, std::shared_ptr<llvm::FileCollector> FileCollector)
-      : FileCollector(FileCollector), TrackSystemDeps(TrackSystemDeps) {}
+      IntermoduleDepTrackingMode Mode,
+      std::shared_ptr<llvm::FileCollector> FileCollector)
+      : FileCollector(FileCollector), Mode(Mode) {}
 
   void excludePath(StringRef filename) {
     ExcludedPaths.insert(filename);
@@ -345,7 +346,9 @@ class ClangImporterDependencyCollector : public clang::DependencyCollector
             || Filename == ImporterImpl::bridgingHeaderBufferName);
   }
 
-  bool needSystemDependencies() override { return TrackSystemDeps; }
+  bool needSystemDependencies() override {
+    return Mode == IntermoduleDepTrackingMode::IncludeSystem;
+  }
 
   bool sawDependency(StringRef Filename, bool FromClangModule,
                      bool IsSystem, bool IsClangModuleFile,
@@ -375,8 +378,9 @@ class ClangImporterDependencyCollector : public clang::DependencyCollector
 
 std::shared_ptr<clang::DependencyCollector>
 ClangImporter::createDependencyCollector(
-    bool TrackSystemDeps, std::shared_ptr<llvm::FileCollector> FileCollector) {
-  return std::make_shared<ClangImporterDependencyCollector>(TrackSystemDeps,
+    IntermoduleDepTrackingMode Mode,
+    std::shared_ptr<llvm::FileCollector> FileCollector) {
+  return std::make_shared<ClangImporterDependencyCollector>(Mode,
                                                             FileCollector);
 }
 

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -81,14 +81,19 @@ bool ArgsToFrontendOptionsConverter::convert(
 
   Opts.EnableImplicitDynamic |= Args.hasArg(OPT_enable_implicit_dynamic);
 
-  Opts.TrackSystemDeps |= Args.hasArg(OPT_track_system_dependencies);
+  if (Args.hasArg(OPT_track_system_dependencies)) {
+    Opts.IntermoduleDependencyTracking =
+        IntermoduleDepTrackingMode::IncludeSystem;
+  }
 
   Opts.DisableImplicitModules |= Args.hasArg(OPT_disable_implicit_swift_modules);
 
   // Always track system dependencies when scanning dependencies.
   if (const Arg *ModeArg = Args.getLastArg(OPT_modes_Group)) {
-    if (ModeArg->getOption().matches(OPT_scan_dependencies))
-      Opts.TrackSystemDeps = true;
+    if (ModeArg->getOption().matches(OPT_scan_dependencies)) {
+      Opts.IntermoduleDependencyTracking =
+          IntermoduleDepTrackingMode::IncludeSystem;
+    }
   }
 
   Opts.SerializeModuleInterfaceDependencyHashes |=

lib/Frontend/Frontend.cpp

@@ -288,9 +288,36 @@ void CompilerInstance::setupDiagnosticVerifierIfNeeded() {
   }
 }
 
+void CompilerInstance::setupDependencyTrackerIfNeeded() {
+  assert(!Context && "Must be called before the ASTContext is created");
+
+  const auto &Invocation = getInvocation();
+  const auto &opts = Invocation.getFrontendOptions();
+
+  // Note that we may track dependencies even when we don't need to write them
+  // directly; in particular, -track-system-dependencies affects how module
+  // interfaces get loaded, and so we need to be consistently tracking system
+  // dependencies throughout the compiler.
+  auto collectionMode = opts.IntermoduleDependencyTracking;
+  if (!collectionMode) {
+    // If we have an output path specified, but no other tracking options,
+    // default to non-system dependency tracking.
+    if (opts.InputsAndOutputs.hasDependencyTrackerPath() ||
+        !opts.IndexStorePath.empty()) {
+      collectionMode = IntermoduleDepTrackingMode::ExcludeSystem;
+    }
+  }
+  if (!collectionMode)
+    return;
+
+  DepTracker = std::make_unique<DependencyTracker>(*collectionMode);
+}
+
 bool CompilerInstance::setup(const CompilerInvocation &Invok) {
   Invocation = Invok;
 
+  setupDependencyTrackerIfNeeded();
+
   // If initializing the overlay file system fails there's no sense in
   // continuing because the compiler will read the wrong files.
   if (setUpVirtualFileSystemOverlays())
@@ -685,8 +712,8 @@ bool CompilerInvocation::shouldImportSwiftONoneSupport() const {
   // This optimization is disabled by -track-system-dependencies to preserve
   // the explicit dependency.
   const auto &options = getFrontendOptions();
-  return options.TrackSystemDeps
-      || FrontendOptions::doesActionGenerateSIL(options.RequestedAction);
+  return options.shouldTrackSystemDependencies() ||
+         FrontendOptions::doesActionGenerateSIL(options.RequestedAction);
 }
 
 ImplicitImportInfo CompilerInstance::getImplicitImportInfo() const {

lib/Frontend/FrontendOptions.cpp

@@ -13,6 +13,7 @@
 #include "swift/Frontend/FrontendOptions.h"
 
 #include "swift/AST/DiagnosticsFrontend.h"
+#include "swift/AST/ModuleLoader.h"
 #include "swift/Option/Options.h"
 #include "swift/Parse/Lexer.h"
 #include "swift/Strings.h"
@@ -592,3 +593,8 @@ const PrimarySpecificPaths &
 FrontendOptions::getPrimarySpecificPathsForPrimary(StringRef filename) const {
   return InputsAndOutputs.getPrimarySpecificPathsForPrimary(filename);
 }
+
+bool FrontendOptions::shouldTrackSystemDependencies() const {
+  return IntermoduleDependencyTracking ==
+         IntermoduleDepTrackingMode::IncludeSystem;
+}

lib/Frontend/ModuleInterfaceLoader.cpp

@@ -1126,9 +1126,14 @@ InterfaceSubContextDelegateImpl::InterfaceSubContextDelegateImpl(
     GenericArgs.push_back("-prebuilt-module-cache-path");
     GenericArgs.push_back(prebuiltCachePath);
   }
-  subInvocation.getFrontendOptions().TrackSystemDeps = trackSystemDependencies;
   if (trackSystemDependencies) {
+    subInvocation.getFrontendOptions().IntermoduleDependencyTracking =
+        IntermoduleDepTrackingMode::IncludeSystem;
     GenericArgs.push_back("-track-system-dependencies");
+  } else {
+    // Always track at least the non-system dependencies for interface building.
+    subInvocation.getFrontendOptions().IntermoduleDependencyTracking =
+        IntermoduleDepTrackingMode::ExcludeSystem;
   }
   if (LoaderOpts.disableImplicitSwiftModule) {
     subInvocation.getFrontendOptions().DisableImplicitModules = true;
@@ -1252,7 +1257,7 @@ InterfaceSubContextDelegateImpl::getCacheHash(StringRef useInterfacePath) {
 
       // Whether or not we're tracking system dependencies affects the
       // invalidation behavior of this cache item.
-      subInvocation.getFrontendOptions().TrackSystemDeps);
+      subInvocation.getFrontendOptions().shouldTrackSystemDependencies());
 
   return llvm::APInt(64, H).toString(36, /*Signed=*/false);
 }
@@ -1330,8 +1335,6 @@ bool InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleN
 
   ForwardingDiagnosticConsumer FDC(Diags);
   subInstance.addDiagnosticConsumer(&FDC);
-  subInstance.createDependencyTracker(subInvocation.getFrontendOptions()
-    .TrackSystemDeps);
   if (subInstance.setup(subInvocation)) {
     return true;
   }