Merge pull request #64693 from xymus/access-level-import-testable

xymus · web-flow · commit 1fea84e915c3 · 2023-03-29T16:39:51.000-07:00
[Serialization] Load non-public transitive dependencies on `@testable` imports
diff --git a/include/swift/AST/FileUnit.h b/include/swift/AST/FileUnit.h
@@ -294,6 +294,9 @@ class FileUnit : public DeclContext, public ASTAllocated<FileUnit> {
   virtual void
   collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const {}
 
+  /// Load extra dependencies of this module to satisfy a testable import.
+  virtual void loadDependenciesForTestable(SourceLoc diagLoc) const {}
+
   /// Returns the path of the file or directory that defines the module
   /// represented by this \c FileUnit, or empty string if there is none.
   /// Cross-import overlay specifiers are found relative to this path.
diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h
@@ -454,6 +454,8 @@ class SerializedASTFile final : public LoadedFile {
   virtual void
   collectLinkLibraries(ModuleDecl::LinkLibraryCallback callback) const override;
 
+  virtual void loadDependenciesForTestable(SourceLoc diagLoc) const override;
+
   Identifier getDiscriminatorForPrivateValue(const ValueDecl *D) const override;
 
   virtual StringRef getFilename() const override;
diff --git a/include/swift/Serialization/Validation.h b/include/swift/Serialization/Validation.h
@@ -251,6 +251,23 @@ void diagnoseSerializedASTLoadFailure(
     StringRef moduleBufferID, StringRef moduleDocBufferID,
     ModuleFile *loadedModuleFile, Identifier ModuleName);
 
+/// Emit diagnostics explaining a failure to load a serialized AST,
+/// this version only supports diagnostics relevant to transitive dependencies:
+/// missing dependency, missing underlying module, or circular dependency.
+///
+/// \see diagnoseSerializedASTLoadFailure that supports all diagnostics.
+///
+/// - \p Ctx is an AST context through which any diagnostics are surfaced.
+/// - \p diagLoc is the (possibly invalid) location used in the diagnostics.
+/// - \p status describes the issue with loading the AST. It must not be
+///   Status::Valid.
+/// - \p loadedModuleFile is an invalid loaded module.
+/// - \p ModuleName is the name used to refer to the module in diagnostics.
+/// - \p forTestable indicates if we loaded the AST for a @testable import.
+void diagnoseSerializedASTLoadFailureTransitive(
+    ASTContext &Ctx, SourceLoc diagLoc, const serialization::Status status,
+    ModuleFile *loadedModuleFile, Identifier ModuleName, bool forTestable);
+
 } // end namespace serialization
 } // end namespace swift
 
diff --git a/lib/Sema/ImportResolution.cpp b/lib/Sema/ImportResolution.cpp
@@ -324,6 +324,15 @@ void ImportResolver::bindImport(UnboundImport &&I) {
     return;
   }
 
+  // Load more dependencies for testable imports.
+  if (I.import.options.contains(ImportFlags::Testable)) {
+    SourceLoc diagLoc;
+    if (ID) diagLoc = ID.get()->getStartLoc();
+
+    for (auto file: M->getFiles())
+      file->loadDependenciesForTestable(diagLoc);
+  }
+
   auto topLevelModule = I.getTopLevelModule(M, SF);
 
   I.validateOptions(topLevelModule, SF);
diff --git a/lib/Serialization/ModuleFile.cpp b/lib/Serialization/ModuleFile.cpp
@@ -123,56 +123,19 @@ bool ModuleFile::allowCompilerErrors() const {
   return getContext().LangOpts.AllowModuleWithCompilerErrors;
 }
 
-Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
-                                            bool recoverFromIncompatibility) {
-  PrettyStackTraceModuleFile stackEntry(*this);
-
-  assert(!hasError() && "error already detected; should not call this");
-  assert(!FileContext && "already associated with an AST module");
-  FileContext = file;
-  Status status = Status::Valid;
-
-  ModuleDecl *M = file->getParentModule();
-  // The real (on-disk) name of the module should be checked here as that's the
-  // actually loaded module. In case module aliasing is used when building the main
-  // module, e.g. -module-name MyModule -module-alias Foo=Bar, the loaded module
-  // that maps to 'Foo' is actually Bar.swiftmodule|.swiftinterface (applies to swift
-  // modules only), which is retrieved via M->getRealName(). If no module aliasing is
-  // used, M->getRealName() will return the same value as M->getName(), which is 'Foo'.
-  if (M->getRealName().str() != Core->Name) {
-    return error(Status::NameMismatch);
-  }
-
+Status
+ModuleFile::loadDependenciesForFileContext(const FileUnit *file,
+                                           SourceLoc diagLoc,
+                                           bool forTestable) {
   ASTContext &ctx = getContext();
-
-  llvm::Triple moduleTarget(llvm::Triple::normalize(Core->TargetTriple));
-  if (!areCompatibleArchitectures(moduleTarget, ctx.LangOpts.Target) ||
-      !areCompatibleOSs(moduleTarget, ctx.LangOpts.Target)) {
-    status = Status::TargetIncompatible;
-    if (!recoverFromIncompatibility)
-      return error(status);
-  } else if (ctx.LangOpts.EnableTargetOSChecking && !M->isResilient() &&
-             isTargetTooNew(moduleTarget, ctx.LangOpts.Target)) {
-    status = Status::TargetTooNew;
-    if (!recoverFromIncompatibility)
-      return error(status);
-  }
-
-  StringRef SDKPath = ctx.SearchPathOpts.getSDKPath();
-  if (SDKPath.empty() ||
-      !Core->ModuleInputBuffer->getBufferIdentifier().startswith(SDKPath)) {
-    for (const auto &searchPath : Core->SearchPaths) {
-      ctx.addSearchPath(
-        ctx.SearchPathOpts.SearchPathRemapper.remapPath(searchPath.Path),
-        searchPath.IsFramework,
-        searchPath.IsSystem);
-    }
-  }
-
   auto clangImporter = static_cast<ClangImporter *>(ctx.getClangModuleLoader());
+  ModuleDecl *M = file->getParentModule();
 
   bool missingDependency = false;
   for (auto &dependency : Dependencies) {
+    if (forTestable && dependency.isLoaded())
+      continue;
+
     assert(!dependency.isLoaded() && "already loaded?");
 
     if (dependency.isHeader()) {
@@ -196,7 +159,7 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
     }
 
     ModuleLoadingBehavior transitiveBehavior =
-      getTransitiveLoadingBehavior(dependency);
+      getTransitiveLoadingBehavior(dependency, forTestable);
 
     if (ctx.LangOpts.EnableModuleLoadingRemarks) {
       ctx.Diags.diagnose(diagLoc,
@@ -259,6 +222,59 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
     return error(Status::MissingDependency);
   }
 
+  return Status::Valid;
+}
+
+Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
+                                            bool recoverFromIncompatibility) {
+  PrettyStackTraceModuleFile stackEntry(*this);
+
+  assert(!hasError() && "error already detected; should not call this");
+  assert(!FileContext && "already associated with an AST module");
+  FileContext = file;
+  Status status = Status::Valid;
+
+  ModuleDecl *M = file->getParentModule();
+  // The real (on-disk) name of the module should be checked here as that's the
+  // actually loaded module. In case module aliasing is used when building the main
+  // module, e.g. -module-name MyModule -module-alias Foo=Bar, the loaded module
+  // that maps to 'Foo' is actually Bar.swiftmodule|.swiftinterface (applies to swift
+  // modules only), which is retrieved via M->getRealName(). If no module aliasing is
+  // used, M->getRealName() will return the same value as M->getName(), which is 'Foo'.
+  if (M->getRealName().str() != Core->Name) {
+    return error(Status::NameMismatch);
+  }
+
+  ASTContext &ctx = getContext();
+
+  llvm::Triple moduleTarget(llvm::Triple::normalize(Core->TargetTriple));
+  if (!areCompatibleArchitectures(moduleTarget, ctx.LangOpts.Target) ||
+      !areCompatibleOSs(moduleTarget, ctx.LangOpts.Target)) {
+    status = Status::TargetIncompatible;
+    if (!recoverFromIncompatibility)
+      return error(status);
+  } else if (ctx.LangOpts.EnableTargetOSChecking && !M->isResilient() &&
+             isTargetTooNew(moduleTarget, ctx.LangOpts.Target)) {
+    status = Status::TargetTooNew;
+    if (!recoverFromIncompatibility)
+      return error(status);
+  }
+
+  StringRef SDKPath = ctx.SearchPathOpts.getSDKPath();
+  if (SDKPath.empty() ||
+      !Core->ModuleInputBuffer->getBufferIdentifier().startswith(SDKPath)) {
+    for (const auto &searchPath : Core->SearchPaths) {
+      ctx.addSearchPath(
+        ctx.SearchPathOpts.SearchPathRemapper.remapPath(searchPath.Path),
+        searchPath.IsFramework,
+        searchPath.IsSystem);
+    }
+  }
+
+  Status res = loadDependenciesForFileContext(file, diagLoc,
+                                            /*forTestable=*/false);
+  if (res != Status::Valid) return res;
+
   if (Core->Bits.HasEntryPoint) {
     FileContext->getParentModule()->registerEntryPointFile(FileContext,
                                                            SourceLoc(),
@@ -269,7 +285,8 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
 }
 
 ModuleLoadingBehavior
-ModuleFile::getTransitiveLoadingBehavior(const Dependency &dependency) const {
+ModuleFile::getTransitiveLoadingBehavior(const Dependency &dependency,
+    bool forTestable) const {
   ASTContext &ctx = getContext();
   ModuleDecl *mod = FileContext->getParentModule();
 
@@ -280,7 +297,8 @@ ModuleFile::getTransitiveLoadingBehavior(const Dependency &dependency) const {
   return Core->getTransitiveLoadingBehavior(dependency.Core,
                                             ctx.LangOpts.DebuggerSupport,
                                             isPartialModule,
-                                            ctx.LangOpts.PackageName);
+                                            ctx.LangOpts.PackageName,
+                                            forTestable);
 }
 
 bool ModuleFile::mayHaveDiagnosticsPointingAtBuffer() const {
diff --git a/lib/Serialization/ModuleFile.h b/lib/Serialization/ModuleFile.h
@@ -649,9 +649,23 @@ class ModuleFile
   Status associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
                                   bool recoverFromIncompatibility);
 
+  /// Load dependencies of this module.
+  ///
+  /// \param file The FileUnit that represents this file's place in the AST.
+  /// \param diagLoc A location used for diagnostics that occur during loading.
+  /// This does not include diagnostics about \e this file failing to load,
+  /// but rather other things that might be imported as part of bringing the
+  /// file into the AST.
+  ///
+  /// \returns any error that occurred during loading dependencies.
+  Status
+  loadDependenciesForFileContext(const FileUnit *file, SourceLoc diagLoc,
+                               bool forTestable);
+
   /// How should \p dependency be loaded for a transitive import via \c this?
   ModuleLoadingBehavior
-  getTransitiveLoadingBehavior(const Dependency &dependency) const;
+  getTransitiveLoadingBehavior(const Dependency &dependency,
+                               bool forTestable) const;
 
   /// Returns `true` if there is a buffer that might contain source code where
   /// other parts of the compiler could have emitted diagnostics, to indicate
diff --git a/lib/Serialization/ModuleFileSharedCore.cpp b/lib/Serialization/ModuleFileSharedCore.cpp
@@ -1688,7 +1688,8 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
                                           const Dependency &dependency,
                                           bool debuggerMode,
                                           bool isPartialModule,
-                                          StringRef packageName) const {
+                                          StringRef packageName,
+                                          bool forTestable) const {
   if (isPartialModule) {
     // Keep the merge-module behavior for legacy support. In that case
     // we load all transitive dependencies from partial modules and
@@ -1717,7 +1718,7 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
     // Non-public imports are similar to implementation-only, the module
     // loading behavior differs on loading those dependencies
     // on testable imports.
-    if (isTestable() || !moduleIsResilient) {
+    if (forTestable || !moduleIsResilient) {
       return ModuleLoadingBehavior::Required;
     } else if (debuggerMode) {
       return ModuleLoadingBehavior::Optional;
@@ -1730,6 +1731,7 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
     // Package dependencies are usually loaded only for import from the same
     // package.
     if ((!packageName.empty() && packageName == getModulePackageName()) ||
+        forTestable ||
         !moduleIsResilient) {
       return ModuleLoadingBehavior::Required;
     } else if (debuggerMode) {
diff --git a/lib/Serialization/ModuleFileSharedCore.h b/lib/Serialization/ModuleFileSharedCore.h
@@ -614,11 +614,17 @@ class ModuleFileSharedCore {
   ///
   /// If \p packageName is set, transitive package dependencies are loaded if
   /// loaded from the same package.
+  ///
+  /// If \p forTestable, get the desired loading behavior for a @testable
+  /// import. Reports non-public dependencies as required for a testable
+  /// client so it can access internal details, which in turn can reference
+  /// those non-public dependencies.
   ModuleLoadingBehavior
   getTransitiveLoadingBehavior(const Dependency &dependency,
                                bool debuggerMode,
                                bool isPartialModule,
-                               StringRef packageName) const;
+                               StringRef packageName,
+                               bool forTestable) const;
 };
 
 template <typename T, typename RawData>
diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp
diff --git a/test/Serialization/access-level-import-dependencies.swift b/test/Serialization/access-level-import-dependencies.swift
