Skip to content

[5.9][Serialization] Load non-public transitive dependencies on @testable imports #64794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions include/swift/AST/DiagnosticsSema.def
Original file line number Diff line number Diff line change
Expand Up @@ -1041,6 +1041,11 @@ REMARK(module_loaded,none,
"%select{| (overlay for a clang dependency)}1"
"; source: '%2', loaded: '%3'",
(Identifier, unsigned, StringRef, StringRef))

REMARK(transitive_dependency_behavior,none,
"%1 has %select{a required|an optional|an ignored}2 "
"transitive dependency on '%0'",
(StringRef, Identifier, unsigned))

REMARK(explicit_interface_build_skipped,none,
"Skipped rebuilding module at %0 - up-to-date",
Expand Down
3 changes: 3 additions & 0 deletions include/swift/AST/FileUnit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 2 additions & 0 deletions include/swift/Serialization/SerializedModuleLoader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
17 changes: 17 additions & 0 deletions include/swift/Serialization/Validation.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions lib/Sema/ImportResolution.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
123 changes: 75 additions & 48 deletions lib/Serialization/ModuleFile.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "ModuleFormat.h"
#include "swift/Serialization/SerializationOptions.h"
#include "swift/Subsystems.h"
#include "swift/AST/DiagnosticsSema.h"
#include "swift/AST/ASTContext.h"
#include "swift/AST/ASTMangler.h"
#include "swift/AST/GenericSignature.h"
Expand Down Expand Up @@ -122,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()) {
Expand All @@ -195,7 +159,15 @@ Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
}

ModuleLoadingBehavior transitiveBehavior =
getTransitiveLoadingBehavior(dependency);
getTransitiveLoadingBehavior(dependency, forTestable);

if (ctx.LangOpts.EnableModuleLoadingRemarks) {
ctx.Diags.diagnose(diagLoc,
diag::transitive_dependency_behavior,
dependency.Core.getPrettyPrintedPath(),
M->getName(),
unsigned(transitiveBehavior));
}

// Skip this dependency?
if (transitiveBehavior == ModuleLoadingBehavior::Ignored)
Expand Down Expand Up @@ -250,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(),
Expand All @@ -260,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();

Expand All @@ -271,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 {
Expand Down
16 changes: 15 additions & 1 deletion lib/Serialization/ModuleFile.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions lib/Serialization/ModuleFileSharedCore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand All @@ -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) {
Expand Down
8 changes: 7 additions & 1 deletion lib/Serialization/ModuleFileSharedCore.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
Loading