Skip to content

Commit d609029

Browse files
authored
[clang][modules] Allow module maps with textual headers to be non-affecting (#89441)
When writing out a PCM, we skip serializing headers' `HeaderFileInfo` struct whenever this condition evaluates to `true`: ```c++ !HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ``` However, when Clang parses a module map file, each textual header gets a `HFI` with `isModuleHeader=false`, `isTextualModuleHeader=true` and `isCompilingModuleHeader=false`. This means the condition evaluates to `false` even if the header was never included and the module map did not affect the compilation. Each PCM file that happened to parse such module map then contains a copy of the `HeaderFileInfo` struct for all textual headers, and considers the containing module map affecting. This patch makes it so that we skip headers that have not been included, essentially removing the virality of textual headers when it comes to PCM serialization.
1 parent b10e4b8 commit d609029

File tree

4 files changed

+101
-38
lines changed

4 files changed

+101
-38
lines changed

clang/include/clang/Lex/HeaderSearch.h

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,12 @@ class TargetInfo;
5656
/// The preprocessor keeps track of this information for each
5757
/// file that is \#included.
5858
struct HeaderFileInfo {
59+
// TODO: Whether the file was included is not a property of the file itself.
60+
// It's a preprocessor state, move it there.
61+
/// True if this file has been included (or imported) **locally**.
62+
LLVM_PREFERRED_TYPE(bool)
63+
unsigned IsLocallyIncluded : 1;
64+
5965
// TODO: Whether the file was imported is not a property of the file itself.
6066
// It's a preprocessor state, move it there.
6167
/// True if this is a \#import'd file.
@@ -135,10 +141,10 @@ struct HeaderFileInfo {
135141
StringRef Framework;
136142

137143
HeaderFileInfo()
138-
: isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
139-
External(false), isModuleHeader(false), isTextualModuleHeader(false),
140-
isCompilingModuleHeader(false), Resolved(false),
141-
IndexHeaderMapHeader(false), IsValid(false) {}
144+
: IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
145+
DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
146+
isTextualModuleHeader(false), isCompilingModuleHeader(false),
147+
Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
142148

143149
/// Retrieve the controlling macro for this header file, if
144150
/// any.

clang/lib/Lex/HeaderSearch.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1574,6 +1574,7 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
15741574
}
15751575
}
15761576

1577+
FileInfo.IsLocallyIncluded = true;
15771578
IsFirstIncludeOfFile = PP.markIncluded(File);
15781579
return true;
15791580
}

clang/lib/Serialization/ASTWriter.cpp

Lines changed: 44 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -171,34 +171,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
171171
.ModulesPruneNonAffectingModuleMaps)
172172
return std::nullopt;
173173

174-
SmallVector<const Module *> ModulesToProcess{RootModule};
175-
176174
const HeaderSearch &HS = PP.getHeaderSearchInfo();
177-
178-
SmallVector<OptionalFileEntryRef, 16> FilesByUID;
179-
HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
180-
181-
if (FilesByUID.size() > HS.header_file_size())
182-
FilesByUID.resize(HS.header_file_size());
183-
184-
for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
185-
OptionalFileEntryRef File = FilesByUID[UID];
186-
if (!File)
187-
continue;
188-
189-
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
190-
if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
191-
continue;
192-
193-
for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
194-
if (!KH.getModule())
195-
continue;
196-
ModulesToProcess.push_back(KH.getModule());
197-
}
198-
}
199-
200175
const ModuleMap &MM = HS.getModuleMap();
201-
SourceManager &SourceMgr = PP.getSourceManager();
176+
const SourceManager &SourceMgr = PP.getSourceManager();
202177

203178
std::set<const FileEntry *> ModuleMaps;
204179
auto CollectIncludingModuleMaps = [&](FileID FID, FileEntryRef F) {
@@ -233,12 +208,48 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
233208
}
234209
};
235210

236-
for (const Module *CurrentModule : ModulesToProcess) {
211+
// Handle all the affecting modules referenced from the root module.
212+
213+
std::queue<const Module *> Q;
214+
Q.push(RootModule);
215+
while (!Q.empty()) {
216+
const Module *CurrentModule = Q.front();
217+
Q.pop();
218+
237219
CollectIncludingMapsFromAncestors(CurrentModule);
238220
for (const Module *ImportedModule : CurrentModule->Imports)
239221
CollectIncludingMapsFromAncestors(ImportedModule);
240222
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
241223
CollectIncludingMapsFromAncestors(UndeclaredModule);
224+
225+
for (auto *M : CurrentModule->submodules())
226+
Q.push(M);
227+
}
228+
229+
// Handle textually-included headers that belong to other modules.
230+
231+
SmallVector<OptionalFileEntryRef, 16> FilesByUID;
232+
HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
233+
234+
if (FilesByUID.size() > HS.header_file_size())
235+
FilesByUID.resize(HS.header_file_size());
236+
237+
for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
238+
OptionalFileEntryRef File = FilesByUID[UID];
239+
if (!File)
240+
continue;
241+
242+
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
243+
if (!HFI)
244+
continue; // We have no information on this being a header file.
245+
if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
246+
continue; // Modular header, handled in the above module-based loop.
247+
if (!HFI->isCompilingModuleHeader && !HFI->IsLocallyIncluded)
248+
continue; // Non-modular header not included locally is not affecting.
249+
250+
for (const auto &KH : HS.findResolvedModulesForHeader(*File))
251+
if (const Module *M = KH.getModule())
252+
CollectIncludingMapsFromAncestors(M);
242253
}
243254

244255
return ModuleMaps;
@@ -2053,14 +2064,13 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
20532064
if (!File)
20542065
continue;
20552066

2056-
// Get the file info. Skip emitting this file if we have no information on
2057-
// it as a header file (in which case HFI will be null) or if it hasn't
2058-
// changed since it was loaded. Also skip it if it's for a modular header
2059-
// from a different module; in that case, we rely on the module(s)
2060-
// containing the header to provide this information.
20612067
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
2062-
if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
2063-
continue;
2068+
if (!HFI)
2069+
continue; // We have no information on this being a header file.
2070+
if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
2071+
continue; // Header file info is tracked by the owning module file.
2072+
if (!HFI->isCompilingModuleHeader && !PP->alreadyIncluded(*File))
2073+
continue; // Non-modular header not included is not needed.
20642074

20652075
// Massage the file path into an appropriate form.
20662076
StringRef Filename = File->getName();
Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
// This test checks that a module map with a textual header can be marked as
2+
// non-affecting.
3+
4+
// RUN: rm -rf %t && mkdir %t
5+
// RUN: split-file %s %t
6+
7+
//--- X.modulemap
8+
module X { textual header "X.h" }
9+
//--- X.h
10+
typedef int X_int;
11+
12+
//--- Y.modulemap
13+
module Y { textual header "Y.h" }
14+
//--- Y.h
15+
typedef int Y_int;
16+
17+
//--- A.modulemap
18+
module A { header "A.h" export * }
19+
//--- A.h
20+
#include "X.h"
21+
22+
// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A0.pcm \
23+
// RUN: -fmodule-map-file=%t/X.modulemap
24+
// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A0 --implicit-check-not=Y.modulemap
25+
// A0: Input file: {{.*}}X.modulemap
26+
27+
// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A1.pcm \
28+
// RUN: -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
29+
// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A1 \
30+
// RUN: --implicit-check-not=Y.modulemap
31+
// A1: Input file: {{.*}}X.modulemap
32+
33+
// RUN: diff %t/A0.pcm %t/A1.pcm
34+
35+
//--- B.modulemap
36+
module B { header "B.h" export * }
37+
//--- B.h
38+
#include "A.h"
39+
typedef X_int B_int;
40+
41+
// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B.pcm \
42+
// RUN: -fmodule-file=A=%t/A0.pcm \
43+
// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
44+
// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/B.pcm | FileCheck %s --check-prefix=B \
45+
// RUN: --implicit-check-not=X.modulemap --implicit-check-not=Y.modulemap
46+
// B: Input file: {{.*}}B.modulemap

0 commit comments

Comments
 (0)