[clang] [clang][modules] Only modulemaps of included textual headers are affecting (PR #89729)
via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 23 02:46:27 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang
Author: Sam McCall (sam-mccall)
<details>
<summary>Changes</summary>
Prior to this change, modulemaps describing textual headers are considered
to affect the current module whenever HeaderFileInfos for those headers exist.
This wastes creates false dependencies that (among other things) waste
SourceLocation space.
After this change, textual headers don't cause their modulemap to be considered
affecting, unless those textual headers are included (in which case their
modulemap is required e.g. for layering check).
---
Full diff: https://github.com/llvm/llvm-project/pull/89729.diff
5 Files Affected:
- (modified) clang/include/clang/Lex/HeaderSearch.h (+4)
- (modified) clang/lib/Lex/HeaderSearch.cpp (+4)
- (modified) clang/lib/Lex/PPDirectives.cpp (+1)
- (modified) clang/lib/Serialization/ASTWriter.cpp (+6-4)
- (added) clang/test/Modules/prune-non-affecting-module-map-files-textual.c (+47)
``````````diff
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index c5f90ef4cb3682..c24ef15005f36b 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -544,6 +544,10 @@ class HeaderSearch {
bool isImport, bool ModulesEnabled, Module *M,
bool &IsFirstIncludeOfFile);
+ /// Record that we're parsing this file as part of the current module, if any.
+ /// The header's owning modulemap is considered to affect the current module.
+ void EnteredTextualFile(FileEntryRef File);
+
/// Return whether the specified file is a normal header,
/// a system header, or a C++ friendly system header.
SrcMgr::CharacteristicKind getFileDirFlavor(FileEntryRef File) {
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 0632882b296146..84c7e09638d574 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1441,6 +1441,10 @@ void HeaderSearch::MarkFileModuleHeader(FileEntryRef FE,
HFI.isCompilingModuleHeader |= isCompilingModuleHeader;
}
+void HeaderSearch::EnteredTextualFile(FileEntryRef File) {
+ getFileInfo(File).isCompilingModuleHeader = true;
+}
+
bool HeaderSearch::ShouldEnterIncludeFile(Preprocessor &PP,
FileEntryRef File, bool isImport,
bool ModulesEnabled, Module *M,
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 0b22139ebe81de..196c7795ca05e8 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2609,6 +2609,7 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
}
assert(!IsImportDecl && "failed to diagnose missing module for import decl");
+ HeaderInfo.EnteredTextualFile(*File);
return {ImportAction::None};
}
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 8a4b36207c4734..4825c245a4b846 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,7 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
continue;
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+ if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+ !HFI->isCompilingModuleHeader))
continue;
for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -2055,11 +2056,12 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
// 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 different module; in that case, we rely on the module(s)
+ // changed since it was loaded. Also skip it if it's for a non-excluded
+ // header from a different 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))
+ if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
+ !HFI->isCompilingModuleHeader))
continue;
// Massage the file path into an appropriate form.
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..12a42c52a940b9
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -0,0 +1,47 @@
+// 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
+
``````````
</details>
https://github.com/llvm/llvm-project/pull/89729
More information about the cfe-commits
mailing list