[clang] [clang][modules] Only modulemaps of included textual headers are affecting (PR #89729)
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 23 02:45:58 PDT 2024
https://github.com/sam-mccall created https://github.com/llvm/llvm-project/pull/89729
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).
>From 9165d6086e2570198fba1c333d0c9cb9c09037c7 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Fri, 19 Apr 2024 12:13:06 -0700
Subject: [PATCH 1/2] [clang][modules] Allow module map files with textual
header be non-affecting
---
clang/lib/Serialization/ASTWriter.cpp | 10 ++++---
...e-non-affecting-module-map-files-textual.c | 26 +++++++++++++++++++
2 files changed, 32 insertions(+), 4 deletions(-)
create mode 100644 clang/test/Modules/prune-non-affecting-module-map-files-textual.c
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..8f8f00560b1834
--- /dev/null
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -0,0 +1,26 @@
+// 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
+
+//--- A.modulemap
+module A { textual header "A.h" }
+//--- B.modulemap
+module B { header "B.h" export * }
+//--- A.h
+typedef int A_int;
+//--- B.h
+#include "A.h"
+typedef A_int B_int;
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A.pcm \
+// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
+// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
+// RUN: -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+
+// RUN: diff %t/B0.pcm %t/B1.pcm
>From 0bf2b1583b9b4f5fdbee2e690d075f1b41905532 Mon Sep 17 00:00:00 2001
From: Sam McCall <sam.mccall at gmail.com>
Date: Tue, 23 Apr 2024 11:04:32 +0200
Subject: [PATCH 2/2] Track textually included files, and consider their
modulemaps affecting.
Those modulemaps affect the legality of the inclusions and therefore
the module's compilation.
---
clang/include/clang/Lex/HeaderSearch.h | 4 ++
clang/lib/Lex/HeaderSearch.cpp | 4 ++
clang/lib/Lex/PPDirectives.cpp | 1 +
...e-non-affecting-module-map-files-textual.c | 47 ++++++++++++++-----
4 files changed, 43 insertions(+), 13 deletions(-)
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/test/Modules/prune-non-affecting-module-map-files-textual.c b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
index 8f8f00560b1834..12a42c52a940b9 100644
--- a/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
+++ b/clang/test/Modules/prune-non-affecting-module-map-files-textual.c
@@ -4,23 +4,44 @@
// 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 { textual header "A.h" }
+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 * }
-//--- A.h
-typedef int A_int;
//--- B.h
#include "A.h"
-typedef A_int B_int;
-
-// RUN: %clang_cc1 -fmodules -emit-module %t/A.modulemap -fmodule-name=A -o %t/A.pcm \
-// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap
-
-// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B0.pcm \
-// RUN: -fmodule-map-file=%t/A.modulemap -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+typedef X_int B_int;
-// RUN: %clang_cc1 -fmodules -emit-module %t/B.modulemap -fmodule-name=B -o %t/B1.pcm \
-// RUN: -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+// 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
-// RUN: diff %t/B0.pcm %t/B1.pcm
More information about the cfe-commits
mailing list