[ParseableInterface] Distinguish SDK and non-SDK dependencies

This allows the SDK to be relocated without automatically resulting in a
rebuild.

Based on an old patch from Jordan Rose.

include/swift/Serialization/ModuleFormat.h

@@ -52,7 +52,7 @@ const uint16_t SWIFTMODULE_VERSION_MAJOR = 0;
 /// describe what change you made. The content of this comment isn't important;
 /// it just ensures a conflict if two people change the module format.
 /// Don't worry about adhering to the 80-column limit for this line.
-const uint16_t SWIFTMODULE_VERSION_MINOR = 483; // Remove default arg expansion
+const uint16_t SWIFTMODULE_VERSION_MINOR = 484; // SDK-relative dependencies flag
 
 using DeclIDField = BCFixed<31>;
 
@@ -687,6 +687,7 @@ namespace input_block {
     FileSizeField,                 // file size (for validation)
     FileModTimeOrContentHashField, // mtime or content hash (for validation)
     BCFixed<1>,                    // are we reading mtime (0) or hash (1)?
+    BCFixed<1>,                    // SDK-relative?
     BCBlob                         // path
   >;
 }

include/swift/Serialization/SerializationOptions.h

@@ -39,11 +39,14 @@ namespace swift {
     /// appropriate strategy for how to verify if it's up-to-date.
     class FileDependency {
       /// The size of the file on disk, in bytes.
-      uint64_t Size : 63;
+      uint64_t Size : 62;
 
       /// A dependency can be either hash-based or modification-time-based.
       bool IsHashBased : 1;
 
+      /// The dependency path can be absolute or relative to the SDK
+      bool IsSDKRelative : 1;
+
       union {
         /// The last modification time of the file.
         uint64_t ModificationTime;
@@ -56,22 +59,22 @@ namespace swift {
       std::string Path;
 
       FileDependency(uint64_t size, bool isHash, uint64_t hashOrModTime,
-                     StringRef path):
+                     StringRef path, bool isSDKRelative):
-        Size(size), IsHashBased(isHash), ModificationTime(hashOrModTime),
+        Size(size), IsHashBased(isHash), IsSDKRelative(isSDKRelative),
-        Path(path) {}
+        ModificationTime(hashOrModTime), Path(path) {}
     public:
       FileDependency() = delete;
 
       /// Creates a new hash-based file dependency.
       static FileDependency
-      hashBased(StringRef path, uint64_t size, uint64_t hash) {
+      hashBased(StringRef path, bool isSDKRelative, uint64_t size, uint64_t hash) {
-        return FileDependency(size, /*isHash*/true, hash, path);
+        return FileDependency(size, /*isHash*/true, hash, path, isSDKRelative);
       }
 
       /// Creates a new modification time-based file dependency.
       static FileDependency
-      modTimeBased(StringRef path, uint64_t size, uint64_t mtime) {
+      modTimeBased(StringRef path, bool isSDKRelative, uint64_t size, uint64_t mtime) {
-        return FileDependency(size, /*isHash*/false, mtime, path);
+        return FileDependency(size, /*isHash*/false, mtime, path, isSDKRelative);
       }
 
       /// Updates the last-modified time of this dependency.
@@ -94,6 +97,9 @@ namespace swift {
       /// based on content hash.
       bool isHashBased() const { return IsHashBased; }
 
+      /// Determines if this dependency is absolute or relative to the SDK.
+      bool isSDKRelative() const { return IsSDKRelative; }
+
       /// Determines if this dependency is hash-based and should be validated
       /// based on modification time.
       bool isModificationTimeBased() const { return !IsHashBased; }

lib/AST/ModuleLoader.cpp

@@ -34,9 +34,8 @@ void
 DependencyTracker::addDependency(StringRef File, bool IsSystem) {
   // DependencyTracker exposes an interface that (intentionally) does not talk
   // about clang at all, nor about missing deps. It does expose an IsSystem
-  // dimension, though it is presently always false, we accept it and pass it
+  // dimension, which we accept and pass along to the clang DependencyCollector.
-  // along to the clang DependencyCollector in case Swift callers start setting
+  // along to the clang DependencyCollector.
-  // it to true someday.
   clangCollector->maybeAddDependency(File, /*FromModule=*/false,
                                      IsSystem, /*IsModuleFile=*/false,
                                      /*IsMissing=*/false);

lib/Frontend/ParseableInterfaceModuleLoader.cpp

@@ -368,6 +368,8 @@ class swift::ParseableInterfaceBuilder {
   bool collectDepsForSerialization(CompilerInstance &SubInstance,
                                    SmallVectorImpl<FileDependency> &Deps,
                                    bool IsHashBased) {
+    StringRef SDKPath = SubInstance.getASTContext().SearchPathOpts.SDKPath;
+
     auto DTDeps = SubInstance.getDependencyTracker()->getDependencies();
     SmallVector<StringRef, 16> InitialDepNames(DTDeps.begin(), DTDeps.end());
     InitialDepNames.push_back(interfacePath);
@@ -396,17 +398,39 @@ class swift::ParseableInterfaceBuilder {
       if (!Status)
         return true;
 
+      bool IsSDKRelative = false;
+      StringRef DepNameToStore = DepName;
+      if (SDKPath.size() > 1 && DepName.startswith(SDKPath)) {
+        assert(DepName.size() > SDKPath.size() &&
+            "should never depend on a directory");
+        if (llvm::sys::path::is_separator(DepName[SDKPath.size()])) {
+          // Is the DepName something like ${SDKPath}/foo.h"?
+          DepNameToStore = DepName.substr(SDKPath.size() + 1);
+          IsSDKRelative = true;
+        } else if (llvm::sys::path::is_separator(SDKPath.back())) {
+          // Is the DepName something like "${SDKPath}foo.h", where SDKPath
+          // itself contains a trailing slash?
+          DepNameToStore = DepName.substr(SDKPath.size());
+          IsSDKRelative = true;
+        } else {
+          // We have something next to an SDK, like "Foo.sdk.h", that's somehow
+          // become a dependency.
+        }
+      }
+
       if (IsHashBased) {
         auto buf = getDepBuf();
         if (!buf) return true;
         uint64_t hash = xxHash64(buf->getBuffer());
         Deps.push_back(
-          FileDependency::hashBased(DepName, Status->getSize(), hash));
+          FileDependency::hashBased(DepNameToStore, IsSDKRelative,
+                                    Status->getSize(), hash));
       } else {
         uint64_t mtime =
           Status->getLastModificationTime().time_since_epoch().count();
         Deps.push_back(
-          FileDependency::modTimeBased(DepName, Status->getSize(), mtime));
+          FileDependency::modTimeBased(DepNameToStore, IsSDKRelative,
+                                       Status->getSize(), mtime));
       }
 
       if (moduleCachePath.empty())
@@ -681,10 +705,23 @@ class ParseableInterfaceModuleLoaderImpl {
     OutPath.append(OutExt);
   }
 
+  /// Constructs the full path of the dependency \p dep by prepending the SDK
+  /// path if necessary.
+  StringRef getFullDependencyPath(const FileDependency &dep,
+                                  SmallVectorImpl<char> &scratch) const {
+    if (!dep.isSDKRelative())
+      return dep.getPath();
+
+    StringRef SDKPath = ctx.SearchPathOpts.SDKPath;
+    scratch.assign(SDKPath.begin(), SDKPath.end());
+    llvm::sys::path::append(scratch, dep.getPath());
+    return StringRef(scratch.data(), scratch.size());
+  }
+
   // Checks that a dependency read from the cached module is up to date compared
   // to the interface file it represents.
-  bool dependencyIsUpToDate(const FileDependency &dep) {
+  bool dependencyIsUpToDate(const FileDependency &dep, StringRef fullPath) {
-    auto status = getStatusOfDependency(fs, dep.getPath(), interfacePath,
+    auto status = getStatusOfDependency(fs, fullPath, interfacePath,
                                         diags, diagnosticLoc);
     if (!status) return false;
 
@@ -701,7 +738,7 @@ class ParseableInterfaceModuleLoaderImpl {
 
     // Slow path: if the dependency is verified by content hash, check it vs.
     // the hash of the file.
-    auto buf = getBufferOfDependency(fs, dep.getPath(), interfacePath,
+    auto buf = getBufferOfDependency(fs, fullPath, interfacePath,
                                      diags, diagnosticLoc);
     if (!buf) return false;
 
@@ -711,10 +748,12 @@ class ParseableInterfaceModuleLoaderImpl {
   // Check if all the provided file dependencies are up-to-date compared to
   // what's currently on disk.
   bool dependenciesAreUpToDate(ArrayRef<FileDependency> deps) {
+    SmallString<128> SDKRelativeBuffer;
     for (auto &in : deps) {
+      StringRef fullPath = getFullDependencyPath(in, SDKRelativeBuffer);
       if (dependencyTracker)
-        dependencyTracker->addDependency(in.getPath(), /*isSystem*/false);
+        dependencyTracker->addDependency(fullPath, /*isSystem*/in.isSDKRelative());
-      if (!dependencyIsUpToDate(in)) {
+      if (!dependencyIsUpToDate(in, fullPath)) {
         LLVM_DEBUG(llvm::dbgs() << "Dep " << in.getPath()
                    << " is directly out of date\n");
         return false;
@@ -763,9 +802,12 @@ class ParseableInterfaceModuleLoaderImpl {
 
     // Next, check the dependencies in the forwarding file.
     for (auto &dep : fwd.dependencies) {
+      // Forwarding modules expand SDKRelative paths when generated, so are
+      // guaranteed to be absolute.
       deps.push_back(
         FileDependency::modTimeBased(
-          dep.path, dep.size, dep.lastModificationTime));
+          dep.path, /*isSDKRelative=*/false, dep.size,
+          dep.lastModificationTime));
     }
     if (!dependenciesAreUpToDate(deps))
       return false;
@@ -919,8 +961,9 @@ class ParseableInterfaceModuleLoaderImpl {
     addDependency(fwd.underlyingModulePath);
 
     // Add all the dependencies from the prebuilt module.
+    SmallString<128> SDKRelativeBuffer;
     for (auto dep : deps) {
-      addDependency(dep.getPath());
+      addDependency(getFullDependencyPath(dep, SDKRelativeBuffer));
     }
 
     return withOutputFile(diags, outputPath,
@@ -937,6 +980,7 @@ class ParseableInterfaceModuleLoaderImpl {
   /// loading strategy.
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>>
   findOrBuildLoadableModule() {
+
     // Track system dependencies if the parent tracker is set to do so.
     // FIXME: This means -track-system-dependencies isn't honored when the
     // top-level invocation isn't tracking dependencies
@@ -1057,11 +1101,10 @@ bool ParseableInterfaceModuleLoader::buildSwiftModuleFromSwiftInterface(
   bool SerializeDependencyHashes, bool TrackSystemDependencies) {
   ParseableInterfaceBuilder builder(Ctx, InPath, ModuleName,
                                     CacheDir, PrebuiltCacheDir,
-                                    SerializeDependencyHashes);
+                                    SerializeDependencyHashes,
-  // FIXME: We really only want to serialize 'important' dependencies here, and
+                                    TrackSystemDependencies);
-  //        make them relocatable (SDK-relative) if we want to ship the built
+  // FIXME: We really only want to serialize 'important' dependencies here, if
-  //        swiftmodules to another machine. Just track them as absolute paths
+  //        we want to ship the built swiftmodules to another machine.
-  //        for now, so we can test the dependency tracking locally.
   return builder.buildSwiftModule(OutPath, /*shouldSerializeDeps*/true,
                                   /*ModuleBuffer*/nullptr);
 }

lib/Serialization/ModuleFile.cpp

@@ -253,14 +253,16 @@ static bool validateInputBlock(
     switch (kind) {
     case input_block::FILE_DEPENDENCY: {
       bool isHashBased = scratch[2] != 0;
+      bool isSDKRelative = scratch[3] != 0;
+
       if (isHashBased) {
         dependencies.push_back(
           SerializationOptions::FileDependency::hashBased(
-            blobData, scratch[0], scratch[1]));
+            blobData, isSDKRelative, scratch[0], scratch[1]));
       } else {
         dependencies.push_back(
           SerializationOptions::FileDependency::modTimeBased(
-            blobData, scratch[0], scratch[1]));
+            blobData, isSDKRelative, scratch[0], scratch[1]));
       }
       break;
     }

lib/Serialization/Serialization.cpp

@@ -1076,6 +1076,7 @@ void Serializer::writeInputBlock(const SerializationOptions &options) {
                         dep.getSize(),
                         getRawModTimeOrHash(dep),
                         dep.isHashBased(),
+                        dep.isSDKRelative(),
                         dep.getPath());
   }
 

test/ParseableInterface/ModuleCache/Inputs/mock-sdk/ExportedLib.swiftinterface

@@ -0,0 +1,11 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -parse-stdlib -module-name ExportedLib
+
+@_exported import SomeCModule
+
+public struct ExportedInterface {
+  @inlinable public init() {}
+}
+public var testValue: ExportedInterface {
+  @inlinable get { return ExportedInterface() }
+}

test/ParseableInterface/ModuleCache/Inputs/mock-sdk/SdkLib.swiftinterface

@@ -0,0 +1,8 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -parse-stdlib -module-name SdkLib
+
+@_exported import ExportedLib
+
+public var testValue2: ExportedInterface {
+  @inlinable get { return ExportedInterface() }
+}

test/ParseableInterface/ModuleCache/SDKDependencies.swiftinterface

@@ -0,0 +1,123 @@
+// swift-interface-format-version: 1.0
+// swift-module-flags: -module-name SDKDependencies
+
+// RUN: %empty-directory(%t)
+
+// 1) Build a prebuilt cache for our SDK
+//
+// RUN: mkdir %t/MCP %t/prebuilt-cache %t/my-sdk
+// RUN: cp -r %S/Inputs/mock-sdk/. %t/my-sdk
+// RUN: %target-swift-frontend -build-module-from-parseable-interface -serialize-parseable-module-interface-dependency-hashes -sdk %t/my-sdk -prebuilt-module-cache-path %t/prebuilt-cache -I %t/my-sdk -module-cache-path %t/MCP -o %t/prebuilt-cache/ExportedLib.swiftmodule -track-system-dependencies -module-name ExportedLib %t/my-sdk/ExportedLib.swiftinterface
+// RUN: %target-swift-frontend -build-module-from-parseable-interface -serialize-parseable-module-interface-dependency-hashes -sdk %t/my-sdk -prebuilt-module-cache-path %t/prebuilt-cache -I %t/my-sdk -module-cache-path %t/MCP -o %t/prebuilt-cache/SdkLib.swiftmodule -track-system-dependencies -module-name SdkLib %t/my-sdk/SdkLib.swiftinterface
+//
+// RUN: %empty-directory(%t/MCP)
+// RUN: echo '1: PASSED'
+
+
+// 2) Baseline check: Make sure we use the interface when not passing the prebuilt module cache path
+//
+// RUN: %target-swift-frontend -typecheck -I %t/my-sdk -sdk %t/my-sdk -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
+//
+// Check SdkLib and ExportedLib are in the module cache
+// RUN: test -f %t/MCP/ExportedLib-*.swiftmodule
+// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
+//
+// Check they are *not* forwarding modules
+// RUN: head -n 1 %t/MCP/SdkLib-*.swiftmodule | not grep -e '---'
+// RUN: head -n 1 %t/MCP/ExportedLib-*.swiftmodule | not grep -e '---'
+//
+// RUN: %empty-directory(%t/MCP)
+// RUN: echo '2: PASSED'
+
+
+// 3) Baseline check: Make sure we use the the prebuilt module cache when using the SDK it was built with
+//
+// RUN: %target-swift-frontend -typecheck -I %t/my-sdk -sdk %t/my-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
+//
+// Check SdkLib and ExportedLib are in the module cache
+// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
+// RUN: test -f %t/MCP/ExportedLib-*.swiftmodule
+//
+// Check they *are* forwarding modules
+// RUN: head -n 1 %t/MCP/SdkLib-*.swiftmodule | grep -e '---'
+// RUN: head -n 1 %t/MCP/ExportedLib-*.swiftmodule | grep -e '---'
+//
+// Check they contain the expected dependencies
+// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=EXLIB
+// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefixes=EXLIB,SDKLIB
+//
+// EXLIB-DAG: /prebuilt-cache/ExportedLib.swiftmodule
+// EXLIB-DAG: /my-sdk/usr/include/module.modulemap
+// EXLIB-DAG: /my-sdk/usr/include/SomeCModule.h
+// EXLIB-DAG: /my-sdk/ExportedLib.swiftinterface
+// SDKLIB-DAG: /prebuilt-cache/SdkLib.swiftmodule
+// SDKLIB-DAG: /my-sdk/SdkLib.swiftinterface
+//
+// RUN: %empty-directory(%t/MCP)
+// RUN: echo '3: PASSED'
+
+
+// 4) Move the SDK without changing its contents
+//
+// RUN: mv %t/my-sdk %t/my-new-sdk
+// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
+//
+// Check SdkLib and ExportedLib are in the module cache
+// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
+// RUN: test -f %t/MCP/ExportedLib-*.swiftmodule
+//
+// Check they are still both forwarding modules
+// RUN: head -n 1 %t/MCP/SdkLib-*.swiftmodule | grep -e '---'
+// RUN: head -n 1 %t/MCP/ExportedLib-*.swiftmodule | grep -e '---'
+//
+// Check they contain the expected dependencies
+// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=NEW-EXLIB
+// RUN: cat %t/MCP/SdkLib-*.swiftmodule | %FileCheck %s -check-prefixes=NEW-EXLIB,NEW-SDKLIB
+//
+// NEW-EXLIB-DAG: /prebuilt-cache/ExportedLib.swiftmodule
+// NEW-EXLIB-DAG: /my-new-sdk/usr/include/module.modulemap
+// NEW-EXLIB-DAG: /my-new-sdk/usr/include/SomeCModule.h
+// NEW-EXLIB-DAG: /my-new-sdk/ExportedLib.swiftinterface
+// NEW-SDKLIB-DAG: /prebuilt-cache/SdkLib.swiftmodule
+// NEW-SDKLIB-DAG: /my-new-sdk/SdkLib.swiftinterface
+//
+// RUN: %empty-directory(%t/MCP)
+// RUN: echo '4: PASSED'
+
+
+// 5) Now change the SDK's content and check it no longer uses the prebuilt modules
+//
+// RUN: echo "// size change" >> %t/my-new-sdk/SdkLib.swiftinterface
+// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
+//
+// Check SDKLib and ExportedLib are in the module cache
+// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
+// RUN: test -f %t/MCP/ExportedLib-*.swiftmodule
+//
+// Check ExportedLib is still a forwarding module and SdkLib is not
+// RUN: head -n 1 %t/MCP/ExportedLib-*.swiftmodule | grep -e '---'
+// RUN: head -n 1 %t/MCP/SdkLib-*.swiftmodule | not grep -e '---'
+//
+// Check ExportedLib still contains the same dependencies
+// RUN: cat %t/MCP/ExportedLib-*.swiftmodule | %FileCheck %s -check-prefix=NEW-EXLIB
+//
+// RUN: %empty-directory(%t/MCP)
+//
+// RUN: echo "// size change" >> %t/my-new-sdk/usr/include/SomeCModule.h
+// RUN: %target-swift-frontend -typecheck -I %t/my-new-sdk -sdk %t/my-new-sdk -prebuilt-module-cache-path %t/prebuilt-cache -module-cache-path %t/MCP -emit-dependencies-path %t/dummy.d -track-system-dependencies %s
+//
+// Check SDKLib and ExportLib are in the module cache
+// RUN: test -f %t/MCP/SdkLib-*.swiftmodule
+// RUN: test -f %t/MCP/ExportedLib-*.swiftmodule
+//
+// Check neither are forwarding modules
+// RUN: head -n 1 %t/MCP/SdkLib-*.swiftmodule | not grep -e '---'
+// RUN: head -n 1 %t/MCP/ExportedLib-*.swiftmodule | not grep -e '---'
+//
+// RUN: echo '5: PASSED'
+
+import SdkLib
+
+func foo() -> ExportedInterface {
+	return x > 3 ? testValue : testValue2
+}