Skip to content

Commit 140fd73

Browse files
artemcmartemcm
committed
[Explicit Module Builds] Prevent SerializedModuleLoader from running in Explicit Module Build mode.
In order to avoid accidentally implicitly loading modules that are expected but were not provided as explicit inputs. - Use either SerializedModuleLoader or ExplicitSwiftModuleLoader for loading of partial modules, depending on whether we are in Explicit Module Build or Implicit Module Build mode.
1 parent a6aa7a5 commit 140fd73

File tree

11 files changed

+104
-30
lines changed

11 files changed

+104
-30
lines changed

‎include/swift/Frontend/Frontend.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@
5353

5454
namespace swift {
5555

56-
class SerializedModuleLoader;
56+
class SerializedModuleLoaderBase;
5757
class MemoryBufferSerializedModuleLoader;
5858
class SILModule;
5959

@@ -434,7 +434,7 @@ class CompilerInstance {
434434
std::unique_ptr<UnifiedStatsReporter> Stats;
435435

436436
mutable ModuleDecl *MainModule = nullptr;
437-
SerializedModuleLoader *SML = nullptr;
437+
SerializedModuleLoaderBase *DefaultSerializedLoader = nullptr;
438438
MemoryBufferSerializedModuleLoader *MemoryBufferLoader = nullptr;
439439

440440
/// Contains buffer IDs for input source code files.

‎include/swift/Serialization/SerializedModuleLoader.h‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,9 @@ struct SerializedModuleBaseName {
5151
std::string getName(file_types::ID fileTy) const;
5252
};
5353

54-
/// Common functionality shared between \c SerializedModuleLoader,
55-
/// \c ModuleInterfaceLoader and \c MemoryBufferSerializedModuleLoader.
54+
/// Common functionality shared between \c ImplicitSerializedModuleLoader,
55+
/// \c ModuleInterfaceLoader, \c ExplicitSwiftModuleLoader
56+
/// and \c MemoryBufferSerializedModuleLoader.
5657
class SerializedModuleLoaderBase : public ModuleLoader {
5758
/// A { module, generation # } pair.
5859
using LoadedModulePair = std::pair<std::unique_ptr<ModuleFile>, unsigned>;
@@ -215,9 +216,9 @@ class SerializedModuleLoaderBase : public ModuleLoader {
215216
};
216217

217218
/// Imports serialized Swift modules into an ASTContext.
218-
class SerializedModuleLoader : public SerializedModuleLoaderBase {
219+
class ImplicitSerializedModuleLoader : public SerializedModuleLoaderBase {
219220

220-
SerializedModuleLoader(ASTContext &ctx, DependencyTracker *tracker,
221+
ImplicitSerializedModuleLoader(ASTContext &ctx, DependencyTracker *tracker,
221222
ModuleLoadingMode loadMode, bool IgnoreSwiftSourceInfo)
222223
: SerializedModuleLoaderBase(ctx, tracker, loadMode, IgnoreSwiftSourceInfo)
223224
{}
@@ -236,7 +237,7 @@ class SerializedModuleLoader : public SerializedModuleLoaderBase {
236237
const SerializedModuleBaseName &BaseName) override;
237238

238239
public:
239-
virtual ~SerializedModuleLoader();
240+
virtual ~ImplicitSerializedModuleLoader();
240241

241242
/// Append visible module names to \p names. Note that names are possibly
242243
/// duplicated, and not guaranteed to be ordered in any way.
@@ -245,12 +246,12 @@ class SerializedModuleLoader : public SerializedModuleLoaderBase {
245246

246247
/// Create a new importer that can load serialized Swift modules
247248
/// into the given ASTContext.
248-
static std::unique_ptr<SerializedModuleLoader>
249+
static std::unique_ptr<ImplicitSerializedModuleLoader>
249250
create(ASTContext &ctx, DependencyTracker *tracker = nullptr,
250251
ModuleLoadingMode loadMode = ModuleLoadingMode::PreferSerialized,
251252
bool IgnoreSwiftSourceInfo = false) {
252-
return std::unique_ptr<SerializedModuleLoader>{
253-
new SerializedModuleLoader(ctx, tracker, loadMode, IgnoreSwiftSourceInfo)
253+
return std::unique_ptr<ImplicitSerializedModuleLoader>{
254+
new ImplicitSerializedModuleLoader(ctx, tracker, loadMode, IgnoreSwiftSourceInfo)
254255
};
255256
}
256257
};

‎include/swift/Serialization/Validation.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ ValidationInfo validateSerializedAST(
172172
/// Status::Valid.
173173
/// - \p moduleBufferID and \p moduleDocBufferID are the buffer identifiers
174174
/// of the module input and doc input buffers respectively (\ref
175-
/// SerializedModuleLoader::loadAST, \ref ModuleFile::load).
175+
/// SerializedModuleLoaderBase::loadAST, \ref ModuleFile::load).
176176
/// - \p loadedModuleFile is an invalid loaded module.
177177
/// - \p ModuleName is the name used to refer to the module in diagnostics.
178178
void diagnoseSerializedASTLoadFailure(

‎lib/Frontend/Frontend.cpp‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -424,14 +424,17 @@ void CompilerInstance::setUpDiagnosticOptions() {
424424
// Ideally, we'd get rid of it.
425425
// 2. MemoryBufferSerializedModuleLoader: This is used by LLDB, because it might
426426
// already have the module available in memory.
427-
// 3. ModuleInterfaceLoader: Tries to find an up-to-date swiftmodule. If it
427+
// 3. ExplicitSwiftModuleLoader: Loads a serialized module if it can, provided
428+
// this modules was specified as an explicit input to the compiler.
429+
// 4. ModuleInterfaceLoader: Tries to find an up-to-date swiftmodule. If it
428430
// succeeds, it issues a particular "error" (see
429-
// [Note: ModuleInterfaceLoader-defer-to-SerializedModuleLoader]), which
431+
// [Note: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]), which
430432
// is interpreted by the overarching loader as a command to use the
431-
// SerializedModuleLoader. If we failed to find a .swiftmodule, this falls
433+
// ImplicitSerializedModuleLoader. If we failed to find a .swiftmodule, this falls
432434
// back to using an interface. Actual errors lead to diagnostics.
433-
// 4. SerializedModuleLoader: Loads a serialized module if it can.
434-
// 5. ClangImporter: This must come after all the Swift module loaders because
435+
// 5. ImplicitSerializedModuleLoader: Loads a serialized module if it can.
436+
// Used for implicit loading of modules from the compiler's search paths.
437+
// 6. ClangImporter: This must come after all the Swift module loaders because
435438
// in the presence of overlays and mixed-source frameworks, we want to prefer
436439
// the overlay or framework module over the underlying Clang module.
437440
bool CompilerInstance::setUpModuleLoaders() {
@@ -484,15 +487,18 @@ bool CompilerInstance::setUpModuleLoaders() {
484487

485488
// If implicit modules are disabled, we need to install an explicit module
486489
// loader.
487-
if (Invocation.getFrontendOptions().DisableImplicitModules) {
490+
bool ExplicitModuleBuild = Invocation.getFrontendOptions().DisableImplicitModules;
491+
if (ExplicitModuleBuild) {
488492
auto ESML = ExplicitSwiftModuleLoader::create(
489493
*Context,
490494
getDependencyTracker(), MLM,
491495
Invocation.getSearchPathOptions().ExplicitSwiftModules,
492496
Invocation.getSearchPathOptions().ExplicitSwiftModuleMap,
493497
IgnoreSourceInfoFile);
498+
this->DefaultSerializedLoader = ESML.get();
494499
Context->addModuleLoader(std::move(ESML));
495500
}
501+
496502
if (MLM != ModuleLoadingMode::OnlySerialized) {
497503
auto const &Clang = clangImporter->getClangInstance();
498504
std::string ModuleCachePath = getModuleCachePathFromClang(Clang);
@@ -507,12 +513,14 @@ bool CompilerInstance::setUpModuleLoaders() {
507513
Context->addModuleLoader(std::move(PIML), false, false, true);
508514
}
509515

510-
std::unique_ptr<SerializedModuleLoader> SML =
511-
SerializedModuleLoader::create(*Context, getDependencyTracker(), MLM,
516+
if (!ExplicitModuleBuild) {
517+
std::unique_ptr<ImplicitSerializedModuleLoader> ISML =
518+
ImplicitSerializedModuleLoader::create(*Context, getDependencyTracker(), MLM,
512519
IgnoreSourceInfoFile);
513-
this->SML = SML.get();
514-
Context->addModuleLoader(std::move(SML));
515-
520+
this->DefaultSerializedLoader = ISML.get();
521+
Context->addModuleLoader(std::move(ISML));
522+
}
523+
516524
Context->addModuleLoader(std::move(clangImporter), /*isClang*/ true);
517525

518526
return false;
@@ -868,6 +876,7 @@ bool CompilerInstance::loadStdlibIfNeeded() {
868876

869877
bool CompilerInstance::loadPartialModulesAndImplicitImports(
870878
ModuleDecl *mod, SmallVectorImpl<FileUnit *> &partialModules) const {
879+
assert(DefaultSerializedLoader && "Expected module loader in Compiler Instance");
871880
FrontendStatsTracer tracer(getStatsReporter(),
872881
"load-partial-modules-and-implicit-imports");
873882
// Force loading implicit imports. This is currently needed to allow
@@ -881,7 +890,7 @@ bool CompilerInstance::loadPartialModulesAndImplicitImports(
881890
for (auto &PM : PartialModules) {
882891
assert(PM.ModuleBuffer);
883892
auto *file =
884-
SML->loadAST(*mod, /*diagLoc*/ SourceLoc(), /*moduleInterfacePath*/ "",
893+
DefaultSerializedLoader->loadAST(*mod, /*diagLoc*/ SourceLoc(), /*moduleInterfacePath*/ "",
885894
std::move(PM.ModuleBuffer), std::move(PM.ModuleDocBuffer),
886895
std::move(PM.ModuleSourceInfoBuffer),
887896
/*isFramework*/ false);
@@ -974,7 +983,7 @@ void CompilerInstance::freeASTContext() {
974983
TheSILTypes.reset();
975984
Context.reset();
976985
MainModule = nullptr;
977-
SML = nullptr;
986+
DefaultSerializedLoader = nullptr;
978987
MemoryBufferLoader = nullptr;
979988
PrimaryBufferIDs.clear();
980989
}

‎lib/Serialization/SerializedModuleLoader.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ SerializedModuleLoaderBase::SerializedModuleLoaderBase(
115115
IgnoreSwiftSourceInfoFile(IgnoreSwiftSourceInfoFile) {}
116116

117117
SerializedModuleLoaderBase::~SerializedModuleLoaderBase() = default;
118-
SerializedModuleLoader::~SerializedModuleLoader() = default;
118+
ImplicitSerializedModuleLoader::~ImplicitSerializedModuleLoader() = default;
119119
MemoryBufferSerializedModuleLoader::~MemoryBufferSerializedModuleLoader() =
120120
default;
121121

@@ -243,7 +243,7 @@ void SerializedModuleLoaderBase::collectVisibleTopLevelModuleNamesImpl(
243243
});
244244
}
245245

246-
void SerializedModuleLoader::collectVisibleTopLevelModuleNames(
246+
void ImplicitSerializedModuleLoader::collectVisibleTopLevelModuleNames(
247247
SmallVectorImpl<Identifier> &names) const {
248248
collectVisibleTopLevelModuleNamesImpl(
249249
names, file_types::getExtension(file_types::TY_SwiftModuleFile));
@@ -397,7 +397,7 @@ llvm::ErrorOr<ModuleDependencies> SerializedModuleLoaderBase::scanModuleFile(
397397
return std::move(dependencies);
398398
}
399399

400-
std::error_code SerializedModuleLoader::findModuleFilesInDirectory(
400+
std::error_code ImplicitSerializedModuleLoader::findModuleFilesInDirectory(
401401
AccessPathElem ModuleID,
402402
const SerializedModuleBaseName &BaseName,
403403
SmallVectorImpl<char> *ModuleInterfacePath,
@@ -434,7 +434,7 @@ std::error_code SerializedModuleLoader::findModuleFilesInDirectory(
434434
return std::error_code();
435435
}
436436

437-
bool SerializedModuleLoader::maybeDiagnoseTargetMismatch(
437+
bool ImplicitSerializedModuleLoader::maybeDiagnoseTargetMismatch(
438438
SourceLoc sourceLocation, StringRef moduleName,
439439
const SerializedModuleBaseName &absoluteBaseName) {
440440
llvm::vfs::FileSystem &fs = *Ctx.SourceMgr.getFileSystem();

‎test/ScanDependencies/can_import_with_map.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,14 @@
99
// RUN: echo "\"modulePath\": \"%/t/inputs/Foo.swiftmodule\"," >> %/t/inputs/map.json
1010
// RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json
1111
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json
12+
// RUN: echo "}," >> %/t/inputs/map.json
13+
// RUN: echo "{" >> %/t/inputs/map.json
14+
// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
15+
// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json
16+
// RUN: echo "}," >> %/t/inputs/map.json
17+
// RUN: echo "{" >> %/t/inputs/map.json
18+
// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
19+
// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json
1220
// RUN: echo "}]" >> %/t/inputs/map.json
1321

1422
// RUN: %target-swift-frontend -typecheck %s -explicit-swift-module-map-file %t/inputs/map.json -disable-implicit-swift-modules

‎test/ScanDependencies/explicit-module-map-forwarding-module.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,14 @@
1919
// RUN: echo "\"modulePath\": \"%/t/inputs/Foo-from-interface.swiftmodule\"," >> %/t/inputs/map.json
2020
// RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json
2121
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json
22+
// RUN: echo "}," >> %/t/inputs/map.json
23+
// RUN: echo "{" >> %/t/inputs/map.json
24+
// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
25+
// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json
26+
// RUN: echo "}," >> %/t/inputs/map.json
27+
// RUN: echo "{" >> %/t/inputs/map.json
28+
// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
29+
// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json
2230
// RUN: echo "}]" >> %/t/inputs/map.json
2331

2432
// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -enable-swiftsourceinfo -source-filename %s -explicit-swift-module-map-file %t/inputs/map.json | %FileCheck %s

‎test/ScanDependencies/explicit-module-map.swift‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@
1010
// RUN: echo "\"modulePath\": \"%/t/inputs/Foo.swiftmodule\"," >> %/t/inputs/map.json
1111
// RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json
1212
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json
13+
// RUN: echo "}," >> %/t/inputs/map.json
14+
// RUN: echo "{" >> %/t/inputs/map.json
15+
// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
16+
// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json
17+
// RUN: echo "}," >> %/t/inputs/map.json
18+
// RUN: echo "{" >> %/t/inputs/map.json
19+
// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
20+
// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json
1321
// RUN: echo "}]" >> %/t/inputs/map.json
1422

1523
// RUN: %target-swift-ide-test -print-module-comments -module-to-print=Foo -enable-swiftsourceinfo -source-filename %s -explicit-swift-module-map-file %t/inputs/map.json | %FileCheck %s

‎test/ScanDependencies/module_not_found.swift‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// RUN: %empty-directory(%t)
2+
// RUN: mkdir -p %t/clang-module-cache
3+
// RUN: mkdir -p %t/inputs
4+
// RUN: echo "public func foo() {}" >> %t/foo.swift
5+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/inputs/Foo.swiftmodule -emit-module-doc-path %t/inputs/Foo.swiftdoc -emit-module-source-info -emit-module-source-info-path %t/inputs/Foo.swiftsourceinfo -module-cache-path %t.module-cache %t/foo.swift -module-name Foo
6+
7+
// RUN: echo "[{" >> %/t/inputs/map.json
8+
// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
9+
// RUN: echo "\"modulePath\": \"%stdlib_dir/Swift.swiftmodule/%module-target-triple.swiftmodule\"," >> %/t/inputs/map.json
10+
// RUN: echo "}," >> %/t/inputs/map.json
11+
// RUN: echo "{" >> %/t/inputs/map.json
12+
// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
13+
// RUN: echo "\"modulePath\": \"%stdlib_dir/SwiftOnoneSupport.swiftmodule/%module-target-triple.swiftmodule\"," >> %/t/inputs/map.json
14+
// RUN: echo "}]" >> %/t/inputs/map.json
15+
16+
// Add the -I search path to ensure we do not accidentally implicitly load Foo.swiftmodule
17+
// RUN: not %target-swift-frontend -typecheck %s -I %t/inputs -explicit-swift-module-map-file %t/inputs/map.json -disable-implicit-swift-modules
18+
import Foo
19+
// CHECK: error: no such module 'Foo'
20+

‎test/lit.cfg‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1439,6 +1439,26 @@ config.substitutions.append(('%module-target-future', target_future))
14391439
# Add 'target-sdk-name' as the name for platform-specific directories
14401440
config.substitutions.append(('%target-sdk-name', config.target_sdk_name))
14411441

1442+
# Add 'stdlib_dir' as the path to the stdlib resource directory
1443+
stdlib_dir = config.swift_lib_dir + "/swift/"
1444+
if platform.system() == 'Linux':
1445+
stdlib_dir += config.target_sdk_name + "/" + run_cpu
1446+
else:
1447+
stdlib_dir += config.target_sdk_name
1448+
config.substitutions.append(('%stdlib_dir', stdlib_dir))
1449+
1450+
# Add 'stdlib_module' as the path to the stdlib .swiftmodule file
1451+
stdlib_module = stdlib_dir + "/Swift.swiftmodule"
1452+
if platform.system() == 'Darwin':
1453+
stdlib_module += "/" + target_specific_module_triple + ".swiftmodule"
1454+
config.substitutions.append(('%stdlib_module', stdlib_module))
1455+
1456+
# Add 'ononesupport_module' as the path to the SwiftOnoneSupport .swiftmodule file
1457+
ononesupport_module = stdlib_dir + "/SwiftOnoneSupport.swiftmodule"
1458+
if platform.system() == 'Darwin':
1459+
ononesupport_module += "/" + target_specific_module_triple + ".swiftmodule"
1460+
config.substitutions.append(('%ononesupport_module', ononesupport_module))
1461+
14421462
# Different OS's require different prefixes for the environment variables to be
14431463
# propagated to the calling contexts.
14441464
# In order to make tests OS-agnostic, names of environment variables should be

0 commit comments

Comments
 (0)