[clang] [clang][modules] Allow module maps with textual headers to be non-affecting (PR #89441)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 23 13:08:03 PDT 2024
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/89441
>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/4] [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 17b1860b25de99f8b2b2e047ef1590b8ede6648b Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 22 Apr 2024 09:28:26 -0700
Subject: [PATCH 2/4] [clang][modules] Mark 'use'd modules as affecting
---
clang/lib/Serialization/ASTWriter.cpp | 2 ++
1 file changed, 2 insertions(+)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 4825c245a4b846..8c303fa67e3023 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -238,6 +238,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
CollectIncludingMapsFromAncestors(CurrentModule);
for (const Module *ImportedModule : CurrentModule->Imports)
CollectIncludingMapsFromAncestors(ImportedModule);
+ for (const Module *UsedModule : CurrentModule->DirectUses)
+ CollectIncludingMapsFromAncestors(UsedModule);
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
CollectIncludingMapsFromAncestors(UndeclaredModule);
}
>From 9eeea3f468432ec810a9e09696cf6a527c7db233 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 07:51:17 -0700
Subject: [PATCH 3/4] [clang][modules] Do consider module maps of included
textual headers affecting
---
clang/lib/Serialization/ASTWriter.cpp | 16 +++++++++-------
...rune-non-affecting-module-map-files-textual.c | 14 ++++----------
2 files changed, 13 insertions(+), 17 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 8c303fa67e3023..24f5c3ebbc1457 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,8 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
continue;
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
- !HFI->isCompilingModuleHeader))
+ if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
+ (HFI->isTextualModuleHeader && !PP.alreadyIncluded(*File)))
continue;
for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -2058,12 +2058,14 @@ 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 non-excluded
- // header from a different module; in that case, we rely on the module(s)
- // containing the header to provide this information.
+ // 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)
+ // containing the header to provide this information. Also skip it if it's
+ // for a textual header from a different module that has not been included;
+ // in that case, we don't need the information at all.
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || ((HFI->isModuleHeader || HFI->isTextualModuleHeader) &&
- !HFI->isCompilingModuleHeader))
+ if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
+ (HFI->isTextualModuleHeader && !PP->alreadyIncluded(*File)))
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
index 8f8f00560b1834..485e360d72bec1 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
@@ -1,5 +1,5 @@
// This test checks that a module map with a textual header can be marked as
-// non-affecting.
+// non-affecting if its header did not get included.
// RUN: rm -rf %t && mkdir %t
// RUN: split-file %s %t
@@ -7,20 +7,14 @@
//--- A.modulemap
module A { textual header "A.h" }
//--- B.modulemap
-module B { header "B.h" export * }
+module B { header "B.h" }
//--- 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: -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/B1.pcm \
-// RUN: -fmodule-map-file=%t/B.modulemap -fmodule-file=%t/A.pcm
+// RUN: -fmodule-map-file=%t/B.modulemap
// RUN: diff %t/B0.pcm %t/B1.pcm
>From ddc6bf9af7f84123a93ed9bb13138013a73456f9 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 23 Apr 2024 12:47:22 -0700
Subject: [PATCH 4/4] Reject all non-included headers from that are
!isCompilingModuleHeader, not just textual module headers. Also remove
DirectUses.
---
clang/lib/Serialization/ASTWriter.cpp | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 24f5c3ebbc1457..d10be3fca3194e 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -187,8 +187,8 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
continue;
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
- (HFI->isTextualModuleHeader && !PP.alreadyIncluded(*File)))
+ if (!HFI || (!HFI->isCompilingModuleHeader &&
+ (HFI->isModuleHeader || !PP.alreadyIncluded(*File))))
continue;
for (const auto &KH : HS.findResolvedModulesForHeader(*File)) {
@@ -238,8 +238,6 @@ GetAffectingModuleMaps(const Preprocessor &PP, Module *RootModule) {
CollectIncludingMapsFromAncestors(CurrentModule);
for (const Module *ImportedModule : CurrentModule->Imports)
CollectIncludingMapsFromAncestors(ImportedModule);
- for (const Module *UsedModule : CurrentModule->DirectUses)
- CollectIncludingMapsFromAncestors(UsedModule);
for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
CollectIncludingMapsFromAncestors(UndeclaredModule);
}
@@ -2061,11 +2059,11 @@ void ASTWriter::WriteHeaderSearch(const HeaderSearch &HS) {
// 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)
// containing the header to provide this information. Also skip it if it's
- // for a textual header from a different module that has not been included;
- // in that case, we don't need the information at all.
+ // for any header not from this module that has not been included; in that
+ // case, we don't need the information at all.
const HeaderFileInfo *HFI = HS.getExistingLocalFileInfo(*File);
- if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader) ||
- (HFI->isTextualModuleHeader && !PP->alreadyIncluded(*File)))
+ if (!HFI || (!HFI->isCompilingModuleHeader &&
+ (HFI->isModuleHeader || !PP->alreadyIncluded(*File))))
continue;
// Massage the file path into an appropriate form.
More information about the cfe-commits
mailing list