Skip to content

Commit 77fde76

Browse files
committed
Ignore in-package transitive dependencies when building from non-package textual interface
This change ensures that when loading some module dependency 'Bar' which has a package-only dependency on 'Foo', only the following clients attempt to resolve/load 'Foo': - Source compilation with package-name equal to that of 'Bar'. - Textual interface compilation of a *'package'* interface with package-name equal to that of 'Bar'. Ensuring that the following kinds of clients do not attempt to resolve/load 'Foo': - Source compilation with package-name different to that of 'Bar' - Textual interface compilation of a public or private interface, regardless of package name. This fixes the behavior where previously compilation of a Swift textual interface dependency 'X' from its public or private interface, with an interface-specified package-name, from a client without a matching package-name, resulted in a lookup of package-only dependencies of modules loaded into 'X'. This behavior is invalid if we are not building from the package textual interface, becuase the module dependency graph is defined by the package name of the source client, not individual module dependency package name. i.e. In-package module dependencies are resolved/loaded only if the parent source compile matches the package name. Resolves rdar://139979180
1 parent 0234f7a commit 77fde76

8 files changed

+93
-5
lines changed

include/swift/AST/SearchPathOptions.h

+9
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,15 @@ class SearchPathOptions {
567567
/// dependencies in the scanner itself.
568568
bool ScannerModuleValidation = false;
569569

570+
/// Whether this compilation should attempt to resolve in-package
571+
/// imports of its module dependencies.
572+
///
573+
/// Source compilation and 'package' textual interface compilation both
574+
/// require that package-only imports of module dependencies be resolved.
575+
/// Otherwise, compilation of non-package textual interfaces, even if
576+
/// "in-package", must not require package-only module dependencies.
577+
bool ResolveInPackageModuleDependencies = false;
578+
570579
/// Return all module search paths that (non-recursively) contain a file whose
571580
/// name is in \p Filenames.
572581
SmallVector<const ModuleSearchPath *, 4>

lib/Frontend/CompilerInvocation.cpp

+12-1
Original file line numberDiff line numberDiff line change
@@ -2131,6 +2131,7 @@ static bool validateSwiftModuleFileArgumentAndAdd(const std::string &swiftModule
21312131
static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
21322132
DiagnosticEngine &Diags,
21332133
const CASOptions &CASOpts,
2134+
const FrontendOptions &FrontendOpts,
21342135
StringRef workingDirectory) {
21352136
using namespace options;
21362137
namespace path = llvm::sys::path;
@@ -2299,6 +2300,16 @@ static bool ParseSearchPathArgs(SearchPathOptions &Opts, ArgList &Args,
22992300
Opts.ScannerModuleValidation |= Args.hasFlag(OPT_scanner_module_validation,
23002301
OPT_no_scanner_module_validation,
23012302
CASOpts.EnableCaching);
2303+
bool buildingFromInterface =
2304+
FrontendOptions::doesActionBuildModuleFromInterface(
2305+
FrontendOpts.RequestedAction);
2306+
auto firstInputPath =
2307+
FrontendOpts.InputsAndOutputs.hasInputs()
2308+
? FrontendOpts.InputsAndOutputs.getFilenameOfFirstInput()
2309+
: "";
2310+
Opts.ResolveInPackageModuleDependencies |=
2311+
!buildingFromInterface ||
2312+
StringRef(firstInputPath).ends_with(".package.swiftinterface");
23022313

23032314
std::optional<std::string> forceModuleLoadingMode;
23042315
if (auto *A = Args.getLastArg(OPT_module_load_mode))
@@ -3759,7 +3770,7 @@ bool CompilerInvocation::parseArgs(
37593770
ParseSymbolGraphArgs(SymbolGraphOpts, ParsedArgs, Diags, LangOpts);
37603771

37613772
if (ParseSearchPathArgs(SearchPathOpts, ParsedArgs, Diags,
3762-
CASOpts, workingDirectory)) {
3773+
CASOpts, FrontendOpts, workingDirectory)) {
37633774
return true;
37643775
}
37653776

lib/Serialization/ModuleFile.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,8 @@ ModuleFile::getTransitiveLoadingBehavior(const Dependency &dependency,
314314

315315
return Core->getTransitiveLoadingBehavior(
316316
dependency.Core, ctx.LangOpts.ImportNonPublicDependencies,
317-
isPartialModule, ctx.LangOpts.PackageName, forTestable);
317+
isPartialModule, ctx.LangOpts.PackageName,
318+
ctx.SearchPathOpts.ResolveInPackageModuleDependencies, forTestable);
318319
}
319320

320321
bool ModuleFile::mayHaveDiagnosticsPointingAtBuffer() const {
@@ -1418,4 +1419,4 @@ StringRef SerializedASTFile::getPublicModuleName() const {
14181419

14191420
llvm::VersionTuple SerializedASTFile::getSwiftInterfaceCompilerVersion() const {
14201421
return File.getSwiftInterfaceCompilerVersion();
1421-
}
1422+
}

lib/Serialization/ModuleFileSharedCore.cpp

+4
Original file line numberDiff line numberDiff line change
@@ -1853,6 +1853,7 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
18531853
bool importNonPublicDependencies,
18541854
bool isPartialModule,
18551855
StringRef packageName,
1856+
bool resolveInPackageModuleDependencies,
18561857
bool forTestable) const {
18571858
if (isPartialModule) {
18581859
// Keep the merge-module behavior for legacy support. In that case
@@ -1892,6 +1893,9 @@ ModuleFileSharedCore::getTransitiveLoadingBehavior(
18921893
}
18931894

18941895
if (dependency.isPackageOnly()) {
1896+
if (!resolveInPackageModuleDependencies)
1897+
return ModuleLoadingBehavior::Ignored;
1898+
18951899
// Package dependencies are usually loaded only for import from the same
18961900
// package.
18971901
if ((!packageName.empty() && packageName == getModulePackageName()) ||

lib/Serialization/ModuleFileSharedCore.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -681,7 +681,8 @@ class ModuleFileSharedCore {
681681
/// those non-public dependencies.
682682
ModuleLoadingBehavior getTransitiveLoadingBehavior(
683683
const Dependency &dependency, bool importNonPublicDependencies,
684-
bool isPartialModule, StringRef packageName, bool forTestable) const;
684+
bool isPartialModule, StringRef packageName,
685+
bool resolveInPackageModuleDependencies, bool forTestable) const;
685686
};
686687

687688
template <typename T, typename RawData>

lib/Serialization/SerializedModuleLoader.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,9 @@ SerializedModuleLoaderBase::getImportsOfModule(
455455
loadedModuleFile.getTransitiveLoadingBehavior(
456456
dependency,
457457
/*importPrivateDependencies*/ false,
458-
/*isPartialModule*/ false, packageName, isTestableImport);
458+
/*isPartialModule*/ false, packageName,
459+
/*resolveInPackageModuleDependencies */ true,
460+
isTestableImport);
459461
if (dependencyTransitiveBehavior > transitiveBehavior)
460462
continue;
461463

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: %empty-directory(%t/clang-module-cache)
3+
// RUN: %empty-directory(%t/Modules)
4+
// RUN: %empty-directory(%t/PackageModules)
5+
// RUN: %empty-directory(%t/PackageModules/Foo.swiftmodule)
6+
// RUN: %empty-directory(%t/Modules/Bar.swiftmodule)
7+
// RUN: %empty-directory(%t/TextualInterfaces)
8+
// RUN: %empty-directory(%t/TextualInterfaces/Bar.swiftmodule)
9+
// RUN: split-file %s %t
10+
11+
// Step 1: Build Foo Swift binary module
12+
// RUN: %target-swift-frontend -emit-module %t/Foo.swift -emit-module-path %t/PackageModules/Foo.swiftmodule/%target-swiftmodule-name -module-name Foo
13+
14+
// Step 2: Build Bar Swift binary module and textual interface with a package-only import
15+
// RUN: %target-swift-frontend -emit-module %t/Bar.swift -emit-module-path %t/Modules/Bar.swiftmodule/%target-swiftmodule-name -module-name Bar -enable-library-evolution -emit-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-swiftinterface-name -emit-private-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-private-swiftinterface-name -emit-package-module-interface-path %t/TextualInterfaces/Bar.swiftmodule/%target-package-swiftinterface-name -I %t/PackageModules/ -package-name BarTest
16+
17+
// Step 3: Now that Bar has been built, remove package-only dependency 'Foo' so that any clients of 'Bar' fail to build if they search for it
18+
// RUN: rm -rf %t/PackageModules/*
19+
20+
21+
// Test 1: Build a textual interface client which imports Bar and is in Bar's package but not being built from a package interface, therefore it must not import Bar's package-only dependencies
22+
// RUN: %target-swift-frontend -compile-module-from-interface -explicit-interface-module-build %t/Client.swiftinterface -o %t/Modules/Client.swiftmodule -module-name Client -I %t/Modules/ -package-name BarTest
23+
24+
// Test 2: Build a textual interface client which imports Bar and is in Bar's package and is being built from a package interface, therefore it must import Bar's package-only dependencies
25+
// RUN: not %target-swift-frontend -compile-module-from-interface -explicit-interface-module-build %t/Client.package.swiftinterface -o %t/Modules/Client.package.swiftmodule -module-name Client -I %t/Modules/ -package-name BarTest &> %t/error.txt
26+
// RUN %FileCheck --check-prefix=CHECK-MISSING-FOO %s < %t/error.txt
27+
28+
// Test 3: Build a source client which imports Bar but is not in Bar's package, therefore it must not import Bar's package-only dependencies
29+
// RUN: %target-swift-frontend -emit-module %t/Client.swift -emit-module-path %t/Modules/SourceClient.swiftmodule/%target-swiftmodule-name -module-name Client -I %t/Modules/
30+
31+
// Test 4: Build a source client which imports Bar but and is in Bar's package, therefore it must import Bar's package-only dependencies
32+
// RUN: not %target-swift-frontend -emit-module %t/Client.swift -emit-module-path %t/Modules/SourceClient.swiftmodule/%target-swiftmodule-name -module-name Client -I %t/Modules/ -package-name BarTest &> %t/source_error.txt
33+
// RUN %FileCheck --check-prefix=CHECK-MISSING-FOO %s < %t/source_error.txt
34+
35+
// CHECK-MISSING-FOO: error: missing required module 'Foo'
36+
37+
//--- Foo.swift
38+
public func foo() {}
39+
40+
//--- Bar.swift
41+
package import Foo
42+
43+
//--- Client.swiftinterface
44+
// swift-interface-format-version: 1.0
45+
// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name Client
46+
import Bar
47+
public func test() {}
48+
49+
//--- Client.package.swiftinterface
50+
// swift-interface-format-version: 1.0
51+
// swift-module-flags: -swift-version 5 -enable-library-evolution -module-name Client -package-name BarTest
52+
import Bar
53+
public func test() {}
54+
55+
//--- Client.swift
56+
import Bar
57+
58+
59+

test/lit.cfg

+1
Original file line numberDiff line numberDiff line change
@@ -2936,6 +2936,7 @@ config.substitutions.append(('%target-swiftdoc-name', target_specific_module_tri
29362936
config.substitutions.append(('%target-swiftsourceinfo-name', target_specific_module_triple + '.swiftsourceinfo'))
29372937
config.substitutions.append(('%target-swiftinterface-name', target_specific_module_triple + '.swiftinterface'))
29382938
config.substitutions.append(('%target-private-swiftinterface-name', target_specific_module_triple + '.private.swiftinterface'))
2939+
config.substitutions.append(('%target-package-swiftinterface-name', target_specific_module_triple + '.package.swiftinterface'))
29392940

29402941
config.substitutions.append(('%target-object-format', config.target_object_format))
29412942
config.substitutions.append(('%{target-shared-library-prefix}', config.target_shared_library_prefix))

0 commit comments

Comments
 (0)