[clang] [clang][modules] Track Included Files Per Submodule (PR #170215)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Fri Mar 6 14:00:28 PST 2026
https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/170215
>From c27d30ead152fba032870cdf53e3988e2aef102f Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Thu, 2 Nov 2023 14:35:30 -0700
Subject: [PATCH 1/4] [clang][modules] Track included files per submodule
(cherry picked from commit 9debc58d5135fbde51967dfb076d0ec5d954df38)
Conflicts:
clang/include/clang/Lex/Preprocessor.h
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTReaderInternals.h
clang/lib/Serialization/ASTWriter.cpp
---
clang/include/clang/Basic/Module.h | 2 +
clang/include/clang/Lex/Preprocessor.h | 38 ++++++++++++---
clang/lib/Serialization/ASTReader.cpp | 33 ++++++++-----
clang/lib/Serialization/ASTWriter.cpp | 46 ++++++++++++++----
clang/test/Modules/per-submodule-includes.m | 53 +++++++++++++++++++++
5 files changed, 145 insertions(+), 27 deletions(-)
create mode 100644 clang/test/Modules/per-submodule-includes.m
diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h
index 69a1de6f79b35..02552ed221036 100644
--- a/clang/include/clang/Basic/Module.h
+++ b/clang/include/clang/Basic/Module.h
@@ -498,6 +498,8 @@ class alignas(8) Module {
/// to import but didn't because they are not direct uses.
llvm::SmallSetVector<const Module *, 2> UndeclaredUses;
+ llvm::DenseSet<const FileEntry *> Includes;
+
/// A library or framework to link against when an entity from this
/// module is used.
struct LinkLibrary {
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index b6e42a6151ac3..66a18dd3f24b6 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1058,6 +1058,8 @@ class Preprocessor {
/// The set of modules that are visible within the submodule.
VisibleModuleSet VisibleModules;
+ /// The files that have been included.
+ IncludedFilesSet IncludedFiles;
// FIXME: CounterValue?
// FIXME: PragmaPushMacroInfo?
};
@@ -1070,8 +1072,8 @@ class Preprocessor {
/// in a submodule.
SubmoduleState *CurSubmoduleState;
- /// The files that have been included.
- IncludedFilesSet IncludedFiles;
+ /// The files that have been included outside of (sub)modules.
+ IncludedFilesSet Includes;
/// The set of top-level modules that affected preprocessing, but were not
/// imported.
@@ -1556,19 +1558,41 @@ class Preprocessor {
/// Mark the file as included.
/// Returns true if this is the first time the file was included.
bool markIncluded(FileEntryRef File) {
+ bool AlreadyIncluded = alreadyIncluded(File);
HeaderInfo.getFileInfo(File).IsLocallyIncluded = true;
- return IncludedFiles.insert(File).second;
+ CurSubmoduleState->IncludedFiles.insert(File);
+ if (!BuildingSubmoduleStack.empty())
+ BuildingSubmoduleStack.back().M->Includes.insert(File);
+ else if (Module *M = getCurrentModule())
+ M->Includes.insert(File);
+ else
+ Includes.insert(File);
+ return !AlreadyIncluded;
}
/// Return true if this header has already been included.
bool alreadyIncluded(FileEntryRef File) const {
HeaderInfo.getFileInfo(File);
- return IncludedFiles.count(File);
+ if (CurSubmoduleState->IncludedFiles.contains(File))
+ return true;
+ // TODO: Do this more efficiently.
+ for (const auto &[Name, M] : HeaderInfo.getModuleMap().modules())
+ if (CurSubmoduleState->VisibleModules.isVisible(M))
+ if (M->Includes.contains(File))
+ return true;
+ return false;
+ }
+
+ void markIncludedOnTopLevel(const FileEntry *File) {
+ Includes.insert(File);
+ CurSubmoduleState->IncludedFiles.insert(File);
+ }
+
+ void markIncludedInModule(Module *M, const FileEntry *File) {
+ M->Includes.insert(File);
}
- /// Get the set of included files.
- IncludedFilesSet &getIncludedFiles() { return IncludedFiles; }
- const IncludedFilesSet &getIncludedFiles() const { return IncludedFiles; }
+ const IncludedFilesSet &getTopLevelIncludes() const { return Includes; }
/// Return the name of the macro defined before \p Loc that has
/// spelling \p Tokens. If there are multiple macros with same spelling,
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index bde000234a062..b738743e92249 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2374,14 +2374,6 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
HeaderFileInfo HFI;
unsigned Flags = *d++;
- OptionalFileEntryRef FE;
- bool Included = (Flags >> 6) & 0x01;
- if (Included)
- if ((FE = getFile(key)))
- // Not using \c Preprocessor::markIncluded(), since that would attempt to
- // deserialize this header file info again.
- Reader.getPreprocessor().getIncludedFiles().insert(*FE);
-
// FIXME: Refactor with mergeHeaderFileInfo in HeaderSearch.cpp.
HFI.isImport |= (Flags >> 5) & 0x01;
HFI.isPragmaOnce |= (Flags >> 4) & 0x01;
@@ -2389,6 +2381,27 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
HFI.LazyControllingMacro = Reader.getGlobalIdentifierID(
M, endian::readNext<IdentifierID, llvm::endianness::little>(d));
+ auto FE = getFile(key);
+ Preprocessor &PP = Reader.getPreprocessor();
+ ModuleMap &ModMap = PP.getHeaderSearchInfo().getModuleMap();
+
+ unsigned IncludedCount =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(d);
+ for (unsigned I = 0; I < IncludedCount; ++I) {
+ uint32_t LocalSMID =
+ endian::readNext<uint32_t, llvm::endianness::little, unaligned>(d);
+ if (!FE)
+ continue;
+
+ if (LocalSMID == 0) {
+ PP.markIncludedOnTopLevel(*FE);
+ } else {
+ SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
+ Module *Mod = Reader.getSubmodule(GlobalSMID);
+ PP.markIncludedInModule(Mod, *FE);
+ }
+ }
+
assert((End - d) % 4 == 0 &&
"Wrong data length in HeaderFileInfo deserialization");
while (d != End) {
@@ -2401,10 +2414,8 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
// implicit module import.
SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
Module *Mod = Reader.getSubmodule(GlobalSMID);
- ModuleMap &ModMap =
- Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- if (FE || (FE = getFile(key))) {
+ if (FE) {
// FIXME: NameAsWritten
Module::Header H = {std::string(key.Filename), "", *FE};
ModMap.addHeader(Mod, H, HeaderRole, /*Imported=*/true);
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index af46f84d5aac0..046875b4c2145 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2098,14 +2098,15 @@ namespace {
llvm::PointerIntPair<Module *, 2, ModuleMap::ModuleHeaderRole>;
struct data_type {
- data_type(const HeaderFileInfo &HFI, bool AlreadyIncluded,
+ data_type(const HeaderFileInfo &HFI,
+ const std::vector<const Module *> &Includers,
ArrayRef<ModuleMap::KnownHeader> KnownHeaders,
UnresolvedModule Unresolved)
- : HFI(HFI), AlreadyIncluded(AlreadyIncluded),
- KnownHeaders(KnownHeaders), Unresolved(Unresolved) {}
+ : HFI(HFI), Includers(Includers), KnownHeaders(KnownHeaders),
+ Unresolved(Unresolved) {}
HeaderFileInfo HFI;
- bool AlreadyIncluded;
+ std::vector<const Module *> Includers;
SmallVector<ModuleMap::KnownHeader, 1> KnownHeaders;
UnresolvedModule Unresolved;
};
@@ -2128,6 +2129,12 @@ namespace {
EmitKeyDataLength(raw_ostream& Out, key_type_ref key, data_type_ref Data) {
unsigned KeyLen = key.Filename.size() + 1 + 8 + 8;
unsigned DataLen = 1 + sizeof(IdentifierID);
+
+ DataLen += 4;
+ for (const Module *M : Data.Includers)
+ if (!M || Writer.getLocalOrImportedSubmoduleID(M))
+ DataLen += 4;
+
for (auto ModInfo : Data.KnownHeaders)
if (Writer.getLocalOrImportedSubmoduleID(ModInfo.getModule()))
DataLen += 4;
@@ -2154,8 +2161,7 @@ namespace {
endian::Writer LE(Out, llvm::endianness::little);
uint64_t Start = Out.tell(); (void)Start;
- unsigned char Flags = (Data.AlreadyIncluded << 6)
- | (Data.HFI.isImport << 5)
+ unsigned char Flags = (Data.HFI.isImport << 5)
| (Writer.isWritingStdCXXNamedModules() ? 0 :
Data.HFI.isPragmaOnce << 4)
| (Data.HFI.DirInfo << 1);
@@ -2167,6 +2173,14 @@ namespace {
LE.write<IdentifierID>(
Writer.getIdentifierRef(Data.HFI.LazyControllingMacro.getPtr()));
+ LE.write<uint32_t>(Data.Includers.size());
+ for (const Module *M : Data.Includers) {
+ if (!M)
+ LE.write<uint32_t>(0);
+ else if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M))
+ LE.write<uint32_t>(ModID);
+ }
+
auto EmitModule = [&](Module *M, ModuleMap::ModuleHeaderRole Role) {
if (uint32_t ModID = Writer.getLocalOrImportedSubmoduleID(M)) {
uint32_t Value = (ModID << 3) | (unsigned)Role;
@@ -2238,7 +2252,7 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
HeaderFileInfoTrait::key_type Key = {
FilenameDup, *U.Size, IncludeTimestamps ? *U.ModTime : 0};
HeaderFileInfoTrait::data_type Data = {
- Empty, false, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
+ Empty, {}, {}, {M, ModuleMap::headerKindToRole(U.Kind)}};
// FIXME: Deal with cases where there are multiple unresolved header
// directives in different submodules for the same header.
Generator.insert(Key, Data, GeneratorTrait);
@@ -2278,13 +2292,27 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
SavedStrings.push_back(Filename.data());
}
- bool Included = HFI->IsLocallyIncluded || PP->alreadyIncluded(*File);
+ std::vector<const Module *> Includers;
+ if (WritingModule) {
+ llvm::DenseSet<const Module *> Seen;
+ std::function<void(const Module *)> Visit = [&](const Module *M) {
+ if (!Seen.insert(M).second)
+ return;
+ if (M->Includes.contains(*File))
+ Includers.push_back(M);
+ for (const Module *SubM : M->submodules())
+ Visit(SubM);
+ };
+ Visit(WritingModule);
+ } else if (PP->getTopLevelIncludes().contains(*File)) {
+ Includers.push_back(nullptr);
+ }
HeaderFileInfoTrait::key_type Key = {
Filename, File->getSize(), getTimestampForOutput(*File)
};
HeaderFileInfoTrait::data_type Data = {
- *HFI, Included, HS.getModuleMap().findResolvedModulesForHeader(*File), {}
+ *HFI, Includers, HS.getModuleMap().findResolvedModulesForHeader(*File), {}
};
Generator.insert(Key, Data, GeneratorTrait);
++NumHeaderSearchEntries;
diff --git a/clang/test/Modules/per-submodule-includes.m b/clang/test/Modules/per-submodule-includes.m
new file mode 100644
index 0000000000000..39fd255e8e93e
--- /dev/null
+++ b/clang/test/Modules/per-submodule-includes.m
@@ -0,0 +1,53 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- frameworks/Textual.framework/Headers/Header.h
+static int symbol;
+
+//--- frameworks/FW.framework/Modules/module.modulemap
+framework module FW {
+ umbrella header "FW.h"
+ export *
+ module * { export * }
+}
+//--- frameworks/FW.framework/Headers/FW.h
+#import <FW/Sub1.h>
+#import <FW/Sub2.h>
+//--- frameworks/FW.framework/Headers/Sub1.h
+//--- frameworks/FW.framework/Headers/Sub2.h
+#import <Textual/Header.h>
+
+//--- pch.modulemap
+module __PCH {
+ header "pch.h"
+ export *
+}
+//--- pch.h
+#import <FW/Sub1.h>
+
+//--- tu.m
+#import <Textual/Header.h>
+int fn() { return symbol; }
+
+// Compilation using the PCH regularly succeeds. The import of FW/Sub1.h in the
+// PCH is treated textually due to -fmodule-name=FW.
+//
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps -F %t/frameworks -fmodule-name=FW \
+// RUN: -emit-pch -x objective-c %t/pch.h -o %t/pch.h.gch
+//
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps -F %t/frameworks -fmodule-name=FW \
+// RUN: -include-pch %t/pch.h.gch -fsyntax-only %t/tu.m
+
+// Compilation using the PCH as precompiled module fails. The import of FW/Sub1.h
+// in the PCH is translated to an import. Nothing is preventing that now that
+// -fmodule-name=FW has been replaced with -fmodule-name=__PCH.
+//
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps -F %t/frameworks \
+// RUN: -emit-module -fmodule-name=__PCH -x objective-c %t/pch.modulemap -o %t/pch.h.pcm
+//
+// Loading FW.pcm marks Textual/Header.h as imported (because it is imported in
+// FW.Sub2), so the TU does not import it again. It's contents remain invisible,
+// though.
+//
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fimplicit-module-maps -F %t/frameworks \
+// RUN: -include %t/pch.h -fmodule-map-file=%t/pch.modulemap -fsyntax-only %t/tu.m
>From feb55219a106d93395322f25bb7612269d3567b3 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Mon, 1 Dec 2025 14:07:07 -0800
Subject: [PATCH 2/4] Move the included vector into
HeaderFileInfoTrait::data_type and add a test case.
---
clang/lib/Serialization/ASTWriter.cpp | 21 +++---
.../Modules/import-submodule-visibility.c | 64 +++++++++++++++++++
2 files changed, 76 insertions(+), 9 deletions(-)
create mode 100644 clang/test/Modules/import-submodule-visibility.c
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 046875b4c2145..868d4da70b778 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2099,11 +2099,11 @@ namespace {
struct data_type {
data_type(const HeaderFileInfo &HFI,
- const std::vector<const Module *> &Includers,
+ std::vector<const Module *> &&Includers,
ArrayRef<ModuleMap::KnownHeader> KnownHeaders,
UnresolvedModule Unresolved)
- : HFI(HFI), Includers(Includers), KnownHeaders(KnownHeaders),
- Unresolved(Unresolved) {}
+ : HFI(HFI), Includers(std::move(Includers)),
+ KnownHeaders(KnownHeaders), Unresolved(Unresolved) {}
HeaderFileInfo HFI;
std::vector<const Module *> Includers;
@@ -2161,10 +2161,11 @@ namespace {
endian::Writer LE(Out, llvm::endianness::little);
uint64_t Start = Out.tell(); (void)Start;
- unsigned char Flags = (Data.HFI.isImport << 5)
- | (Writer.isWritingStdCXXNamedModules() ? 0 :
- Data.HFI.isPragmaOnce << 4)
- | (Data.HFI.DirInfo << 1);
+ unsigned char Flags =
+ (Data.HFI.isImport << 5) |
+ (Writer.isWritingStdCXXNamedModules() ? 0
+ : Data.HFI.isPragmaOnce << 4) |
+ (Data.HFI.DirInfo << 1);
LE.write<uint8_t>(Flags);
if (Data.HFI.LazyControllingMacro.isID())
@@ -2312,8 +2313,10 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
Filename, File->getSize(), getTimestampForOutput(*File)
};
HeaderFileInfoTrait::data_type Data = {
- *HFI, Includers, HS.getModuleMap().findResolvedModulesForHeader(*File), {}
- };
+ *HFI,
+ std::move(Includers),
+ HS.getModuleMap().findResolvedModulesForHeader(*File),
+ {}};
Generator.insert(Key, Data, GeneratorTrait);
++NumHeaderSearchEntries;
}
diff --git a/clang/test/Modules/import-submodule-visibility.c b/clang/test/Modules/import-submodule-visibility.c
new file mode 100644
index 0000000000000..3491494ea7cc0
--- /dev/null
+++ b/clang/test/Modules/import-submodule-visibility.c
@@ -0,0 +1,64 @@
+// This test checks that imports of headers that appeared in a different submodule than
+// what is imported by the current TU don't affect the compilation.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- C/C.h
+#include "Textual.h"
+//--- C/module.modulemap
+module C { header "C.h" }
+
+//--- D/D1.h
+#include "Textual.h"
+//--- D/D2.h
+//--- D/module.modulemap
+module D {
+ module D1 { header "D1.h" }
+ module D2 { header "D2.h" }
+}
+
+//--- E/E1.h
+#include "E2.h"
+//--- E/E2.h
+#include "Textual.h"
+//--- E/module.modulemap
+module E {
+ module E1 { header "E1.h" }
+ module E2 { header "E2.h" }
+}
+
+//--- Textual.h
+#define MACRO_TEXTUAL 1
+
+//--- test_top.c
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+//--- test_sub.c
+#import "D/D2.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+//--- test_transitive.c
+#import "E/E1.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// Simply loading a PCM file containing top-level module including a header does
+// not prevent inclusion of that header in the TU.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/C/module.modulemap -fmodule-name=C -o %t/C.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_top.c -fmodule-file=%t/C.pcm
+
+// Loading a PCM file and importing its empty submodule does not prevent
+// inclusion of headers included by invisible sibling submodules.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/D/module.modulemap -fmodule-name=D -o %t/D.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_sub.c -fmodule-file=%t/D.pcm
+
+// Loading a PCM file and importing a submodule does not prevent inclusion of
+// headers included by some of its transitive un-exported dependencies.
+//
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/E/module.modulemap -fmodule-name=E -o %t/E.pcm
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_transitive.c -fmodule-file=%t/E.pcm
>From ede8c85444c2b38d47de2966b271dcd31aed0ff8 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Sat, 13 Dec 2025 14:55:36 -0800
Subject: [PATCH 3/4] Avoid looping in Preprocessor::alreadyIncluded.
---
clang/include/clang/Lex/Preprocessor.h | 25 +++++++++++++------------
clang/lib/Lex/Preprocessor.cpp | 2 ++
clang/lib/Serialization/ASTReader.cpp | 1 +
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 66a18dd3f24b6..739e01748efa2 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -1058,8 +1058,10 @@ class Preprocessor {
/// The set of modules that are visible within the submodule.
VisibleModuleSet VisibleModules;
- /// The files that have been included.
+ /// The files that have been included. This set includes the headers
+ /// that have been included and visible in this submodule.
IncludedFilesSet IncludedFiles;
+
// FIXME: CounterValue?
// FIXME: PragmaPushMacroInfo?
};
@@ -1561,26 +1563,22 @@ class Preprocessor {
bool AlreadyIncluded = alreadyIncluded(File);
HeaderInfo.getFileInfo(File).IsLocallyIncluded = true;
CurSubmoduleState->IncludedFiles.insert(File);
- if (!BuildingSubmoduleStack.empty())
- BuildingSubmoduleStack.back().M->Includes.insert(File);
- else if (Module *M = getCurrentModule())
+
+ Module *M = BuildingSubmoduleStack.empty()
+ ? getCurrentModule()
+ : BuildingSubmoduleStack.back().M;
+ if (M)
M->Includes.insert(File);
else
Includes.insert(File);
+
return !AlreadyIncluded;
}
/// Return true if this header has already been included.
bool alreadyIncluded(FileEntryRef File) const {
HeaderInfo.getFileInfo(File);
- if (CurSubmoduleState->IncludedFiles.contains(File))
- return true;
- // TODO: Do this more efficiently.
- for (const auto &[Name, M] : HeaderInfo.getModuleMap().modules())
- if (CurSubmoduleState->VisibleModules.isVisible(M))
- if (M->Includes.contains(File))
- return true;
- return false;
+ return CurSubmoduleState->IncludedFiles.contains(File);
}
void markIncludedOnTopLevel(const FileEntry *File) {
@@ -1590,6 +1588,9 @@ class Preprocessor {
void markIncludedInModule(Module *M, const FileEntry *File) {
M->Includes.insert(File);
+
+ if (CurSubmoduleState->VisibleModules.isVisible(M))
+ CurSubmoduleState->IncludedFiles.insert(File);
}
const IncludedFilesSet &getTopLevelIncludes() const { return Includes; }
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index a531f51408dae..efefd4e41d98e 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -1466,6 +1466,8 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc,
<< Message;
});
+ CurSubmoduleState->IncludedFiles.insert_range(M->Includes);
+
// Add this module to the imports list of the currently-built submodule.
if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M)
BuildingSubmoduleStack.back().M->Imports.insert(M);
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index b738743e92249..5a92f8da4b95b 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2414,6 +2414,7 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
// implicit module import.
SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
Module *Mod = Reader.getSubmodule(GlobalSMID);
+ PP.markIncludedInModule(Mod, *FE);
if (FE) {
// FIXME: NameAsWritten
>From 9d0b8af34dad3c08c709cf240f0597159ff98b78 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Fri, 6 Mar 2026 14:00:10 -0800
Subject: [PATCH 4/4] Adding two tests, and removing dead code.
---
clang/lib/Lex/Preprocessor.cpp | 5 +-
clang/lib/Serialization/ASTReader.cpp | 2 -
.../import-textual-reenter-not-visible.c | 156 +++++++++++++++
.../Modules/import-textual-skip-visible.c | 186 ++++++++++++++++++
4 files changed, 343 insertions(+), 6 deletions(-)
create mode 100644 clang/test/Modules/import-textual-reenter-not-visible.c
create mode 100644 clang/test/Modules/import-textual-skip-visible.c
diff --git a/clang/lib/Lex/Preprocessor.cpp b/clang/lib/Lex/Preprocessor.cpp
index efefd4e41d98e..b5b9fe7d307f5 100644
--- a/clang/lib/Lex/Preprocessor.cpp
+++ b/clang/lib/Lex/Preprocessor.cpp
@@ -1461,13 +1461,10 @@ void Preprocessor::makeModuleVisible(Module *M, SourceLocation Loc,
// FIXME: Include the path in the diagnostic.
// FIXME: Include the import location for the conflicting module.
Diag(ModuleImportLoc, diag::warn_module_conflict)
- << Path[0]->getFullModuleName()
- << Conflict->getFullModuleName()
+ << Path[0]->getFullModuleName() << Conflict->getFullModuleName()
<< Message;
});
- CurSubmoduleState->IncludedFiles.insert_range(M->Includes);
-
// Add this module to the imports list of the currently-built submodule.
if (!BuildingSubmoduleStack.empty() && M != BuildingSubmoduleStack.back().M)
BuildingSubmoduleStack.back().M->Imports.insert(M);
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 5a92f8da4b95b..5e095d6c496a1 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -2414,8 +2414,6 @@ HeaderFileInfoTrait::ReadData(internal_key_ref key, const unsigned char *d,
// implicit module import.
SubmoduleID GlobalSMID = Reader.getGlobalSubmoduleID(M, LocalSMID);
Module *Mod = Reader.getSubmodule(GlobalSMID);
- PP.markIncludedInModule(Mod, *FE);
-
if (FE) {
// FIXME: NameAsWritten
Module::Header H = {std::string(key.Filename), "", *FE};
diff --git a/clang/test/Modules/import-textual-reenter-not-visible.c b/clang/test/Modules/import-textual-reenter-not-visible.c
new file mode 100644
index 0000000000000..9254283fc43f2
--- /dev/null
+++ b/clang/test/Modules/import-textual-reenter-not-visible.c
@@ -0,0 +1,156 @@
+// Test that #import can re-enter a textual header when the header was included
+// by a module that is reachable but NOT visible. The key invariant:
+//
+// If a textual header was included by module M, and M is NOT visible in the
+// current TU, then #import of that header MUST re-enter the file (not skip it).
+//
+// This is the complement of import-textual-skip-visible.c, which verifies
+// the opposite: visible modules DO suppress re-entry. Here we verify that
+// non-visible modules do NOT suppress re-entry, even when the PCM containing
+// the header info has been loaded.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- Textual.h
+#define MACRO_TEXTUAL 1
+
+// =========================================================================
+// Module E: E1 includes E2, E2 includes Textual.h. NO export *.
+// =========================================================================
+//--- E/E1.h
+#include "E2.h"
+//--- E/E2.h
+#include "Textual.h"
+//--- E/module.modulemap
+module E {
+ module E1 { header "E1.h" }
+ module E2 { header "E2.h" }
+}
+
+// =========================================================================
+// Module F: F1 imports E1 (cross-module via -fmodule-file). NO export *.
+// =========================================================================
+//--- F/F1.h
+#include "E/E1.h"
+//--- F/module.modulemap
+module F {
+ module F1 { header "F1.h" }
+}
+
+// =========================================================================
+// Module G: G1 imports E1 with export *, but E1 itself does NOT export E2.
+// So importing G1 makes E1 visible (via G1's export *), but E2 is still not
+// visible because E1 has no export *.
+// =========================================================================
+//--- G/G1.h
+#include "E/E1.h"
+//--- G/module.modulemap
+module G {
+ module G1 {
+ header "G1.h"
+ export *
+ }
+}
+
+// =========================================================================
+// Module H: H1 imports E1, H2 is empty. Import H2 only — E2 is not visible.
+// =========================================================================
+//--- H/H1.h
+#include "E/E1.h"
+//--- H/H2.h
+// empty
+//--- H/module.modulemap
+module H {
+ module H1 { header "H1.h" }
+ module H2 { header "H2.h" }
+}
+
+// =========================================================================
+// Module J: another independent module that also includes Textual.h.
+// =========================================================================
+//--- J/J1.h
+#include "Textual.h"
+//--- J/module.modulemap
+module J {
+ module J1 { header "J1.h" }
+}
+
+// =========================================================================
+// Test 1: Cross-module, no export.
+// Import F1, which imports E1 (no export *). E2 included Textual.h but is
+// NOT visible. #import Textual.h must re-enter the file.
+// =========================================================================
+//--- test_cross_no_export.c
+#import "F/F1.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 2: Cross-module, partial export chain.
+// Import G1 (export *) → E1 (no export *). E1 is visible but E2 is not.
+// Textual.h was included by E2 — must be re-enterable.
+// =========================================================================
+//--- test_partial_export.c
+#import "G/G1.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 3: Import a sibling; the other sibling's transitive deps included
+// Textual.h. Import H2 only — H1 (and its dep E1, E2) are not visible.
+// Textual.h must be re-enterable.
+// =========================================================================
+//--- test_sibling_invisible.c
+#import "H/H2.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 4: Multiple PCMs loaded, none with Textual.h visible.
+// Load both E.pcm and J.pcm. Import neither E2 nor J1.
+// Textual.h was included by both E2 and J1, but neither is visible.
+// Must be re-enterable.
+// =========================================================================
+//--- test_multiple_pcms.c
+// Don't import anything — just load PCMs via -fmodule-file.
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 5: Cross-module transitive, no export at any level.
+// Import F1 → E1 → E2 → Textual.h. Neither F1 nor E1 has export *.
+// E2 is not visible. Textual.h must be re-enterable.
+// =========================================================================
+//--- test_deep_no_export.c
+#import "F/F1.h"
+#import "Textual.h"
+static int x = MACRO_TEXTUAL;
+
+// =========================================================================
+// Build PCMs.
+// =========================================================================
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/E/module.modulemap -fmodule-name=E -o %t/E.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/F/module.modulemap -fmodule-name=F -fmodule-file=%t/E.pcm -o %t/F.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/G/module.modulemap -fmodule-name=G -fmodule-file=%t/E.pcm -o %t/G.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/H/module.modulemap -fmodule-name=H -fmodule-file=%t/E.pcm -o %t/H.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/J/module.modulemap -fmodule-name=J -o %t/J.pcm
+
+// =========================================================================
+// Run tests.
+// =========================================================================
+
+// Test 1: cross-module, no export — Textual.h must be re-entered.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_cross_no_export.c -fmodule-file=%t/F.pcm
+
+// Test 2: partial export chain — Textual.h must be re-entered.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_partial_export.c -fmodule-file=%t/G.pcm
+
+// Test 3: sibling invisible — Textual.h must be re-entered.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_sibling_invisible.c -fmodule-file=%t/H.pcm
+
+// Test 4: multiple PCMs, none visible — Textual.h must be re-entered.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_multiple_pcms.c -fmodule-file=%t/E.pcm -fmodule-file=%t/J.pcm
+
+// Test 5: deep transitive, no export — Textual.h must be re-entered.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_deep_no_export.c -fmodule-file=%t/F.pcm
diff --git a/clang/test/Modules/import-textual-skip-visible.c b/clang/test/Modules/import-textual-skip-visible.c
new file mode 100644
index 0000000000000..c107472128dbd
--- /dev/null
+++ b/clang/test/Modules/import-textual-skip-visible.c
@@ -0,0 +1,186 @@
+// Test that #import does NOT re-enter a textual header when the header was
+// included by a module that IS visible. The key invariant:
+//
+// If a textual header was included by module M, and M is visible in the
+// current TU (directly or via transitive export), then #import of that
+// header MUST be skipped (the file is already included).
+//
+// This is the complement of import-textual-reenter-not-visible.c, which
+// verifies the opposite: non-visible modules do NOT suppress re-entry.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// Textual.h has no include guard. If #import incorrectly re-enters the file,
+// the duplicate typedef will produce a compilation error.
+//--- Textual.h
+typedef int textual_type_t;
+#define MACRO_TEXTUAL 1
+
+// =========================================================================
+// Module E: E1 includes E2, E2 includes Textual.h. E1 has export *.
+// =========================================================================
+//--- E/E1.h
+#include "E2.h"
+//--- E/E2.h
+#include "Textual.h"
+//--- E/module.modulemap
+module E {
+ module E1 {
+ header "E1.h"
+ export *
+ }
+ module E2 { header "E2.h" }
+}
+
+// =========================================================================
+// Module F: F1 imports E1 cross-module. F1 has export *.
+// =========================================================================
+//--- F/F1.h
+#include "E/E1.h"
+//--- F/module.modulemap
+module F {
+ module F1 {
+ header "F1.h"
+ export *
+ }
+}
+
+// =========================================================================
+// Module G: G1 imports F1 cross-module. G1 has export *.
+// =========================================================================
+//--- G/G1.h
+#include "F/F1.h"
+//--- G/module.modulemap
+module G {
+ module G1 {
+ header "G1.h"
+ export *
+ }
+}
+
+// =========================================================================
+// Module A: A1 independently includes Textual.h (for early deserialization).
+// =========================================================================
+//--- A/A1.h
+#include "Textual.h"
+//--- A/module.modulemap
+module A {
+ module A1 { header "A1.h" }
+}
+
+// =========================================================================
+// Test 1: Sibling re-export within a single module.
+// Import E1 (export *) → E2 becomes visible → Textual.h's macro should
+// be available as a module macro without an explicit #import.
+// =========================================================================
+//--- test_sibling_reexport.c
+#import "E/E1.h"
+#ifdef MACRO_TEXTUAL
+textual_type_t val = MACRO_TEXTUAL;
+#else
+#error "MACRO_TEXTUAL should be visible via E1's re-export of E2"
+#endif
+
+// =========================================================================
+// Test 2: Cross-module transitive re-export (depth 2).
+// Import F1 (export *) → E1 (export *) → E2 visible. Macro should be
+// available without explicit #import.
+// =========================================================================
+//--- test_cross_depth2.c
+#import "F/F1.h"
+#ifdef MACRO_TEXTUAL
+textual_type_t val = MACRO_TEXTUAL;
+#else
+#error "MACRO_TEXTUAL should be visible via F1 -> E1 -> E2 export chain"
+#endif
+
+// =========================================================================
+// Test 3: Cross-module transitive re-export (depth 3).
+// Import G1 (export *) → F1 (export *) → E1 (export *) → E2 visible.
+// =========================================================================
+//--- test_cross_depth3.c
+#import "G/G1.h"
+#ifdef MACRO_TEXTUAL
+textual_type_t val = MACRO_TEXTUAL;
+#else
+#error "MACRO_TEXTUAL should be visible via G1 -> F1 -> E1 -> E2 export chain"
+#endif
+
+// =========================================================================
+// Test 4: Explicit #import of already-visible textual header is suppressed.
+// Import E1 (export *) → E2 visible → Textual.h already included.
+// A subsequent #import Textual.h should be a no-op. The typedef in
+// Textual.h (no include guard) would cause a duplicate definition error
+// if the file were incorrectly re-entered.
+// =========================================================================
+//--- test_import_suppressed.c
+#import "E/E1.h"
+#import "Textual.h"
+textual_type_t val = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 5: Cross-module #import suppression (depth 2).
+// Import F1 (export * chain) → E2 visible. Explicit #import of Textual.h
+// must be suppressed.
+// =========================================================================
+//--- test_import_suppressed_depth2.c
+#import "F/F1.h"
+#import "Textual.h"
+textual_type_t val = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 6: Cross-module #import suppression (depth 3).
+// Import G1 (export * chain) → E2 visible. Explicit #import of Textual.h
+// must be suppressed.
+// =========================================================================
+//--- test_import_suppressed_depth3.c
+#import "G/G1.h"
+#import "Textual.h"
+textual_type_t val = MACRO_TEXTUAL;
+
+// =========================================================================
+// Test 7: Early deserialization — import a different module that also
+// includes Textual.h BEFORE importing E1. This forces Textual.h's header
+// info to be deserialized (via markIncludedInModule) at a time when E2 is
+// NOT yet visible. Then import E1 (export *) → E2 becomes visible.
+// The subsequent #import Textual.h must still be suppressed.
+// =========================================================================
+//--- test_early_deser.c
+#import "A/A1.h"
+#import "E/E1.h"
+#import "Textual.h"
+textual_type_t val = MACRO_TEXTUAL;
+
+// =========================================================================
+// Build PCMs.
+// =========================================================================
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/E/module.modulemap -fmodule-name=E -o %t/E.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/F/module.modulemap -fmodule-name=F -fmodule-file=%t/E.pcm -o %t/F.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/G/module.modulemap -fmodule-name=G -fmodule-file=%t/F.pcm -o %t/G.pcm
+// RUN: %clang_cc1 -fmodules -I %t -emit-module %t/A/module.modulemap -fmodule-name=A -o %t/A.pcm
+
+// =========================================================================
+// Run tests.
+// =========================================================================
+
+// Test 1: sibling re-export — macro visible without #import.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_sibling_reexport.c -fmodule-file=%t/E.pcm
+
+// Test 2: depth-2 cross-module re-export — macro visible.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_cross_depth2.c -fmodule-file=%t/F.pcm
+
+// Test 3: depth-3 cross-module re-export — macro visible.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_cross_depth3.c -fmodule-file=%t/G.pcm
+
+// Test 4: explicit #import suppressed (single module).
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_import_suppressed.c -fmodule-file=%t/E.pcm
+
+// Test 5: explicit #import suppressed (depth 2).
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_import_suppressed_depth2.c -fmodule-file=%t/F.pcm
+
+// Test 6: explicit #import suppressed (depth 3).
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_import_suppressed_depth3.c -fmodule-file=%t/G.pcm
+
+// Test 7: early deserialization — #import suppressed despite prior deser.
+// RUN: %clang_cc1 -fmodules -I %t -fsyntax-only %t/test_early_deser.c -fmodule-file=%t/A.pcm -fmodule-file=%t/E.pcm
More information about the cfe-commits
mailing list