Skip to content

Commit 965ca69

Browse files
artemcmartemcm
committed
Add in-source rationale for why Placeholder dependencies are needed.
1 parent 746e89b commit 965ca69

File tree

7 files changed

+107
-89
lines changed

7 files changed

+107
-89
lines changed

‎include/swift/AST/ModuleDependencies.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class Identifier;
3535
/// Which kind of module dependencies we are looking for.
3636
enum class ModuleDependenciesKind : int8_t {
3737
Swift,
38-
// Placeholder dependencies are a kind of dependencies used only by the
38+
// Placeholder dependencies are a kind of dependencies used only by the
3939
// dependency scanner. They are swift modules that the scanner will not be
4040
// able to locate in its search paths and which are the responsibility of the
4141
// scanner's client to ensure are provided.
@@ -361,6 +361,9 @@ class ModuleDependenciesCache {
361361
/// Dependencies for Swift modules that have already been computed.
362362
llvm::StringMap<ModuleDependencies> SwiftModuleDependencies;
363363

364+
/// Dependencies for Swift placeholder dependency modules.
365+
llvm::StringMap<ModuleDependencies> PlaceholderSwiftModuleDependencies;
366+
364367
/// Dependencies for Clang modules that have already been computed.
365368
llvm::StringMap<ModuleDependencies> ClangModuleDependencies;
366369

‎include/swift/Frontend/ModuleInterfaceLoader.h

Lines changed: 62 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,71 @@ class ExplicitModuleMapParser {
194194
ExplicitModuleMapParser(llvm::BumpPtrAllocator &Allocator) : Saver(Allocator) {}
195195

196196
std::error_code
197-
parseSwiftExplicitModuleMap(const StringRef fileName,
198-
llvm::StringMap<ExplicitModuleInfo> &moduleMap);
197+
parseSwiftExplicitModuleMap(llvm::StringRef fileName,
198+
llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
199+
using namespace llvm::yaml;
200+
// Load the input file.
201+
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> fileBufOrErr =
202+
llvm::MemoryBuffer::getFile(fileName);
203+
if (!fileBufOrErr) {
204+
return std::make_error_code(std::errc::no_such_file_or_directory);
205+
}
206+
StringRef Buffer = fileBufOrErr->get()->getBuffer();
207+
// Use a new source manager instead of the one from ASTContext because we
208+
// don't want the JSON file to be persistent.
209+
llvm::SourceMgr SM;
210+
Stream Stream(llvm::MemoryBufferRef(Buffer, fileName), SM);
211+
for (auto DI = Stream.begin(); DI != Stream.end(); ++DI) {
212+
assert(DI != Stream.end() && "Failed to read a document");
213+
if (auto *MN = dyn_cast_or_null<SequenceNode>(DI->getRoot())) {
214+
for (auto &entry : *MN) {
215+
if (parseSingleModuleEntry(entry, moduleMap)) {
216+
return std::make_error_code(std::errc::invalid_argument);
217+
}
218+
}
219+
} else {
220+
return std::make_error_code(std::errc::invalid_argument);
221+
}
222+
}
223+
return std::error_code{}; // success
224+
}
199225

200226
private:
201-
StringRef getScalaNodeText(llvm::yaml::Node *N);
227+
StringRef getScalaNodeText(llvm::yaml::Node *N) {
228+
SmallString<32> Buffer;
229+
return Saver.save(cast<llvm::yaml::ScalarNode>(N)->getValue(Buffer));
230+
}
231+
202232
bool parseSingleModuleEntry(llvm::yaml::Node &node,
203-
llvm::StringMap<ExplicitModuleInfo> &moduleMap);
233+
llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
234+
using namespace llvm::yaml;
235+
auto *mapNode = dyn_cast<MappingNode>(&node);
236+
if (!mapNode)
237+
return true;
238+
StringRef moduleName;
239+
ExplicitModuleInfo result;
240+
for (auto &entry : *mapNode) {
241+
auto key = getScalaNodeText(entry.getKey());
242+
auto val = getScalaNodeText(entry.getValue());
243+
if (key == "moduleName") {
244+
moduleName = val;
245+
} else if (key == "modulePath") {
246+
result.modulePath = val;
247+
} else if (key == "docPath") {
248+
result.moduleDocPath = val;
249+
} else if (key == "sourceInfoPath") {
250+
result.moduleSourceInfoPath = val;
251+
} else {
252+
// Being forgiving for future fields.
253+
continue;
254+
}
255+
}
256+
if (moduleName.empty())
257+
return true;
258+
moduleMap[moduleName] = std::move(result);
259+
return false;
260+
}
261+
204262
llvm::StringSaver Saver;
205263
};
206264

‎lib/AST/ModuleDependencies.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ ModuleDependenciesCache::getDependenciesMap(ModuleDependenciesKind kind) {
113113
case ModuleDependenciesKind::Swift:
114114
return SwiftModuleDependencies;
115115
case ModuleDependenciesKind::SwiftPlaceholder:
116-
return ExternalSwiftModuleDependencies;
116+
return PlaceholderSwiftModuleDependencies;
117117
case ModuleDependenciesKind::Clang:
118118
return ClangModuleDependencies;
119119
}
@@ -126,7 +126,7 @@ ModuleDependenciesCache::getDependenciesMap(ModuleDependenciesKind kind) const {
126126
case ModuleDependenciesKind::Swift:
127127
return SwiftModuleDependencies;
128128
case ModuleDependenciesKind::SwiftPlaceholder:
129-
return ExternalSwiftModuleDependencies;
129+
return PlaceholderSwiftModuleDependencies;
130130
case ModuleDependenciesKind::Clang:
131131
return ClangModuleDependencies;
132132
}

‎lib/AST/ModuleLoader.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ ModuleDependencies::collectCrossImportOverlayNames(ASTContext &ctx,
181181
} else if (auto *clangDep = dyn_cast<ClangModuleDependenciesStorage>(storage.get())){
182182
modulePath = clangDep->moduleMapFile;
183183
assert(modulePath.hasValue());
184-
} else { // ExternalSwiftModuleDependency
184+
} else { // PlaceholderSwiftModuleDependencies
185185
return result;
186186
}
187187
findOverlayFilesInternal(ctx, *modulePath, moduleName, SourceLoc(),

‎lib/Frontend/ModuleInterfaceLoader.cpp

Lines changed: 0 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,70 +1530,6 @@ struct ExplicitSwiftModuleLoader::Implementation {
15301530
}
15311531
};
15321532

1533-
StringRef ExplicitModuleMapParser::getScalaNodeText(llvm::yaml::Node *N) {
1534-
SmallString<32> Buffer;
1535-
return Saver.save(cast<llvm::yaml::ScalarNode>(N)->getValue(Buffer));
1536-
}
1537-
1538-
bool ExplicitModuleMapParser::parseSingleModuleEntry(
1539-
llvm::yaml::Node &node, llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
1540-
using namespace llvm::yaml;
1541-
auto *mapNode = dyn_cast<MappingNode>(&node);
1542-
if (!mapNode)
1543-
return true;
1544-
StringRef moduleName;
1545-
ExplicitModuleInfo result;
1546-
for (auto &entry : *mapNode) {
1547-
auto key = getScalaNodeText(entry.getKey());
1548-
auto val = getScalaNodeText(entry.getValue());
1549-
if (key == "moduleName") {
1550-
moduleName = val;
1551-
} else if (key == "modulePath") {
1552-
result.modulePath = val;
1553-
} else if (key == "docPath") {
1554-
result.moduleDocPath = val;
1555-
} else if (key == "sourceInfoPath") {
1556-
result.moduleSourceInfoPath = val;
1557-
} else {
1558-
// Being forgiving for future fields.
1559-
continue;
1560-
}
1561-
}
1562-
if (moduleName.empty())
1563-
return true;
1564-
moduleMap[moduleName] = std::move(result);
1565-
return false;
1566-
}
1567-
1568-
std::error_code ExplicitModuleMapParser::parseSwiftExplicitModuleMap(
1569-
const StringRef fileName, llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
1570-
using namespace llvm::yaml;
1571-
// Load the input file.
1572-
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> fileBufOrErr =
1573-
llvm::MemoryBuffer::getFile(fileName);
1574-
if (!fileBufOrErr) {
1575-
return std::make_error_code(std::errc::no_such_file_or_directory);
1576-
}
1577-
StringRef Buffer = fileBufOrErr->get()->getBuffer();
1578-
// Use a new source manager instead of the one from ASTContext because we
1579-
// don't want the JSON file to be persistent.
1580-
llvm::SourceMgr SM;
1581-
Stream Stream(llvm::MemoryBufferRef(Buffer, fileName), SM);
1582-
for (auto DI = Stream.begin(); DI != Stream.end(); ++DI) {
1583-
assert(DI != Stream.end() && "Failed to read a document");
1584-
if (auto *MN = dyn_cast_or_null<SequenceNode>(DI->getRoot())) {
1585-
for (auto &entry : *MN) {
1586-
if (parseSingleModuleEntry(entry, moduleMap)) {
1587-
return std::make_error_code(std::errc::invalid_argument);
1588-
}
1589-
}
1590-
} else {
1591-
return std::make_error_code(std::errc::invalid_argument);
1592-
}
1593-
}
1594-
return std::error_code{}; // success
1595-
}
1596-
15971533
ExplicitSwiftModuleLoader::ExplicitSwiftModuleLoader(
15981534
ASTContext &ctx,
15991535
DependencyTracker *tracker,

‎lib/Serialization/ModuleDependencyScanner.cpp

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,14 @@
1010
//
1111
//===----------------------------------------------------------------------===//
1212

13-
#include "swift/Serialization/SerializedModuleLoader.h"
1413
#include "swift/AST/ASTContext.h"
1514
#include "swift/AST/DiagnosticSuppression.h"
15+
#include "swift/AST/DiagnosticsFrontend.h"
1616
#include "swift/AST/ModuleDependencies.h"
1717
#include "swift/AST/SourceFile.h"
1818
#include "swift/Basic/FileTypes.h"
19+
#include "swift/Frontend/ModuleInterfaceLoader.h"
20+
#include "swift/Serialization/SerializedModuleLoader.h"
1921
#include "swift/Subsystems.h"
2022
using namespace swift;
2123
using llvm::ErrorOr;
@@ -100,7 +102,22 @@ class ModuleDependencyScanner : public SerializedModuleLoaderBase {
100102
/// This "loader" will not attempt to load any module files.
101103
class PlaceholderSwiftModuleScanner : public ModuleDependencyScanner {
102104
/// Scan the given placeholder module map
103-
void parsePlaceholderModuleMap(StringRef fileName);
105+
void parsePlaceholderModuleMap(StringRef fileName) {
106+
ExplicitModuleMapParser parser(Allocator);
107+
auto result =
108+
parser.parseSwiftExplicitModuleMap(fileName, PlaceholderDependencyModuleMap);
109+
if (result == std::errc::invalid_argument) {
110+
Ctx.Diags.diagnose(SourceLoc(),
111+
diag::placeholder_dependency_module_map_corrupted,
112+
fileName);
113+
}
114+
else if (result == std::errc::no_such_file_or_directory) {
115+
Ctx.Diags.diagnose(SourceLoc(),
116+
diag::placeholder_dependency_module_map_missing,
117+
fileName);
118+
}
119+
}
120+
104121
llvm::StringMap<ExplicitModuleInfo> PlaceholderDependencyModuleMap;
105122
llvm::BumpPtrAllocator Allocator;
106123

@@ -143,18 +160,6 @@ class PlaceholderSwiftModuleScanner : public ModuleDependencyScanner {
143160
};
144161
} // namespace
145162

146-
void PlaceholderSwiftModuleScanner::parsePlaceholderModuleMap(StringRef fileName) {
147-
ExplicitModuleMapParser parser(Allocator);
148-
auto result =
149-
parser.parseSwiftExplicitModuleMap(fileName, PlaceholderDependencyModuleMap);
150-
if (result == std::errc::invalid_argument)
151-
Ctx.Diags.diagnose(
152-
SourceLoc(), diag::placeholder_dependency_module_map_corrupted, fileName);
153-
else if (result == std::errc::no_such_file_or_directory)
154-
Ctx.Diags.diagnose(SourceLoc(),
155-
diag::placeholder_dependency_module_map_missing, fileName);
156-
}
157-
158163
static std::vector<std::string> getCompiledCandidates(ASTContext &ctx,
159164
StringRef moduleName,
160165
StringRef interfacePath) {

‎test/ScanDependencies/module_deps_external.swift

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,22 @@
22
// RUN: mkdir -p %t/clang-module-cache
33
// RUN: mkdir -p %t/inputs
44

5+
// RUN: echo "[{" > %/t/inputs/map.json
6+
// RUN: echo "\"moduleName\": \"SomeExternalModule\"," >> %/t/inputs/map.json
7+
// RUN: echo "\"modulePath\": \"%/t/inputs/SomeExternalModule.swiftmodule\"," >> %/t/inputs/map.json
8+
// RUN: echo "\"docPath\": \"%/t/inputs/SomeExternalModule.swiftdoc\"," >> %/t/inputs/map.json
9+
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/SomeExternalModule.swiftsourceinfo\"" >> %/t/inputs/map.json
10+
// RUN: echo "}," >> %/t/inputs/map.json
11+
// RUN: echo "{" >> %/t/inputs/map.json
12+
// RUN: echo "\"moduleName\": \"Swift\"," >> %/t/inputs/map.json
13+
// RUN: echo "\"modulePath\": \"%/stdlib_module\"" >> %/t/inputs/map.json
14+
// RUN: echo "}," >> %/t/inputs/map.json
15+
// RUN: echo "{" >> %/t/inputs/map.json
16+
// RUN: echo "\"moduleName\": \"SwiftOnoneSupport\"," >> %/t/inputs/map.json
17+
// RUN: echo "\"modulePath\": \"%/ononesupport_module\"" >> %/t/inputs/map.json
18+
// RUN: echo "}]" >> %/t/inputs/map.json
19+
20+
521
// RUN: echo "[{" > %/t/inputs/map.json
622
// RUN: echo "\"moduleName\": \"SomeExternalModule\"," >> %/t/inputs/map.json
723
// RUN: echo "\"modulePath\": \"%/t/inputs/SomeExternalModule.swiftmodule\"," >> %/t/inputs/map.json
@@ -71,11 +87,11 @@ import SomeExternalModule
7187
// CHECK-NEXT: ]
7288

7389
/// --------Swift external module SomeExternalModule
74-
// CHECK-LABEL: "modulePath": "BUILD_DIR/test-macosx-x86_64/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftmodule",
90+
// CHECK-LABEL: "modulePath": "BUILD_DIR/{{.*}}/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftmodule",
7591
// CHECK-NEXT: "details": {
7692
// CHECK-NEXT: "swiftPlaceholder": {
77-
// CHECK-NEXT: "moduleDocPath": "BUILD_DIR/test-macosx-x86_64/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftdoc",
78-
// CHECK-NEXT: "moduleSourceInfoPath": "BUILD_DIR/test-macosx-x86_64/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftsourceinfo",
93+
// CHECK-NEXT: "moduleDocPath": "BUILD_DIR/{{.*}}/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftdoc",
94+
// CHECK-NEXT: "moduleSourceInfoPath": "BUILD_DIR/{{.*}}/ScanDependencies/Output/module_deps_external.swift.tmp/inputs/SomeExternalModule.swiftsourceinfo",
7995

8096
/// --------Swift module Swift
8197
// CHECK-LABEL: "modulePath": "Swift.swiftmodule",

0 commit comments

Comments
 (0)