[clang] d609029 - [clang][modules] Allow module maps with textual headers to be non-affecting (#89441)
via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 24 09:06:02 PDT 2024
Author: Jan Svoboda
Date: 2024-04-24T09:05:56-07:00
New Revision: d609029d6c9aae84b52238f39c35200316bdbb93
URL: https://github.com/llvm/llvm-project/commit/d609029d6c9aae84b52238f39c35200316bdbb93
DIFF: https://github.com/llvm/llvm-project/commit/d609029d6c9aae84b52238f39c35200316bdbb93.diff
LOG: [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.
Added:
clang/test/Modules/prune-non-affecting-module-map-files-textual.c
Modified:
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
clang/lib/Serialization/ASTWriter.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index c5f90ef4cb3682..5ac63dddd4d4ea 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -56,6 +56,12 @@ class TargetInfo;
/// The preprocessor keeps track of this information for each
/// file that is \#included.
struct HeaderFileInfo {
+ // TODO: Whether the file was included is not a property of the file itself.
+ // It's a preprocessor state, move it there.
+ /// True if this file has been included (or imported) **locally**.
+ LLVM_PREFERRED_TYPE(bool)
+ unsigned IsLocallyIncluded : 1;
+
// TODO: Whether the file was imported is not a property of the file itself.
// It's a preprocessor state, move it there.
/// True if this is a \#import'd file.
@@ -135,10 +141,10 @@ struct HeaderFileInfo {
StringRef Framework;
HeaderFileInfo()
- : isImport(false), isPragmaOnce(false), DirInfo(SrcMgr::C_User),
- External(false), isModuleHeader(false), isTextualModuleHeader(false),
- isCompilingModuleHeader(false), Resolved(false),
- IndexHeaderMapHeader(false), IsValid(false) {}
+ : IsLocallyIncluded(false), isImport(false), isPragmaOnce(false),
+ DirInfo(SrcMgr::C_User), External(false), isModuleHeader(false),
+ isTextualModuleHeader(false), isCompilingModuleHeader(false),
+ Resolved(false), IndexHeaderMapHeader(false), IsValid(false) {}
/// Retrieve the controlling macro for this header file, if
/// any.
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..574723b33866af 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1574,6 +1574,7 @@ bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
}
}
+ FileInfo.IsLocallyIncluded = true;
IsFirstIncludeOfFile = PP.markIncluded(File);
return true;
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index d0c1217156a593..30195868ca9965 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -171,34 +171,9 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
.ModulesPruneNonAffectingModuleMaps)
return std::nullopt;
- SmallVector<const Module *> ModulesToProcess{RootModule};
-
const HeaderSearch &HS = PP.getHeaderSearchInfo();
-
- SmallVector<OptionalFileEntryRef, 16> FilesByUID;
- HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
-
- if (FilesByUID.size() > HS.header_file_size())
- FilesByUID.resize(HS.header_file_size());
-
- for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
- OptionalFileEntryRef File = FilesByUID[UID];
- if (!File)
- continue;
-
- const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
- continue;
-
- for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
- if (!KH.getModule())
- continue;
- ModulesToProcess.push_back(KH.getModule());
- }
- }
-
const ModuleMap &MM = HS.getModuleMap();
- SourceManager &SourceMgr = PP.getSourceManager();
+ const SourceManager &SourceMgr = PP.getSourceManager();
std::set<const FileEntry *> ModuleMaps;
auto CollectIncludingModuleMaps = [&](FileID FID, FileEntryRef F) {
@@ -233,12 +208,48 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
}
};
- for (const Module *CurrentModule : ModulesToProcess) {
+ // Handle all the affecting modules referenced from the root module.
+
+ std::queue<const Module *> Q;
+ Q.push(RootModule);
+ while (!Q.empty()) {
+ const Module *CurrentModule = Q.front();
+ Q.pop();
+
CollectIncludingMapsFromAncestors(CurrentModule);
for (const Module *ImportedModule : CurrentModule->Imports)
CollectIncludingMapsFromAncestors(ImportedModule);
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
CollectIncludingMapsFromAncestors(UndeclaredModule);
+
+ for (auto *M : CurrentModule->submodules())
+ Q.push(M);
+ }
+
+ // Handle textually-included headers that belong to other modules.
+
+ SmallVector<OptionalFileEntryRef, 16> FilesByUID;
+ HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+ if (FilesByUID.size() > HS.header_file_size())
+ FilesByUID.resize(HS.header_file_size());
+
+ for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+ OptionalFileEntryRef File = FilesByUID[UID];
+ if (!File)
+ continue;
+
+ const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
+ if (!HFI)
+ continue; // We have no information on this being a header file.
+ if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
+ continue; // Modular header, handled in the above module-based loop.
+ if (!HFI->isCompilingModuleHeader && !HFI->IsLocallyIncluded)
+ continue; // Non-modular header not included locally is not affecting.
+
+ for (const auto &KH : HS.findResolvedModulesForHeader(*File))
+ if (const Module *M = KH.getModule())
+ CollectIncludingMapsFromAncestors(M);
}
return ModuleMaps;
@@ -2053,14 +2064,13 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
if (!File)
continue;
- // Get the file info. Skip emitting this file if we have no information on
- // it as a header file (in which case HFI will be null) or if it hasn't
- // changed since it was loaded. Also skip it if it's for a modular header
- // from a
diff erent module; in that case, we rely on the module(s)
- // containing the header to provide this information.
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
- continue;
+ if (!HFI)
+ continue; // We have no information on this being a header file.
+ if (!HFI->isCompilingModuleHeader && HFI->isModuleHeader)
+ continue; // Header file info is tracked by the owning module file.
+ if (!HFI->isCompilingModuleHeader && !PP->alreadyIncluded(*File))
+ continue; // Non-modular header not included is not needed.
// Massage the file path into an appropriate form.
StringRef Filename = File->getName();
diff --git a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
new file mode 100644
index 00000000000000..fce325d4774c27
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -0,0 +1,46 @@
+// This test checks that a module map with a textual header can be marked as
+// non-affecting.
+
+// RUN: rm -rf %t && mkdir %t
+// RUN: split-file %s %t
+
+//--- X.modulemap
+module X { textual header "X.h" }
+//--- X.h
+typedef int X_int;
+
+//--- Y.modulemap
+module Y { textual header "Y.h" }
+//--- Y.h
+typedef int Y_int;
+
+//--- A.modulemap
+module A { header "A.h" export * }
+//--- A.h
+#include "X.h"
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A0.pcm \
+// RUN: -fmodule-map-file=%t/X.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A0 --implicit-check-not=Y.modulemap
+// A0: Input file: {{.*}}X.modulemap
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A1.pcm \
+// RUN: -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/A0.pcm | FileCheck %s --check-prefix=A1 \
+// RUN: --implicit-check-not=Y.modulemap
+// A1: Input file: {{.*}}X.modulemap
+
+// RUN:
diff %t/A0.pcm %t/A1.pcm
+
+//--- B.modulemap
+module B { header "B.h" export * }
+//--- B.h
+#include "A.h"
+typedef X_int B_int;
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B.pcm \
+// RUN: -fmodule-file=A=%t/A0.pcm \
+// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/X.modulemap -fmodule-map-file=%t/Y.modulemap
+// RUN: %clang_cc1 -fsyntax-only -module-file-info %t/B.pcm | FileCheck %s --check-prefix=B \
+// RUN: --implicit-check-not=X.modulemap --implicit-check-not=Y.modulemap
+// B: Input file: {{.*}}B.modulemap
More information about the cfe-commits
mailing list