Skip to content

Commit 149367e

Browse files
author
Harlan Haskins
committed
[ParseableInterfaces] Short-circuit module loading
Previously, the ParseableInterfaceModuleLoader relied on the assumption that, if it returned `errc::not_supported`, it would fall through the search paths and then move on to the SerializedModuleLoader. This did not anticipate the possibility of a valid .swiftinterface coming later in the search paths, which can cause issues for the standard library which is in the resource-dir and should always be loaded from there. Instead, make the module loading explicitly short-circuit when seeing `errc::not_supported`, and document it. Also add some more logging throughout `discoverLoadableModule` so we can more easily catch issues like this in the future. Fixes rdar://49479386
1 parent be48d64 commit 149367e

File tree

3 files changed

+59
-6
lines changed

3 files changed

+59
-6
lines changed

include/swift/Serialization/SerializedModuleLoader.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,18 @@ class SerializedModuleLoaderBase : public ModuleLoader {
5151
std::unique_ptr<llvm::MemoryBuffer> *moduleDocBuffer,
5252
bool &isFramework);
5353

54+
/// Attempts to search the provided directory for a loadable serialized
55+
/// .swiftmodule with the provided `ModuleFilename`. Subclasses must
56+
/// override this method to perform their custom module lookup behavior.
57+
///
58+
/// If such a module could not be loaded, the subclass must return a
59+
/// `std::error_code` indicating the failure. There are two specific error
60+
/// codes that will be treated specially:
61+
/// - `errc::no_such_file_or_directory`: The module loader will stop looking
62+
/// for loadable modules and will diagnose the lookup failure.
63+
/// - `errc::not_supported`: The module loader will stop looking for loadable
64+
/// modules and will defer to the remaining module loaders to look up this
65+
/// module.
5466
virtual std::error_code findModuleFilesInDirectory(
5567
AccessPathElem ModuleID, StringRef DirPath, StringRef ModuleFilename,
5668
StringRef ModuleDocFilename,

lib/Frontend/ParseableInterfaceModuleLoader.cpp

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -880,13 +880,25 @@ class ParseableInterfaceModuleLoaderImpl {
880880
if (isForwardingModule) {
881881
if (auto forwardingModule = ForwardingModule::load(*buf)) {
882882
std::unique_ptr<llvm::MemoryBuffer> moduleBuffer;
883-
if (forwardingModuleIsUpToDate(*forwardingModule, deps, moduleBuffer))
883+
if (forwardingModuleIsUpToDate(*forwardingModule, deps,
884+
moduleBuffer)) {
885+
LLVM_DEBUG(llvm::dbgs() << "Found up-to-date forwarding module at "
886+
<< cachedOutputPath << "\n");
884887
return DiscoveredModule::forwarded(
885888
forwardingModule->underlyingModulePath, std::move(moduleBuffer));
889+
}
890+
891+
LLVM_DEBUG(llvm::dbgs() << "Found out-of-date forwarding module at "
892+
<< cachedOutputPath << "\n");
886893
}
887894
// Otherwise, check if the AST buffer itself is up to date.
888895
} else if (serializedASTBufferIsUpToDate(*buf, deps)) {
896+
LLVM_DEBUG(llvm::dbgs() << "Found up-to-date cached module at "
897+
<< cachedOutputPath << "\n");
889898
return DiscoveredModule::normal(cachedOutputPath, std::move(buf));
899+
} else {
900+
LLVM_DEBUG(llvm::dbgs() << "Found out-of-date cached module at "
901+
<< cachedOutputPath << "\n");
890902
}
891903
}
892904

@@ -899,23 +911,39 @@ class ParseableInterfaceModuleLoaderImpl {
899911
llvm::SmallString<256> scratch;
900912
std::unique_ptr<llvm::MemoryBuffer> moduleBuffer;
901913
auto path = computePrebuiltModulePath(scratch);
902-
if (path && swiftModuleIsUpToDate(*path, deps, moduleBuffer))
903-
return DiscoveredModule::prebuilt(*path, std::move(moduleBuffer));
914+
if (path) {
915+
if (swiftModuleIsUpToDate(*path, deps, moduleBuffer)) {
916+
LLVM_DEBUG(llvm::dbgs() << "Found up-to-date prebuilt module at "
917+
<< path->str() << "\n");
918+
return DiscoveredModule::prebuilt(*path, std::move(moduleBuffer));
919+
} else {
920+
LLVM_DEBUG(llvm::dbgs() << "Found out-of-date prebuilt module at "
921+
<< modulePath << "\n");
922+
}
923+
}
904924
}
905925

906926
// Finally, if there's a module adjacent to the .swiftinterface that we can
907927
// _likely_ load (it validates OK and is up to date), bail early with
908928
// errc::not_supported, so the next (serialized) loader in the chain will
909-
// load it. Alternately, if there's a .swiftmodule present but we can't even
929+
// load it.
930+
// Alternately, if there's a .swiftmodule present but we can't even
910931
// read it (for whatever reason), we should let the other module loader
911932
// diagnose it.
912933
if (!shouldLoadAdjacentModule)
913934
return notFoundError;
914935

915936
auto adjacentModuleBuffer = fs.getBufferForFile(modulePath);
916937
if (adjacentModuleBuffer) {
917-
if (serializedASTBufferIsUpToDate(*adjacentModuleBuffer.get(), deps))
938+
if (serializedASTBufferIsUpToDate(*adjacentModuleBuffer.get(), deps)) {
939+
LLVM_DEBUG(llvm::dbgs() << "Found up-to-date module at "
940+
<< modulePath
941+
<< "; deferring to serialized module loader\n");
918942
return std::make_error_code(std::errc::not_supported);
943+
} else {
944+
LLVM_DEBUG(llvm::dbgs() << "Found out-of-date module at "
945+
<< modulePath << "\n");
946+
}
919947
} else if (adjacentModuleBuffer.getError() != notFoundError) {
920948
return std::make_error_code(std::errc::not_supported);
921949
}
@@ -1070,8 +1098,15 @@ std::error_code ParseableInterfaceModuleLoader::findModuleFilesInDirectory(
10701098
auto Ext = file_types::getExtension(file_types::TY_SwiftParseableInterfaceFile);
10711099
InPath = ModPath;
10721100
path::replace_extension(InPath, Ext);
1073-
if (!fs.exists(InPath))
1101+
if (!fs.exists(InPath)) {
1102+
if (fs.exists(ModPath)) {
1103+
LLVM_DEBUG(llvm::dbgs()
1104+
<< "No .swiftinterface file found adjacent to module file "
1105+
<< ModPath.str() << "\n");
1106+
return std::make_error_code(std::errc::not_supported);
1107+
}
10741108
return std::make_error_code(std::errc::no_such_file_or_directory);
1109+
}
10751110

10761111
// Create an instance of the Impl to do the heavy lifting.
10771112
ParseableInterfaceModuleLoaderImpl Impl(

lib/Serialization/SerializedModuleLoader.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,8 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
218218
moduleBuffer, moduleDocBuffer);
219219
if (!result) {
220220
return true;
221+
} else if (result == std::errc::not_supported) {
222+
return false;
221223
} else if (result != std::errc::no_such_file_or_directory) {
222224
return None;
223225
}
@@ -251,6 +253,8 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
251253
moduleBuffer, moduleDocBuffer);
252254
if (!result)
253255
return true;
256+
else if (result == std::errc::not_supported)
257+
return false;
254258
}
255259
}
256260

@@ -320,6 +324,8 @@ SerializedModuleLoaderBase::findModule(AccessPathElem moduleID,
320324
moduleBuffer, moduleDocBuffer);
321325
if (!result)
322326
return true;
327+
else if (result == std::errc::not_supported)
328+
return false;
323329
}
324330
}
325331

0 commit comments

Comments
 (0)