Merge pull request swiftlang#33687 from artemcm/DepScannerOrder

[DependencyScanner] Change the scanner order to resolve placeholders first

Author: artemcm

include/swift/Serialization/ModuleDependencyScanner.h

@@ -19,6 +19,16 @@ namespace swift {
     /// for the purpose of determining dependencies, but does not attempt to
     /// load the module files.
     class ModuleDependencyScanner : public SerializedModuleLoaderBase {
+    public:
+      enum ScannerKind {
+        MDS_plain,
+        MDS_placeholder
+      };
+
+    private:
+      /// The kind of scanner this is (LLVM-style RTTI)
+      const ScannerKind kind;
+
       /// The module we're scanning dependencies of.
       Identifier moduleName;
 
@@ -36,10 +46,11 @@ namespace swift {
       ModuleDependencyScanner(
           ASTContext &ctx, ModuleLoadingMode LoadMode, Identifier moduleName,
           InterfaceSubContextDelegate &astDelegate,
-          ModuleDependenciesKind dependencyKind = ModuleDependenciesKind::Swift)
+          ModuleDependenciesKind dependencyKind = ModuleDependenciesKind::Swift,
+          ScannerKind kind = MDS_plain)
           : SerializedModuleLoaderBase(ctx, nullptr, LoadMode,
                                        /*IgnoreSwiftSourceInfoFile=*/true),
-            moduleName(moduleName), astDelegate(astDelegate),
+            kind(kind), moduleName(moduleName), astDelegate(astDelegate),
             dependencyKind(dependencyKind) {}
 
       std::error_code findModuleFilesInDirectory(
@@ -55,6 +66,11 @@ namespace swift {
           SmallVectorImpl<Identifier> &names) const override {
         llvm_unreachable("Not used");
       }
+
+      ScannerKind getKind() const { return kind; }
+      static bool classof(const ModuleDependencyScanner *MDS) {
+        return MDS->getKind() == MDS_plain;
+      }
     };
 
     /// A ModuleLoader that loads placeholder dependency module stubs specified in
@@ -90,7 +106,8 @@ namespace swift {
                                     StringRef PlaceholderDependencyModuleMap,
                                     InterfaceSubContextDelegate &astDelegate)
           : ModuleDependencyScanner(ctx, LoadMode, moduleName, astDelegate,
-                                    ModuleDependenciesKind::SwiftPlaceholder) {
+                                    ModuleDependenciesKind::SwiftPlaceholder,
+                                    MDS_placeholder) {
 
         // FIXME: Find a better place for this map to live, to avoid
         // doing the parsing on every module.
@@ -106,5 +123,9 @@ namespace swift {
           std::unique_ptr<llvm::MemoryBuffer> *ModuleDocBuffer,
           std::unique_ptr<llvm::MemoryBuffer> *ModuleSourceInfoBuffer,
                                                  bool IsFramework) override;
+
+      static bool classof(const ModuleDependencyScanner *MDS) {
+        return MDS->getKind() == MDS_placeholder;
+      }
     };
 }

lib/Serialization/ModuleDependencyScanner.cpp

@@ -176,13 +176,19 @@ Optional<ModuleDependencies> SerializedModuleLoaderBase::getModuleDependencies(
   auto moduleId = Ctx.getIdentifier(moduleName);
   // Instantiate dependency scanning "loaders".
   SmallVector<std::unique_ptr<ModuleDependencyScanner>, 2> scanners;
-  scanners.push_back(std::make_unique<ModuleDependencyScanner>(
-      Ctx, LoadMode, moduleId, delegate));
+  // Placeholder dependencies must be resolved first, to prevent the ModuleDependencyScanner
+  // from first discovering artifacts of a previous build. Such artifacts are captured
+  // as compiledModuleCandidates in the dependency graph of the placeholder dependency module
+  // itself.
   scanners.push_back(std::make_unique<PlaceholderSwiftModuleScanner>(
       Ctx, LoadMode, moduleId, Ctx.SearchPathOpts.PlaceholderDependencyModuleMap,
       delegate));
+  scanners.push_back(std::make_unique<ModuleDependencyScanner>(
+      Ctx, LoadMode, moduleId, delegate));
 
   // Check whether there is a module with this name that we can import.
+  assert(isa<PlaceholderSwiftModuleScanner>(scanners[0].get()) &&
+         "Expected PlaceholderSwiftModuleScanner as the first dependency scanner loader.");
   for (auto &scanner : scanners) {
     if (scanner->canImportModule({moduleId, SourceLoc()})) {
       // Record the dependencies.

test/ScanDependencies/can_import_placeholder.swift

@@ -8,16 +8,6 @@
 // RUN: echo "\"docPath\": \"%/t/inputs/SomeExternalModule.swiftdoc\"," >> %/t/inputs/map.json
 // RUN: echo "\"sourceInfoPath\": \"%/t/inputs/SomeExternalModule.swiftsourceinfo\"," >> %/t/inputs/map.json
 // RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
-// RUN: echo "}," >> %/t/inputs/map.json
-// RUN: echo "{" >> %/t/inputs/map.json
-// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
-// RUN: echo "\"modulePath\": \"%/stdlib_module\"," >> %/t/inputs/map.json
-// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
-// RUN: echo "}," >> %/t/inputs/map.json
-// RUN: echo "{" >> %/t/inputs/map.json
-// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
-// RUN: echo "\"modulePath\": \"%/ononesupport_module\"," >> %/t/inputs/map.json
-// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
 // RUN: echo "}]" >> %/t/inputs/map.json
 
 // RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -placeholder-dependency-module-map-file %t/inputs/map.json -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4

test/ScanDependencies/module_deps_external.swift

@@ -8,16 +8,6 @@
 // RUN: echo "\"docPath\": \"%/t/inputs/SomeExternalModule.swiftdoc\"," >> %/t/inputs/map.json
 // RUN: echo "\"sourceInfoPath\": \"%/t/inputs/SomeExternalModule.swiftsourceinfo\"," >> %/t/inputs/map.json
 // RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
-// RUN: echo "}," >> %/t/inputs/map.json
-// RUN: echo "{" >> %/t/inputs/map.json
-// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
-// RUN: echo "\"modulePath\": \"%/stdlib_module\"," >> %/t/inputs/map.json
-// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
-// RUN: echo "}," >> %/t/inputs/map.json
-// RUN: echo "{" >> %/t/inputs/map.json
-// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
-// RUN: echo "\"modulePath\": \"%/ononesupport_module\"," >> %/t/inputs/map.json
-// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
 // RUN: echo "}]" >> %/t/inputs/map.json
 
 // RUN: %target-swift-frontend -scan-dependencies -module-cache-path %t/clang-module-cache %s -placeholder-dependency-module-map-file %t/inputs/map.json -o %t/deps.json -I %S/Inputs/CHeaders -I %S/Inputs/Swift -emit-dependencies -emit-dependencies-path %t/deps.d -import-objc-header %S/Inputs/CHeaders/Bridging.h -swift-version 4