https://github.com/ilya-biryukov created
https://github.com/llvm/llvm-project/pull/141792
Currently, Clang picks a module where the header is non-private even when the
header is textual and the other is modular. This change makes it prefer a
module where the header is modular instead. Access checking is done separately
and checks if **any** module has access to a header, so resolving to a
different module should result in better performance with no semantics change.
In cases where the same header is used by multiple modules, this may cause
unnecessary reparses of the textual include and prohibits optimizations
removing modular inputs when `-fmodules-embed-all-files` are used.
I am changing the default behavior as having multiple modules that have the
same header is rare and so this is unlikely to break anyone in the first place,
per discussion #138227 that led to this change.
>From a03d4589f86dd01be358edb1107076730415e014 Mon Sep 17 00:00:00 2001
From: Ilya Biryukov <ibiryu...@google.com>
Date: Wed, 28 May 2025 17:46:23 +0200
Subject: [PATCH] [Module] Prefer precompiled module even when header is
private
Currently, Clang picks a module where the header is non-private even
when the header is textual and the other is modular.
This change makes it prefer a module where the header is modular
instead. Access checking is done separately and checks if **any** module
has access to a header, so resolving to a different module should result
in better performance with no semantics change.
In cases where the same header is used by multiple modules, this may
cause unnecessary reparses of the textual include and prohibits
optimizations removing modular inputs when `-fmodules-embed-all-files`
are used.
I am changing the default behavior as having multiple modules that have
the same header is rare and so this is unlikely to break anyone in the
first place, per discussion #138227 that led to this change.
---
clang/lib/Lex/ModuleMap.cpp | 10 ++--
...ple-modules-per-header-embed-all-files.cpp | 60 +++++++++++++++++++
2 files changed, 65 insertions(+), 5 deletions(-)
create mode 100644
clang/test/Modules/multiple-modules-per-header-embed-all-files.cpp
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 637a08fe4dcdb..fc0a0c1cfc873 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -572,16 +572,16 @@ static bool isBetterKnownHeader(const
ModuleMap::KnownHeader &New,
if (New.getModule()->isAvailable() && !Old.getModule()->isAvailable())
return true;
- // Prefer a public header over a private header.
- if ((New.getRole() & ModuleMap::PrivateHeader) !=
- (Old.getRole() & ModuleMap::PrivateHeader))
- return !(New.getRole() & ModuleMap::PrivateHeader);
-
// Prefer a non-textual header over a textual header.
if ((New.getRole() & ModuleMap::TextualHeader) !=
(Old.getRole() & ModuleMap::TextualHeader))
return !(New.getRole() & ModuleMap::TextualHeader);
+ // Prefer a public header over a private header.
+ if ((New.getRole() & ModuleMap::PrivateHeader) !=
+ (Old.getRole() & ModuleMap::PrivateHeader))
+ return !(New.getRole() & ModuleMap::PrivateHeader);
+
// Prefer a non-excluded header over an excluded header.
if ((New.getRole() == ModuleMap::ExcludedHeader) !=
(Old.getRole() == ModuleMap::ExcludedHeader))
diff --git a/clang/test/Modules/multiple-modules-per-header-embed-all-files.cpp
b/clang/test/Modules/multiple-modules-per-header-embed-all-files.cpp
new file mode 100644
index 0000000000000..19f1db1689667
--- /dev/null
+++ b/clang/test/Modules/multiple-modules-per-header-embed-all-files.cpp
@@ -0,0 +1,60 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+//
+// First, build a module with a header.
+//
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules1.map -fmodule-name=a
-emit-module -xc++ -fmodules-embed-all-files -o %t/a.pcm %t/modules1.map
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules1.map
-fmodule-map-file=%t/modules2.map -fmodule-name=b -emit-module \
+// RUN: -fmodule-file=%t/a.pcm -xc++ -fmodules-embed-all-files -o %t/b.pcm
%t/modules2.map
+//
+// Remove the header and check we can still build the code that finds it in a
PCM.
+//
+// RUN: rm %t/foo.h
+// RUN: %clang_cc1 -fmodules -fmodule-map-file=%t/modules2.map
-fmodule-file=%t/b.pcm -fsyntax-only %t/use.cpp
+
+//--- modules1.map
+module a {
+ module foo {
+ header "foo.h"
+ export *
+ }
+ export *
+}
+
+//--- modules2.map
+module all_textual {
+ module foo {
+ textual header "foo.h"
+ export *
+ }
+ module wrap_foo {
+ textual header "wrap_foo.h"
+ export *
+ }
+ export *
+}
+
+module b {
+ module wrap_foo {
+ private header "wrap_foo.h"
+ export *
+ }
+ export *
+}
+
+
+//--- foo.h
+#ifndef FOO_H
+#define FOO_H
+void foo();
+#endif
+
+//--- wrap_foo.h
+#include "foo.h"
+
+//--- use.cpp
+#include "wrap_foo.h"
+
+void test() {
+ foo();
+}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
