Skip to content

Commit 52c62d4

Browse files
committed
Reland "[modules] Fix error about the same module being defined in different .pcm files when using VFS overlays."
Fixing Windows buildbot by not using "BuildTemporaries/module.modulemap" because it is interpreted as defining a module in "BuildTemporaries" directory. Fix errors like > module 'MultiPath' is defined in both 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-1352QHUF8RNMU.pcm' and 'path/to/modules.cache/3JR48BPRU7BCG/MultiPath-20HNSLLIUDDV1.pcm' To avoid building extra identical modules `-ivfsoverlay` option is not a part of the hash like "/3JR48BPRU7BCG/". And it is build system's responsibility to provide `-ivfsoverlay` options that don't cause observable differences. We also need to make sure the hash like "-1352QHUF8RNMU" is not affected by `-ivfsoverlay`. As this hash is defined by the module map path, use the path prior to any VFS remappings. rdar://111921464 Differential Revision: https://reviews.llvm.org/D156749
1 parent 2628fa3 commit 52c62d4

File tree

3 files changed

+113
-2
lines changed

3 files changed

+113
-2
lines changed

clang/lib/Lex/HeaderSearch.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ std::string HeaderSearch::getCachedModuleFileName(Module *Module) {
177177
// *.modulemap file. In this case, just return an empty string.
178178
if (!ModuleMap)
179179
return {};
180-
return getCachedModuleFileName(Module->Name, ModuleMap->getName());
180+
return getCachedModuleFileName(Module->Name, ModuleMap->getNameAsRequested());
181181
}
182182

183183
std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName,

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1327,7 +1327,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, ASTContext &Context,
13271327

13281328
auto &Map = PP.getHeaderSearchInfo().getModuleMap();
13291329
AddPath(WritingModule->PresumedModuleMapFile.empty()
1330-
? Map.getModuleMapFileForUniquing(WritingModule)->getName()
1330+
? Map.getModuleMapFileForUniquing(WritingModule)
1331+
->getNameAsRequested()
13311332
: StringRef(WritingModule->PresumedModuleMapFile),
13321333
Record);
13331334

clang/test/VFS/module-map-path.m

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,110 @@
1+
// Test the module map path is consistent between clang invocations when using VFS overlays.
2+
3+
// RUN: rm -rf %t
4+
// RUN: split-file %s %t
5+
6+
// Pre-populate the module cache with the modules that don't use VFS overlays.
7+
// RUN: %clang_cc1 -fsyntax-only -F%t/Frameworks -I%t/include %t/prepopulate_module_cache.m \
8+
// RUN: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
9+
10+
// Execute a compilation with VFS overlay. .pcm file path looks like <hash1>/ModuleName-<hash2>.pcm.
11+
// <hash1> corresponds to the compilation settings like language options.
12+
// <hash2> corresponds to the module map path. So if any of those change, we should use a different module.
13+
// But for VFS overlay we make an exception that it's not a part of <hash1> to reduce the number of built .pcm files.
14+
// Test that paths in overlays don't leak into <hash2> and don't cause using 2 .pcm files for the same module.
15+
// DEFINE: %{command} = %clang_cc1 -fsyntax-only -verify -F%t/Frameworks -I%t/include %t/test.m \
16+
// DEFINE: -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/modules.cache
17+
// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@@g" %t/overlay.yaml.template > %t/external-names-default.yaml
18+
// RUN: %{command} -ivfsoverlay %t/external-names-default.yaml
19+
20+
// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': true,@g" %t/overlay.yaml.template > %t/external-names-true.yaml
21+
// RUN: %{command} -ivfsoverlay %t/external-names-true.yaml
22+
23+
// RUN: sed -e "s@TMP_DIR@%{/t:regex_replacement}@g" -e "s@USE_EXTERNAL_NAMES_OPTION@'use-external-names': false,@g" %t/overlay.yaml.template > %t/external-names-false.yaml
24+
// RUN: %{command} -ivfsoverlay %t/external-names-false.yaml
25+
26+
//--- prepopulate_module_cache.m
27+
#import <Redirecting/Redirecting.h>
28+
29+
//--- test.m
30+
// At first import multi-path modules directly, so clang decides which .pcm file they should belong to.
31+
#import <MultiPath/MultiPath.h>
32+
#import <MultiPathHeader.h>
33+
34+
// Then import a module from the module cache and all its transitive dependencies.
35+
// Make sure the .pcm files loaded directly are the same as 'Redirecting' is referencing.
36+
#import <Redirecting/Redirecting.h>
37+
// expected-no-diagnostics
38+
39+
40+
//--- Frameworks/MultiPath.framework/Headers/MultiPath.h
41+
void multiPathFramework(void);
42+
43+
//--- Frameworks/MultiPath.framework/Modules/module.modulemap
44+
framework module MultiPath {
45+
header "MultiPath.h"
46+
export *
47+
}
48+
49+
50+
//--- include/MultiPathHeader.h
51+
void multiPathHeader(void);
52+
53+
//--- include/module.modulemap
54+
module MultiPathHeader {
55+
header "MultiPathHeader.h"
56+
export *
57+
}
58+
59+
60+
//--- Frameworks/Redirecting.framework/Headers/Redirecting.h
61+
#import <MultiPath/MultiPath.h>
62+
#import <MultiPathHeader.h>
63+
64+
//--- Frameworks/Redirecting.framework/Modules/module.modulemap
65+
framework module Redirecting {
66+
header "Redirecting.h"
67+
export *
68+
}
69+
70+
71+
//--- BuildTemporaries/MultiPath.h
72+
void multiPathFramework(void);
73+
//--- BuildTemporaries/framework.modulemap
74+
framework module MultiPath {
75+
header "MultiPath.h"
76+
export *
77+
}
78+
//--- BuildTemporaries/header.h
79+
void multiPathHeader(void);
80+
//--- BuildTemporaries/include.modulemap
81+
module MultiPathHeader {
82+
header "MultiPathHeader.h"
83+
export *
84+
}
85+
86+
//--- overlay.yaml.template
87+
{
88+
'version': 0,
89+
USE_EXTERNAL_NAMES_OPTION
90+
'roots': [
91+
{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Headers', 'type': 'directory',
92+
'contents': [
93+
{ 'name': 'MultiPath.h', 'type': 'file',
94+
'external-contents': 'TMP_DIR/BuildTemporaries/MultiPath.h'}
95+
]},
96+
{ 'name': 'TMP_DIR/Frameworks/MultiPath.framework/Modules', 'type': 'directory',
97+
'contents': [
98+
{ 'name': 'module.modulemap', 'type': 'file',
99+
'external-contents': 'TMP_DIR/BuildTemporaries/framework.modulemap'}
100+
]},
101+
{ 'name': 'TMP_DIR/include', 'type': 'directory',
102+
'contents': [
103+
{ 'name': 'MultiPathHeader.h', 'type': 'file',
104+
'external-contents': 'TMP_DIR/BuildTemporaries/header.h'},
105+
{ 'name': 'module.modulemap', 'type': 'file',
106+
'external-contents': 'TMP_DIR/BuildTemporaries/include.modulemap'}
107+
]}
108+
]
109+
}
110+

0 commit comments

Comments
 (0)