Skip to content

Commit a0b5dd4

Browse files
artemcmartemcm
committed
Separate parsing out Swift and Clang modules from the explicit module map
Since #63178 added support for Clang modules in the explicit module map, it is possible for there to be multiple modules with the same name: a Swift module and a Clang module. The current parsing logic just overwrites the corresponding entry module in a hashmap so we always only preserved the module that comes last, with the same name. This change separates the parsing of the modulemap JSON file to produce a separate Swift module map and Clang module map. The Swift one is used by the 'ExplicitSwiftModuleLoader', as before, and the Clang one is only used to populate the ClangArgs with the requried -fmodule-... flags.
1 parent e851856 commit a0b5dd4

File tree

5 files changed

+105
-51
lines changed

5 files changed

+105
-51
lines changed

‎include/swift/Frontend/ModuleInterfaceLoader.h‎

Lines changed: 76 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -176,24 +176,49 @@ class ExplicitSwiftModuleLoader: public SerializedModuleLoaderBase {
176176
~ExplicitSwiftModuleLoader();
177177
};
178178

179-
/// Information about explicitly specified Swift and Clang module files.
180-
struct ExplicitModuleInfo {
181-
// Path of the .swiftmodule file. Empty for pure Clang modules.
179+
180+
// Explicitly-specified Swift module inputs
181+
struct ExplicitSwiftModuleInputInfo {
182+
ExplicitSwiftModuleInputInfo(std::string modulePath,
183+
llvm::Optional<std::string> moduleDocPath,
184+
llvm::Optional<std::string> moduleSourceInfoPath,
185+
bool isFramework = false,
186+
bool isSystem = false)
187+
: modulePath(modulePath),
188+
moduleDocPath(moduleDocPath),
189+
moduleSourceInfoPath(moduleSourceInfoPath),
190+
isFramework(isFramework),
191+
isSystem(isSystem) {}
192+
// Path of the .swiftmodule file.
193+
std::string modulePath;
194+
// Path of the .swiftmoduledoc file.
195+
llvm::Optional<std::string> moduleDocPath;
196+
// Path of the .swiftsourceinfo file.
197+
llvm::Optional<std::string> moduleSourceInfoPath;
198+
// A flag that indicates whether this module is a framework
199+
bool isFramework = false;
200+
// A flag that indicates whether this module is a system module
201+
bool isSystem = false;
202+
};
203+
204+
// Explicitly-specified Clang module inputs
205+
struct ExplicitClangModuleInputInfo {
206+
ExplicitClangModuleInputInfo(std::string moduleMapPath,
207+
std::string modulePath,
208+
bool isFramework = false,
209+
bool isSystem = false)
210+
: moduleMapPath(moduleMapPath),
211+
modulePath(modulePath),
212+
isFramework(isFramework),
213+
isSystem(isSystem) {}
214+
// Path of the Clang module map file.
215+
std::string moduleMapPath;
216+
// Path of a compiled Clang explicit module file (pcm).
182217
std::string modulePath;
183-
// Path of the .swiftmoduledoc file. Empty for pure Clang modules.
184-
std::string moduleDocPath;
185-
// Path of the .swiftsourceinfo file. Empty for pure Clang modules.
186-
std::string moduleSourceInfoPath;
187218
// A flag that indicates whether this module is a framework
188219
bool isFramework = false;
189220
// A flag that indicates whether this module is a system module
190-
// Set the default to be false.
191221
bool isSystem = false;
192-
// Path of the Clang module map file. Empty for pure Swift modules.
193-
std::string clangModuleMapPath;
194-
// Path of a compiled Clang explicit module file. Empty for pure Swift
195-
// modules.
196-
std::string clangModulePath;
197222
};
198223

199224
/// Parser of explicit module maps passed into the compiler.
@@ -223,7 +248,8 @@ class ExplicitModuleMapParser {
223248

224249
std::error_code
225250
parseSwiftExplicitModuleMap(llvm::StringRef fileName,
226-
llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
251+
llvm::StringMap<ExplicitSwiftModuleInputInfo> &swiftModuleMap,
252+
llvm::StringMap<ExplicitClangModuleInputInfo> &clangModuleMap) {
227253
using namespace llvm::yaml;
228254
// Load the input file.
229255
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> fileBufOrErr =
@@ -240,7 +266,7 @@ class ExplicitModuleMapParser {
240266
assert(DI != Stream.end() && "Failed to read a document");
241267
if (auto *MN = dyn_cast_or_null<SequenceNode>(DI->getRoot())) {
242268
for (auto &entry : *MN) {
243-
if (parseSingleModuleEntry(entry, moduleMap)) {
269+
if (parseSingleModuleEntry(entry, swiftModuleMap, clangModuleMap)) {
244270
return std::make_error_code(std::errc::invalid_argument);
245271
}
246272
}
@@ -269,40 +295,65 @@ class ExplicitModuleMapParser {
269295
}
270296

271297
bool parseSingleModuleEntry(llvm::yaml::Node &node,
272-
llvm::StringMap<ExplicitModuleInfo> &moduleMap) {
298+
llvm::StringMap<ExplicitSwiftModuleInputInfo> &swiftModuleMap,
299+
llvm::StringMap<ExplicitClangModuleInputInfo> &clangModuleMap) {
273300
using namespace llvm::yaml;
274301
auto *mapNode = dyn_cast<MappingNode>(&node);
275302
if (!mapNode)
276303
return true;
277304
StringRef moduleName;
278-
ExplicitModuleInfo result;
305+
llvm::Optional<std::string> swiftModulePath, swiftModuleDocPath,
306+
swiftModuleSourceInfoPath;
307+
std::string clangModuleMapPath = "", clangModulePath = "";
308+
bool isFramework = false, isSystem = false;
279309
for (auto &entry : *mapNode) {
280310
auto key = getScalaNodeText(entry.getKey());
281311
auto val = getScalaNodeText(entry.getValue());
282312
if (key == "moduleName") {
283313
moduleName = val;
284314
} else if (key == "modulePath") {
285-
result.modulePath = val.str();
315+
swiftModulePath = val.str();
286316
} else if (key == "docPath") {
287-
result.moduleDocPath = val.str();
317+
swiftModuleDocPath = val.str();
288318
} else if (key == "sourceInfoPath") {
289-
result.moduleSourceInfoPath = val.str();
319+
swiftModuleSourceInfoPath = val.str();
290320
} else if (key == "isFramework") {
291-
result.isFramework = parseBoolValue(val);
321+
isFramework = parseBoolValue(val);
292322
} else if (key == "isSystem") {
293-
result.isSystem = parseBoolValue(val);
323+
isSystem = parseBoolValue(val);
294324
} else if (key == "clangModuleMapPath") {
295-
result.clangModuleMapPath = val.str();
325+
clangModuleMapPath = val.str();
296326
} else if (key == "clangModulePath") {
297-
result.clangModulePath = val.str();
327+
clangModulePath = val.str();
298328
} else {
299329
// Being forgiving for future fields.
300330
continue;
301331
}
302332
}
303333
if (moduleName.empty())
304334
return true;
305-
moduleMap[moduleName] = std::move(result);
335+
336+
if (swiftModulePath.has_value()) {
337+
assert((clangModuleMapPath.empty() &&
338+
clangModulePath.empty()) &&
339+
"Unexpected Clang dependency details for Swift module");
340+
ExplicitSwiftModuleInputInfo entry(swiftModulePath.value(),
341+
swiftModuleDocPath,
342+
swiftModuleSourceInfoPath,
343+
isFramework,
344+
isSystem);
345+
swiftModuleMap.try_emplace(moduleName, std::move(entry));
346+
} else {
347+
assert((!clangModuleMapPath.empty() ||
348+
!clangModulePath.empty()) &&
349+
"Expected Clang dependency module");
350+
ExplicitClangModuleInputInfo entry(clangModuleMapPath,
351+
clangModulePath,
352+
isFramework,
353+
isSystem);
354+
clangModuleMap.try_emplace(moduleName, std::move(entry));
355+
}
356+
306357
return false;
307358
}
308359

‎include/swift/Serialization/ModuleDependencyScanner.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -84,8 +84,10 @@ namespace swift {
8484
/// Scan the given placeholder module map
8585
void parsePlaceholderModuleMap(StringRef fileName) {
8686
ExplicitModuleMapParser parser(Allocator);
87+
llvm::StringMap<ExplicitClangModuleInputInfo> ClangDependencyModuleMap;
8788
auto result =
88-
parser.parseSwiftExplicitModuleMap(fileName, PlaceholderDependencyModuleMap);
89+
parser.parseSwiftExplicitModuleMap(fileName, PlaceholderDependencyModuleMap,
90+
ClangDependencyModuleMap);
8991
if (result == std::errc::invalid_argument) {
9092
Ctx.Diags.diagnose(SourceLoc(),
9193
diag::placeholder_dependency_module_map_corrupted,
@@ -98,7 +100,7 @@ namespace swift {
98100
}
99101
}
100102

101-
llvm::StringMap<ExplicitModuleInfo> PlaceholderDependencyModuleMap;
103+
llvm::StringMap<ExplicitSwiftModuleInputInfo> PlaceholderDependencyModuleMap;
102104
llvm::BumpPtrAllocator Allocator;
103105

104106
public:

‎lib/Frontend/ModuleInterfaceLoader.cpp‎

Lines changed: 12 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1859,13 +1859,15 @@ InterfaceSubContextDelegateImpl::runInSubCompilerInstance(StringRef moduleName,
18591859
struct ExplicitSwiftModuleLoader::Implementation {
18601860
ASTContext &Ctx;
18611861
llvm::BumpPtrAllocator Allocator;
1862-
llvm::StringMap<ExplicitModuleInfo> ExplicitModuleMap;
1862+
llvm::StringMap<ExplicitSwiftModuleInputInfo> ExplicitModuleMap;
18631863
Implementation(ASTContext &Ctx) : Ctx(Ctx) {}
18641864

18651865
void parseSwiftExplicitModuleMap(StringRef fileName) {
18661866
ExplicitModuleMapParser parser(Allocator);
1867+
llvm::StringMap<ExplicitClangModuleInputInfo> ExplicitClangModuleMap;
18671868
auto result =
1868-
parser.parseSwiftExplicitModuleMap(fileName, ExplicitModuleMap);
1869+
parser.parseSwiftExplicitModuleMap(fileName, ExplicitModuleMap,
1870+
ExplicitClangModuleMap);
18691871
if (result == std::errc::invalid_argument)
18701872
Ctx.Diags.diagnose(SourceLoc(), diag::explicit_swift_module_map_corrupted,
18711873
fileName);
@@ -1877,16 +1879,16 @@ struct ExplicitSwiftModuleLoader::Implementation {
18771879
// we've seen so that we don't generate duplicate flags.
18781880
std::set<std::string> moduleMapsSeen;
18791881
std::vector<std::string> &extraClangArgs = Ctx.ClangImporterOpts.ExtraArgs;
1880-
for (auto &entry : ExplicitModuleMap) {
1881-
const auto &moduleMapPath = entry.getValue().clangModuleMapPath;
1882+
for (auto &entry : ExplicitClangModuleMap) {
1883+
const auto &moduleMapPath = entry.getValue().moduleMapPath;
18821884
if (!moduleMapPath.empty() &&
18831885
moduleMapsSeen.find(moduleMapPath) == moduleMapsSeen.end()) {
18841886
moduleMapsSeen.insert(moduleMapPath);
18851887
extraClangArgs.push_back(
18861888
(Twine("-fmodule-map-file=") + moduleMapPath).str());
18871889
}
18881890

1889-
const auto &modulePath = entry.getValue().clangModulePath;
1891+
const auto &modulePath = entry.getValue().modulePath;
18901892
if (!modulePath.empty()) {
18911893
extraClangArgs.push_back(
18921894
(Twine("-fmodule-file=") + entry.getKey() + "=" + modulePath)
@@ -1899,8 +1901,7 @@ struct ExplicitSwiftModuleLoader::Implementation {
18991901
const std::vector<std::pair<std::string, std::string>>
19001902
&commandLineExplicitInputs) {
19011903
for (const auto &moduleInput : commandLineExplicitInputs) {
1902-
ExplicitModuleInfo entry;
1903-
entry.modulePath = moduleInput.second;
1904+
ExplicitSwiftModuleInputInfo entry(moduleInput.second, {}, {});
19041905
ExplicitModuleMap.try_emplace(moduleInput.first, std::move(entry));
19051906
}
19061907
}
@@ -1939,16 +1940,6 @@ bool ExplicitSwiftModuleLoader::findModule(
19391940
}
19401941
auto &moduleInfo = it->getValue();
19411942

1942-
// If this is only a Clang module with no paired Swift module, return false
1943-
// now so that we don't emit diagnostics about it being missing. This gives
1944-
// ClangImporter an opportunity to import it.
1945-
bool hasClangModule = !moduleInfo.clangModuleMapPath.empty() ||
1946-
!moduleInfo.clangModulePath.empty();
1947-
bool hasSwiftModule = !moduleInfo.modulePath.empty();
1948-
if (hasClangModule && !hasSwiftModule) {
1949-
return false;
1950-
}
1951-
19521943
// Set IsFramework bit according to the moduleInfo
19531944
IsFramework = moduleInfo.isFramework;
19541945
IsSystemModule = moduleInfo.isSystem;
@@ -1990,14 +1981,14 @@ bool ExplicitSwiftModuleLoader::findModule(
19901981
*ModuleBuffer = std::move(moduleBuf.get());
19911982

19921983
// Open .swiftdoc file
1993-
if (!moduleInfo.moduleDocPath.empty()) {
1994-
auto moduleDocBuf = fs.getBufferForFile(moduleInfo.moduleDocPath);
1984+
if (moduleInfo.moduleDocPath.has_value()) {
1985+
auto moduleDocBuf = fs.getBufferForFile(moduleInfo.moduleDocPath.value());
19951986
if (moduleBuf)
19961987
*ModuleDocBuffer = std::move(moduleDocBuf.get());
19971988
}
19981989
// Open .swiftsourceinfo file
1999-
if (!moduleInfo.moduleSourceInfoPath.empty()) {
2000-
auto moduleSourceInfoBuf = fs.getBufferForFile(moduleInfo.moduleSourceInfoPath);
1990+
if (moduleInfo.moduleSourceInfoPath.has_value()) {
1991+
auto moduleSourceInfoBuf = fs.getBufferForFile(moduleInfo.moduleSourceInfoPath.value());
20011992
if (moduleSourceInfoBuf)
20021993
*ModuleSourceInfoBuffer = std::move(moduleSourceInfoBuf.get());
20031994
}

‎lib/Serialization/ModuleDependencyScanner.cpp‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,11 @@ bool PlaceholderSwiftModuleScanner::findModule(
8383
}
8484
auto &moduleInfo = it->getValue();
8585
auto dependencies = ModuleDependencyInfo::forPlaceholderSwiftModuleStub(
86-
moduleInfo.modulePath, moduleInfo.moduleDocPath,
87-
moduleInfo.moduleSourceInfoPath);
86+
moduleInfo.modulePath,
87+
moduleInfo.moduleDocPath.has_value() ?
88+
moduleInfo.moduleDocPath.value() : "",
89+
moduleInfo.moduleSourceInfoPath.has_value() ?
90+
moduleInfo.moduleSourceInfoPath.value() : "");
8891
this->dependencies = std::move(dependencies);
8992
return true;
9093
}

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,18 @@
33
// RUN: mkdir -p %t/inputs
44
// RUN: echo "public func anotherFuncA() {}" > %t/A.swift
55
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/inputs/A.swiftmodule -emit-module-doc-path %t/inputs/A.swiftdoc -emit-module-source-info -emit-module-source-info-path %t/inputs/A.swiftsourceinfo -import-underlying-module -I%S/Inputs/CHeaders -module-cache-path %t.module-cache %t/A.swift -module-name A
6+
// RUN: %target-swift-emit-pcm -module-name A -o %t/inputs/A.pcm %S/Inputs/CHeaders/module.modulemap
67

78
// RUN: echo "[{" > %/t/inputs/map.json
89
// RUN: echo "\"moduleName\": \"A\"," >> %/t/inputs/map.json
910
// RUN: echo "\"modulePath\": \"%/t/inputs/A.swiftmodule\"," >> %/t/inputs/map.json
1011
// RUN: echo "\"docPath\": \"%/t/inputs/A.swiftdoc\"," >> %/t/inputs/map.json
1112
// RUN: echo "\"sourceInfoPath\": \"%/t/inputs/A.swiftsourceinfo\"," >> %/t/inputs/map.json
1213
// RUN: echo "\"isFramework\": false," >> %/t/inputs/map.json
14+
// RUN: echo "}," >> %/t/inputs/map.json
15+
// RUN: echo "{" >> %/t/inputs/map.json
16+
// RUN: echo "\"moduleName\": \"A\"," >> %/t/inputs/map.json
17+
// RUN: echo "\"clangModulePath\": \"%/t/inputs/A.pcm\"," >> %/t/inputs/map.json
1318
// RUN: echo "\"clangModuleMapPath\": \"%/S/Inputs/CHeaders/module.modulemap\"" >> %/t/inputs/map.json
1419
// RUN: echo "}," >> %/t/inputs/map.json
1520
// RUN: echo "{" >> %/t/inputs/map.json
@@ -33,7 +38,9 @@
3338
// RUN: echo "\"isFramework\": false" >> %/t/inputs/map.json
3439
// RUN: echo "}]" >> %/t/inputs/map.json
3540

36-
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/Foo.swiftmodule -module-cache-path %t.module-cache -explicit-swift-module-map-file %t/inputs/map.json %s
41+
// RUN: %target-swift-frontend -emit-module -emit-module-path %t/Foo.swiftmodule -disable-implicit-swift-modules -module-cache-path %t.module-cache -explicit-swift-module-map-file %t/inputs/map.json -Rmodule-loading -Xcc -Rmodule-import %s 2>&1 | %FileCheck %s
42+
43+
// CHECK: <unknown>:0: remark: importing module 'A' from {{.*}}{{/|\\}}explicit-module-map-clang-and-swift.swift.tmp{{/|\\}}inputs{{/|\\}}A.pcm'
3744

3845
import A
3946

0 commit comments

Comments
 (0)