From 140fd73f83d4f023cac50ad349d22ef60569fce8 Mon Sep 17 00:00:00 2001 From: Artem Chikin Date: Tue, 14 Jul 2020 12:10:13 -0700 Subject: [PATCH] [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. --- include/swift/Frontend/Frontend.h | 4 +-- .../Serialization/SerializedModuleLoader.h | 17 ++++----- include/swift/Serialization/Validation.h | 2 +- lib/Frontend/Frontend.cpp | 35 ++++++++++++------- lib/Serialization/SerializedModuleLoader.cpp | 8 ++--- .../can_import_with_map.swift | 8 +++++ ...xplicit-module-map-forwarding-module.swift | 8 +++++ .../explicit-module-map.swift | 8 +++++ test/ScanDependencies/module_not_found.swift | 20 +++++++++++ test/lit.cfg | 20 +++++++++++ .../SourceKit/lib/SwiftLang/SwiftIndexing.cpp | 4 +-- 11 files changed, 104 insertions(+), 30 deletions(-) create mode 100644 test/ScanDependencies/module_not_found.swift diff --git a/include/swift/Frontend/Frontend.h b/include/swift/Frontend/Frontend.h index 72555f19f02e0..f0496f88b5e79 100644 --- a/include/swift/Frontend/Frontend.h +++ b/include/swift/Frontend/Frontend.h @@ -53,7 +53,7 @@ namespace swift { -class SerializedModuleLoader; +class SerializedModuleLoaderBase; class MemoryBufferSerializedModuleLoader; class SILModule; @@ -434,7 +434,7 @@ class CompilerInstance { std::unique_ptr Stats; mutable ModuleDecl *MainModule = nullptr; - SerializedModuleLoader *SML = nullptr; + SerializedModuleLoaderBase *DefaultSerializedLoader = nullptr; MemoryBufferSerializedModuleLoader *MemoryBufferLoader = nullptr; /// Contains buffer IDs for input source code files. diff --git a/include/swift/Serialization/SerializedModuleLoader.h b/include/swift/Serialization/SerializedModuleLoader.h index fd8bfe227266d..ec2ad4a8a29ab 100644 --- a/include/swift/Serialization/SerializedModuleLoader.h +++ b/include/swift/Serialization/SerializedModuleLoader.h @@ -51,8 +51,9 @@ struct SerializedModuleBaseName { std::string getName(file_types::ID fileTy) const; }; -/// Common functionality shared between \c SerializedModuleLoader, -/// \c ModuleInterfaceLoader and \c MemoryBufferSerializedModuleLoader. +/// Common functionality shared between \c ImplicitSerializedModuleLoader, +/// \c ModuleInterfaceLoader, \c ExplicitSwiftModuleLoader +/// and \c MemoryBufferSerializedModuleLoader. class SerializedModuleLoaderBase : public ModuleLoader { /// A { module, generation # } pair. using LoadedModulePair = std::pair, unsigned>; @@ -215,9 +216,9 @@ class SerializedModuleLoaderBase : public ModuleLoader { }; /// Imports serialized Swift modules into an ASTContext. -class SerializedModuleLoader : public SerializedModuleLoaderBase { +class ImplicitSerializedModuleLoader : public SerializedModuleLoaderBase { - SerializedModuleLoader(ASTContext &ctx, DependencyTracker *tracker, + ImplicitSerializedModuleLoader(ASTContext &ctx, DependencyTracker *tracker, ModuleLoadingMode loadMode, bool IgnoreSwiftSourceInfo) : SerializedModuleLoaderBase(ctx, tracker, loadMode, IgnoreSwiftSourceInfo) {} @@ -236,7 +237,7 @@ class SerializedModuleLoader : public SerializedModuleLoaderBase { const SerializedModuleBaseName &BaseName) override; public: - virtual ~SerializedModuleLoader(); + virtual ~ImplicitSerializedModuleLoader(); /// Append visible module names to \p names. Note that names are possibly /// duplicated, and not guaranteed to be ordered in any way. @@ -245,12 +246,12 @@ class SerializedModuleLoader : public SerializedModuleLoaderBase { /// Create a new importer that can load serialized Swift modules /// into the given ASTContext. - static std::unique_ptr + static std::unique_ptr create(ASTContext &ctx, DependencyTracker *tracker = nullptr, ModuleLoadingMode loadMode = ModuleLoadingMode::PreferSerialized, bool IgnoreSwiftSourceInfo = false) { - return std::unique_ptr{ - new SerializedModuleLoader(ctx, tracker, loadMode, IgnoreSwiftSourceInfo) + return std::unique_ptr{ + new ImplicitSerializedModuleLoader(ctx, tracker, loadMode, IgnoreSwiftSourceInfo) }; } }; diff --git a/include/swift/Serialization/Validation.h b/include/swift/Serialization/Validation.h index c727b198cc0de..f0e84e7d7676e 100644 --- a/include/swift/Serialization/Validation.h +++ b/include/swift/Serialization/Validation.h @@ -172,7 +172,7 @@ ValidationInfo validateSerializedAST( /// Status::Valid. /// - \p moduleBufferID and \p moduleDocBufferID are the buffer identifiers /// of the module input and doc input buffers respectively (\ref -/// SerializedModuleLoader::loadAST, \ref ModuleFile::load). +/// SerializedModuleLoaderBase::loadAST, \ref ModuleFile::load). /// - \p loadedModuleFile is an invalid loaded module. /// - \p ModuleName is the name used to refer to the module in diagnostics. void diagnoseSerializedASTLoadFailure( diff --git a/lib/Frontend/Frontend.cpp b/lib/Frontend/Frontend.cpp index 9aff435557268..da7f171f9ba4b 100644 --- a/lib/Frontend/Frontend.cpp +++ b/lib/Frontend/Frontend.cpp @@ -424,14 +424,17 @@ void CompilerInstance::setUpDiagnosticOptions() { // Ideally, we'd get rid of it. // 2. MemoryBufferSerializedModuleLoader: This is used by LLDB, because it might // already have the module available in memory. -// 3. ModuleInterfaceLoader: Tries to find an up-to-date swiftmodule. If it +// 3. ExplicitSwiftModuleLoader: Loads a serialized module if it can, provided +// this modules was specified as an explicit input to the compiler. +// 4. ModuleInterfaceLoader: Tries to find an up-to-date swiftmodule. If it // succeeds, it issues a particular "error" (see -// [Note: ModuleInterfaceLoader-defer-to-SerializedModuleLoader]), which +// [Note: ModuleInterfaceLoader-defer-to-ImplicitSerializedModuleLoader]), which // is interpreted by the overarching loader as a command to use the -// SerializedModuleLoader. If we failed to find a .swiftmodule, this falls +// ImplicitSerializedModuleLoader. If we failed to find a .swiftmodule, this falls // back to using an interface. Actual errors lead to diagnostics. -// 4. SerializedModuleLoader: Loads a serialized module if it can. -// 5. ClangImporter: This must come after all the Swift module loaders because +// 5. ImplicitSerializedModuleLoader: Loads a serialized module if it can. +// Used for implicit loading of modules from the compiler's search paths. +// 6. ClangImporter: This must come after all the Swift module loaders because // in the presence of overlays and mixed-source frameworks, we want to prefer // the overlay or framework module over the underlying Clang module. bool CompilerInstance::setUpModuleLoaders() { @@ -484,15 +487,18 @@ bool CompilerInstance::setUpModuleLoaders() { // If implicit modules are disabled, we need to install an explicit module // loader. - if (Invocation.getFrontendOptions().DisableImplicitModules) { + bool ExplicitModuleBuild = Invocation.getFrontendOptions().DisableImplicitModules; + if (ExplicitModuleBuild) { auto ESML = ExplicitSwiftModuleLoader::create( *Context, getDependencyTracker(), MLM, Invocation.getSearchPathOptions().ExplicitSwiftModules, Invocation.getSearchPathOptions().ExplicitSwiftModuleMap, IgnoreSourceInfoFile); + this->DefaultSerializedLoader = ESML.get(); Context->addModuleLoader(std::move(ESML)); } + if (MLM != ModuleLoadingMode::OnlySerialized) { auto const &Clang = clangImporter->getClangInstance(); std::string ModuleCachePath = getModuleCachePathFromClang(Clang); @@ -507,12 +513,14 @@ bool CompilerInstance::setUpModuleLoaders() { Context->addModuleLoader(std::move(PIML), false, false, true); } - std::unique_ptr SML = - SerializedModuleLoader::create(*Context, getDependencyTracker(), MLM, + if (!ExplicitModuleBuild) { + std::unique_ptr ISML = + ImplicitSerializedModuleLoader::create(*Context, getDependencyTracker(), MLM, IgnoreSourceInfoFile); - this->SML = SML.get(); - Context->addModuleLoader(std::move(SML)); - + this->DefaultSerializedLoader = ISML.get(); + Context->addModuleLoader(std::move(ISML)); + } + Context->addModuleLoader(std::move(clangImporter), /*isClang*/ true); return false; @@ -868,6 +876,7 @@ bool CompilerInstance::loadStdlibIfNeeded() { bool CompilerInstance::loadPartialModulesAndImplicitImports( ModuleDecl *mod, SmallVectorImpl &partialModules) const { + assert(DefaultSerializedLoader && "Expected module loader in Compiler Instance"); FrontendStatsTracer tracer(getStatsReporter(), "load-partial-modules-and-implicit-imports"); // Force loading implicit imports. This is currently needed to allow @@ -881,7 +890,7 @@ bool CompilerInstance::loadPartialModulesAndImplicitImports( for (auto &PM : PartialModules) { assert(PM.ModuleBuffer); auto *file = - SML->loadAST(*mod, /*diagLoc*/ SourceLoc(), /*moduleInterfacePath*/ "", + DefaultSerializedLoader->loadAST(*mod, /*diagLoc*/ SourceLoc(), /*moduleInterfacePath*/ "", std::move(PM.ModuleBuffer), std::move(PM.ModuleDocBuffer), std::move(PM.ModuleSourceInfoBuffer), /*isFramework*/ false); @@ -974,7 +983,7 @@ void CompilerInstance::freeASTContext() { TheSILTypes.reset(); Context.reset(); MainModule = nullptr; - SML = nullptr; + DefaultSerializedLoader = nullptr; MemoryBufferLoader = nullptr; PrimaryBufferIDs.clear(); } diff --git a/lib/Serialization/SerializedModuleLoader.cpp b/lib/Serialization/SerializedModuleLoader.cpp index 25a81335ad015..fd8bb3f1824f7 100644 --- a/lib/Serialization/SerializedModuleLoader.cpp +++ b/lib/Serialization/SerializedModuleLoader.cpp @@ -115,7 +115,7 @@ SerializedModuleLoaderBase::SerializedModuleLoaderBase( IgnoreSwiftSourceInfoFile(IgnoreSwiftSourceInfoFile) {} SerializedModuleLoaderBase::~SerializedModuleLoaderBase() = default; -SerializedModuleLoader::~SerializedModuleLoader() = default; +ImplicitSerializedModuleLoader::~ImplicitSerializedModuleLoader() = default; MemoryBufferSerializedModuleLoader::~MemoryBufferSerializedModuleLoader() = default; @@ -243,7 +243,7 @@ void SerializedModuleLoaderBase::collectVisibleTopLevelModuleNamesImpl( }); } -void SerializedModuleLoader::collectVisibleTopLevelModuleNames( +void ImplicitSerializedModuleLoader::collectVisibleTopLevelModuleNames( SmallVectorImpl &names) const { collectVisibleTopLevelModuleNamesImpl( names, file_types::getExtension(file_types::TY_SwiftModuleFile)); @@ -397,7 +397,7 @@ llvm::ErrorOr SerializedModuleLoaderBase::scanModuleFile( return std::move(dependencies); } -std::error_code SerializedModuleLoader::findModuleFilesInDirectory( +std::error_code ImplicitSerializedModuleLoader::findModuleFilesInDirectory( AccessPathElem ModuleID, const SerializedModuleBaseName &BaseName, SmallVectorImpl *ModuleInterfacePath, @@ -434,7 +434,7 @@ std::error_code SerializedModuleLoader::findModuleFilesInDirectory( return std::error_code(); } -bool SerializedModuleLoader::maybeDiagnoseTargetMismatch( +bool ImplicitSerializedModuleLoader::maybeDiagnoseTargetMismatch( SourceLoc sourceLocation, StringRef moduleName, const SerializedModuleBaseName &absoluteBaseName) { llvm::vfs::FileSystem &fs = *Ctx.SourceMgr.getFileSystem(); diff --git a/test/ScanDependencies/can_import_with_map.swift b/test/ScanDependencies/can_import_with_map.swift index a65d5bd8e128e..42bd4d1c38efc 100644 --- a/test/ScanDependencies/can_import_with_map.swift +++ b/test/ScanDependencies/can_import_with_map.swift @@ -9,6 +9,14 @@ // RUN: echo "\"modulePath\": \"%/t/inputs/Foo.swiftmodule\"," >> %/t/inputs/map.json // RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json // RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json // RUN: echo "}]" >> %/t/inputs/map.json // RUN: %target-swift-frontend -typecheck %s -explicit-swift-module-map-file %t/inputs/map.json -disable-implicit-swift-modules diff --git a/test/ScanDependencies/explicit-module-map-forwarding-module.swift b/test/ScanDependencies/explicit-module-map-forwarding-module.swift index 4d9c00093c554..1e2a7ec047661 100644 --- a/test/ScanDependencies/explicit-module-map-forwarding-module.swift +++ b/test/ScanDependencies/explicit-module-map-forwarding-module.swift @@ -19,6 +19,14 @@ // RUN: echo "\"modulePath\": \"%/t/inputs/Foo-from-interface.swiftmodule\"," >> %/t/inputs/map.json // RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json // RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json // RUN: echo "}]" >> %/t/inputs/map.json // 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 diff --git a/test/ScanDependencies/explicit-module-map.swift b/test/ScanDependencies/explicit-module-map.swift index 78f5e91a80141..401014da2d894 100644 --- a/test/ScanDependencies/explicit-module-map.swift +++ b/test/ScanDependencies/explicit-module-map.swift @@ -10,6 +10,14 @@ // RUN: echo "\"modulePath\": \"%/t/inputs/Foo.swiftmodule\"," >> %/t/inputs/map.json // RUN: echo "\"docPath\": \"%/t/inputs/Foo.swiftdoc\"," >> %/t/inputs/map.json // RUN: echo "\"sourceInfoPath\": \"%/t/inputs/Foo.swiftsourceinfo\"" >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%stdlib_module\"," >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%ononesupport_module\"," >> %/t/inputs/map.json // RUN: echo "}]" >> %/t/inputs/map.json // 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 diff --git a/test/ScanDependencies/module_not_found.swift b/test/ScanDependencies/module_not_found.swift new file mode 100644 index 0000000000000..1674f380b3732 --- /dev/null +++ b/test/ScanDependencies/module_not_found.swift @@ -0,0 +1,20 @@ +// RUN: %empty-directory(%t) +// RUN: mkdir -p %t/clang-module-cache +// RUN: mkdir -p %t/inputs +// RUN: echo "public func foo() {}" >> %t/foo.swift +// 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 + +// RUN: echo "[{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%stdlib_dir/Swift.swiftmodule/%module-target-triple.swiftmodule\"," >> %/t/inputs/map.json +// RUN: echo "}," >> %/t/inputs/map.json +// RUN: echo "{" >> %/t/inputs/map.json +// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json +// RUN: echo "\"modulePath\": \"%stdlib_dir/SwiftOnoneSupport.swiftmodule/%module-target-triple.swiftmodule\"," >> %/t/inputs/map.json +// RUN: echo "}]" >> %/t/inputs/map.json + +// Add the -I search path to ensure we do not accidentally implicitly load Foo.swiftmodule +// RUN: not %target-swift-frontend -typecheck %s -I %t/inputs -explicit-swift-module-map-file %t/inputs/map.json -disable-implicit-swift-modules +import Foo +// CHECK: error: no such module 'Foo' + diff --git a/test/lit.cfg b/test/lit.cfg index bb0f16bc68700..e6973ebf5ae1e 100644 --- a/test/lit.cfg +++ b/test/lit.cfg @@ -1439,6 +1439,26 @@ config.substitutions.append(('%module-target-future', target_future)) # Add 'target-sdk-name' as the name for platform-specific directories config.substitutions.append(('%target-sdk-name', config.target_sdk_name)) +# Add 'stdlib_dir' as the path to the stdlib resource directory +stdlib_dir = config.swift_lib_dir + "/swift/" +if platform.system() == 'Linux': + stdlib_dir += config.target_sdk_name + "/" + run_cpu +else: + stdlib_dir += config.target_sdk_name +config.substitutions.append(('%stdlib_dir', stdlib_dir)) + +# Add 'stdlib_module' as the path to the stdlib .swiftmodule file +stdlib_module = stdlib_dir + "/Swift.swiftmodule" +if platform.system() == 'Darwin': + stdlib_module += "/" + target_specific_module_triple + ".swiftmodule" +config.substitutions.append(('%stdlib_module', stdlib_module)) + +# Add 'ononesupport_module' as the path to the SwiftOnoneSupport .swiftmodule file +ononesupport_module = stdlib_dir + "/SwiftOnoneSupport.swiftmodule" +if platform.system() == 'Darwin': + ononesupport_module += "/" + target_specific_module_triple + ".swiftmodule" +config.substitutions.append(('%ononesupport_module', ononesupport_module)) + # Different OS's require different prefixes for the environment variables to be # propagated to the calling contexts. # In order to make tests OS-agnostic, names of environment variables should be diff --git a/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp b/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp index deff6db7eef8f..2a67cc0490b9f 100644 --- a/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp +++ b/tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp @@ -211,12 +211,12 @@ static void indexModule(llvm::MemoryBuffer *Input, CompilerInstance &CI, ArrayRef Args) { ASTContext &Ctx = CI.getASTContext(); - std::unique_ptr Loader; + std::unique_ptr Loader; ModuleDecl *Mod = nullptr; if (ModuleName == Ctx.StdlibModuleName.str()) { Mod = Ctx.getModule({ {Ctx.StdlibModuleName, SourceLoc()} }); } else { - Loader = SerializedModuleLoader::create(Ctx); + Loader = ImplicitSerializedModuleLoader::create(Ctx); auto Buf = std::unique_ptr( llvm::MemoryBuffer::getMemBuffer(Input->getBuffer(), Input->getBufferIdentifier()));