Merge pull request #23175 from brentdax/theres-a-path-for-everyone

Look for runtime library modules in the SDK, too

Author: beccadax

include/swift/AST/SearchPathOptions.h

@@ -67,11 +67,11 @@ class SearchPathOptions {
   /// Path to search for compiler-relative stdlib dylibs.
   std::string RuntimeLibraryPath;
 
-  /// Path to search for compiler-relative stdlib modules.
-  std::string RuntimeLibraryImportPath;
+  /// Paths to search for stdlib modules. One of these will be compiler-relative.
+  std::vector<std::string> RuntimeLibraryImportPaths;
 
   /// Don't look in for compiler-provided modules.
-  bool SkipRuntimeLibraryImportPath = false;
+  bool SkipRuntimeLibraryImportPaths = false;
 
   /// Return a hash code of any components from these options that should
   /// contribute to a Swift Bridging PCH hash.
@@ -92,7 +92,9 @@ class SearchPathOptions {
       Code = hash_combine(Code, LibraryPath);
     }
     Code = hash_combine(Code, RuntimeResourcePath);
-    Code = hash_combine(Code, RuntimeLibraryImportPath);
+    for (auto RuntimeLibraryImportPath : RuntimeLibraryImportPaths) {
+      Code = hash_combine(Code, RuntimeLibraryImportPath);
+    }
     return Code;
   }
 };

include/swift/Frontend/Frontend.h

@@ -184,9 +184,7 @@ class CompilerInvocation {
 
   void setRuntimeResourcePath(StringRef Path);
 
-  void setSDKPath(const std::string &Path) {
-    SearchPathOpts.SDKPath = Path;
-  }
+  void setSDKPath(const std::string &Path);
 
   StringRef getSDKPath() const {
     return SearchPathOpts.SDKPath;

lib/Frontend/CompilerInvocation.cpp

@@ -42,21 +42,35 @@ void CompilerInvocation::setMainExecutablePath(StringRef Path) {
   setRuntimeResourcePath(LibPath.str());
 }
 
-static void updateRuntimeLibraryPath(SearchPathOptions &SearchPathOpts,
-                                     llvm::Triple &Triple) {
+static void updateRuntimeLibraryPaths(SearchPathOptions &SearchPathOpts,
+                                      llvm::Triple &Triple) {
   llvm::SmallString<128> LibPath(SearchPathOpts.RuntimeResourcePath);
 
   llvm::sys::path::append(LibPath, getPlatformNameForTriple(Triple));
   SearchPathOpts.RuntimeLibraryPath = LibPath.str();
 
+  // Set up the import paths containing the swiftmodules for the libraries in
+  // RuntimeLibraryPath.
+  SearchPathOpts.RuntimeLibraryImportPaths.clear();
+
+  // If this is set, we don't want any runtime import paths.
+  if (SearchPathOpts.SkipRuntimeLibraryImportPaths)
+    return;
+
   if (!Triple.isOSDarwin())
     llvm::sys::path::append(LibPath, swift::getMajorArchitectureName(Triple));
-  SearchPathOpts.RuntimeLibraryImportPath = LibPath.str();
+  SearchPathOpts.RuntimeLibraryImportPaths.push_back(LibPath.str());
+
+  if (!SearchPathOpts.SDKPath.empty()) {
+    LibPath = SearchPathOpts.SDKPath;
+    llvm::sys::path::append(LibPath, "usr", "lib", "swift");
+    SearchPathOpts.RuntimeLibraryImportPaths.push_back(LibPath.str());
+  }
 }
 
 void CompilerInvocation::setRuntimeResourcePath(StringRef Path) {
   SearchPathOpts.RuntimeResourcePath = Path;
-  updateRuntimeLibraryPath(SearchPathOpts, LangOpts.Target);
+  updateRuntimeLibraryPaths(SearchPathOpts, LangOpts.Target);
 }
 
 void CompilerInvocation::setTargetTriple(StringRef Triple) {
@@ -65,7 +79,12 @@ void CompilerInvocation::setTargetTriple(StringRef Triple) {
 
 void CompilerInvocation::setTargetTriple(const llvm::Triple &Triple) {
   LangOpts.setTarget(Triple);
-  updateRuntimeLibraryPath(SearchPathOpts, LangOpts.Target);
+  updateRuntimeLibraryPaths(SearchPathOpts, LangOpts.Target);
+}
+
+void CompilerInvocation::setSDKPath(const std::string &Path) {
+  SearchPathOpts.SDKPath = Path;
+  updateRuntimeLibraryPaths(SearchPathOpts, LangOpts.Target);
 }
 
 SourceFileKind CompilerInvocation::getSourceFileKind() const {
@@ -562,7 +581,7 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts,
   if (const Arg *A = Args.getLastArg(OPT_resource_dir))
     Opts.RuntimeResourcePath = A->getValue();
 
-  Opts.SkipRuntimeLibraryImportPath |= Args.hasArg(OPT_nostdimport);
+  Opts.SkipRuntimeLibraryImportPaths |= Args.hasArg(OPT_nostdimport);
 
   // Opts.RuntimeIncludePath is set by calls to
   // setRuntimeIncludePath() or setMainExecutablePath().
@@ -1279,7 +1298,7 @@ bool CompilerInvocation::parseArgs(
     return true;
   }
 
-  updateRuntimeLibraryPath(SearchPathOpts, LangOpts.Target);
+  updateRuntimeLibraryPaths(SearchPathOpts, LangOpts.Target);
 
   return false;
 }

lib/Serialization/SerializedModuleLoader.cpp

@@ -278,24 +278,35 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
     }
   }
 
-  // If we're not allowed to look in the runtime library import path, stop.
-  if (Ctx.SearchPathOpts.SkipRuntimeLibraryImportPath)
-    return false;
-
-  // Search the runtime import path.
+  // Search the runtime import paths.
   isFramework = false;
-  currPath = Ctx.SearchPathOpts.RuntimeLibraryImportPath;
-  if (Ctx.LangOpts.Target.isOSDarwin()) {
-    // Apple platforms always use architecture-specific files within a
-    // .swiftmodule directory for the stdlib.
-    llvm::sys::path::append(currPath, fileNames.module.str());
-    return findTargetSpecificModuleFiles().getValueOr(false);
+
+  // Apple platforms always use target-specific files within a
+  // .swiftmodule directory for the stdlib; non-Apple platforms
+  // always use single-architecture swiftmodules.
+  // We could move the `if` outside of the `for` instead of inside,
+  // but I/O will be so slow that we won't notice the difference,
+  // so it's not worth the code duplication.
+  bool hasTargetSpecificRuntimeModules = Ctx.LangOpts.Target.isOSDarwin();
+
+  for (auto RuntimeLibraryImportPath :
+       Ctx.SearchPathOpts.RuntimeLibraryImportPaths) {
+    currPath = RuntimeLibraryImportPath;
+    if (hasTargetSpecificRuntimeModules) {
+      llvm::sys::path::append(currPath, fileNames.module.str());
+      if (auto outcome = findTargetSpecificModuleFiles())
+        return *outcome;
+    } else {
+      auto result = findModuleFilesInDirectory(moduleID, currPath,
+                                               fileNames.module.str(),
+                                               fileNames.moduleDoc.str(),
+                                               moduleBuffer, moduleDocBuffer);
+      if (!result)
+        return true;
+    }
   }
-  // Non-Apple platforms always use single-architecture swiftmodules.
-  return !findModuleFilesInDirectory(moduleID, currPath,
-                                     fileNames.module.str(),
-                                     fileNames.moduleDoc.str(),
-                                     moduleBuffer, moduleDocBuffer);
+
+  return false;
 }
 
 static std::pair<StringRef, clang::VersionTuple>

test/Serialization/runtime-import-from-sdk.swift

@@ -0,0 +1,47 @@
+/// Tests the fallback behavior for runtime library import paths. These should
+/// prefer the resource directory, but fall back to the SDK.
+
+// Assumption: We build the standard library with the compiler, so the default
+// resource directory will contain a swiftmodule for the standard library.
+
+// %t/good-sdk contains a loadable standard library.
+// RUN: %empty-directory(%t/good-sdk/usr/lib/swift)
+// RUN: cp -r %platform-module-dir/Swift.swiftmodule %t/good-sdk/usr/lib/swift/Swift.swiftmodule
+
+// %t/bad-sdk contains an invalid standard library that cannot be loaded.
+// RUN: %empty-directory(%t/bad-sdk/usr/lib/swift/Swift.swiftmodule)
+// RUN: touch %t/bad-sdk/usr/lib/swift/Swift.swiftmodule/garbage-garbage-garbage.swiftmodule
+
+// %t/empty-toolchain does not contain a standard library.
+// RUN: %empty-directory(%t/empty-toolchain/usr/lib/swift)
+
+// FIXME: Until we have private imports, we need SwiftShims in the toolchain.
+// RUN: cp -r %test-resource-dir/shims %t/empty-toolchain/usr/lib/swift/shims
+
+// If the compiler's resource directory does not contain a runtime swiftmodule,
+// we should fall back to the SDK.
+
+// RUN: %empty-directory(%t/mcp)
+// RUN: %target-swift-frontend(mock-sdk: -sdk %t/good-sdk) -resource-dir %t/empty-toolchain/usr/lib/swift -module-cache-path %t/mcp -typecheck -verify %s
+
+// If the compiler's resource directory *does* contain a runtime swiftmodule, we
+// should *not* use the one in the SDK. (We assume that the resource directory
+// built with this compiler does contain a standard library.)
+
+// RUN: %empty-directory(%t/mcp)
+// RUN: %target-swift-frontend(mock-sdk: -sdk %t/bad-sdk) -module-cache-path %t/mcp -typecheck -verify %s
+
+// If neither the resource directory nor the SDK contains a runtime swiftmodule,
+// loading should fail. This just proves that we aren't getting runtime imports
+// some other way.
+
+// FIXME: We can't properly test this on a non-Darwin platform because we'll get
+// the same error message for "unloadable standard library" and "no standard
+// library". (SR-10097)
+// REQUIRES: objc_interop
+
+// RUN: %empty-directory(%t/mcp)
+// RUN: not %target-swift-frontend(mock-sdk: -sdk %t/bad-sdk) -resource-dir %t/empty-toolchain/usr/lib/swift -module-cache-path %t/mcp -typecheck %s 2>&1 | %FileCheck %s
+// CHECK: error: could not find module 'Swift' for target '{{.*}}'; found: garbage-garbage-garbage
+
+let x: Int = 1

test/lit.cfg

@@ -310,6 +310,7 @@ else:
     resource_dir_opt = ""
 stdlib_resource_dir_opt = resource_dir_opt
 sourcekitd_framework_dir = config.swift_lib_dir
+config.substitutions.append( ('%test-resource-dir', test_resource_dir) )
 lit_config.note('Using resource dir: ' + test_resource_dir)
 
 # Parse the variant triple.