On 3/7/25 6:19 AM, Nathaniel Shead wrote:
On Thu, Mar 06, 2025 at 12:45:33PM -0500, Jason Merrill wrote:
On 3/5/25 7:54 AM, Nathaniel Shead wrote:
On Wed, Feb 26, 2025 at 10:29:59AM -0500, Jason Merrill wrote:
On 2/21/25 6:05 AM, Nathaniel Shead wrote:
After seeing PR c++/118964 I'm coming back around to this [1] patch
series, since it appears that this can cause errors on otherwise valid
code by instantiations coming into module purview that reference
TU-local entities.
[1]: https://gcc.gnu.org/pipermail/gcc-patches/2024-May/650324.html
We'd previously discussed that the best way to solve this in general
would be to perform all deferred instantiations at the end of the GMF to
ensure that they would not leak into the module purview. I still
tentatively agree that this would be the "nicer" way to go (though I've
since come across https://eel.is/c++draft/temp.point#7 and wonder if
this may not be strictly conforming according to the current wording?).
Hmm, interesting point.
I've not yet managed to implement this however, and even if I had, at
this stage it would definitely not be appropriate for GCC15; would the
approach described below be appropriate for GCC15 as a stop-gap to
reduce these issues?
As I suggested earlier in this discussion, it seems to me that the
purviewness of an implicit instantiation shouldn't matter, they should all
be treated as discardable.
So looking further I believe that currently they are not, in that
implicit instantiations themselves are only ever emitted if they are
reached from elsewhere (regardless of their purviewness), so the issues
are mostly just with deferred explicit instantiations etc. However...
Could we instead set DECL_MODULE_PURVIEW_P as appropriate when we see an
explicit instantiation and use that + DECL_EXPLICIT_INSTANTIATION to
recompute module_kind?
As in the attached, which works for me. Do you see a problem with this
approach?
Ah I see, I think I misunderstood; yes, this makes sense to me and
should work fine. I'm OK with this patch; here's another couple of
testcases I tried as well that also passed with your change:
Thanks, I incorporated them in the patch I'm pushing:
From b360d4aafc1356386313c7392f7444b3fc356681 Mon Sep 17 00:00:00 2001
From: Jason Merrill <ja...@redhat.com>
Date: Thu, 6 Mar 2025 12:39:36 -0500
Subject: [PATCH] c++/modules: purview of explicit instantiations [PR114630]
To: gcc-patches@gcc.gnu.org
When calling instantiate_pending_templates at end of parsing, any new
functions that are instantiated from this point have their module
purview set based on the current value of module_kind.
This is unideal, however, as the modules code will then treat these
instantiations as reachable and cause large swathes of the GMF to be
emitted into the module CMI, despite no code in the actual module
purview referencing it.
This patch fixes this by setting DECL_MODULE_PURVIEW_P as appropriate when
we see an explicit instantiation, and adjusting module_kind accordingly
during deferred instantiation, meaning that GMF entities won't be counted
as reachable unless referenced by an actually reachable entity.
Note that purviewness and attachment etc. is generally only determined
by the base template: this is purely for determining whether an
explicit instantiation is in the module purview and hence whether it
should be streamed out. See the comment on 'set_instantiating_module'.
Incidentally, since the "xtreme" testcases are deliberately large (and this
commit adds another one), let's make sure we only run them once.
PR c++/114630
PR c++/114795
gcc/cp/ChangeLog:
* pt.cc (reopen_tinst_level): Set or clear MK_PURVIEW.
(mark_decl_instantiated): Call set_instantiating_module.
(instantiate_pending_templates): Save and restore module_kind so
it isn't affected by reopen_tinst_level.
gcc/testsuite/ChangeLog:
* g++.dg/modules/modules.exp: Run xtreme tests once.
* g++.dg/modules/gmf-3.C: New test.
* g++.dg/modules/gmf-4.C: New test.
* g++.dg/modules/gmf-xtreme.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Co-authored-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/pt.cc | 17 ++++++++++++-
gcc/testsuite/g++.dg/modules/gmf-3.C | 29 +++++++++++++++++++++++
gcc/testsuite/g++.dg/modules/gmf-4.C | 13 ++++++++++
gcc/testsuite/g++.dg/modules/gmf-xtreme.C | 10 ++++++++
gcc/testsuite/g++.dg/modules/modules.exp | 12 ++++++----
5 files changed, 76 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/gmf-3.C
create mode 100644 gcc/testsuite/g++.dg/modules/gmf-4.C
create mode 100644 gcc/testsuite/g++.dg/modules/gmf-xtreme.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2378f892a53..6ac9e6f163e 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11476,7 +11476,19 @@ reopen_tinst_level (struct tinst_level *level)
pop_tinst_level ();
if (current_tinst_level)
current_tinst_level->errors = errorcount+sorrycount;
- return level->maybe_get_node ();
+
+ tree decl = level->maybe_get_node ();
+ if (decl && modules_p ())
+ {
+ /* An instantiation is in module purview only if it had an explicit
+ instantiation definition in module purview; mark_decl_instantiated uses
+ set_instantiating_module to set the flag in that case. */
+ if (DECL_MODULE_PURVIEW_P (decl))
+ module_kind |= MK_PURVIEW;
+ else
+ module_kind &= ~MK_PURVIEW;
+ }
+ return decl;
}
/* Returns the TINST_LEVEL which gives the original instantiation
@@ -25969,6 +25981,7 @@ mark_decl_instantiated (tree result, int extern_p)
{
mark_definable (result);
mark_needed (result);
+ set_instantiating_module (result);
/* Always make artificials weak. */
if (DECL_ARTIFICIAL (result) && flag_weak)
comdat_linkage (result);
@@ -27930,6 +27943,7 @@ instantiate_pending_templates (int retries)
{
int reconsider;
location_t saved_loc = input_location;
+ unsigned saved_module_kind = module_kind;
/* Instantiating templates may trigger vtable generation. This in turn
may require further template instantiations. We place a limit here
@@ -28020,6 +28034,7 @@ instantiate_pending_templates (int retries)
while (reconsider);
input_location = saved_loc;
+ module_kind = saved_module_kind;
}
/* Substitute ARGVEC into T, which is a list of initializers for
diff --git a/gcc/testsuite/g++.dg/modules/gmf-3.C b/gcc/testsuite/g++.dg/modules/gmf-3.C
new file mode 100644
index 00000000000..6b7ed503669
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/gmf-3.C
@@ -0,0 +1,29 @@
+// PR c++/114630
+// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi M }
+
+module;
+template <typename> struct allocator {
+ allocator() {}
+};
+template class allocator<wchar_t>;
+
+// Deferred instantiation of GM virtual functions or friend functions
+// should not place newly discovered declarations in the module purview.
+template <typename T>
+void go() {
+ extern T fn_decl();
+}
+template <typename T>
+struct S {
+ friend void x() {}
+ virtual void f() {
+ go<char>();
+ }
+};
+inline S<int> s;
+
+export module M;
+
+// The whole GMF should be discarded here
+// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }
diff --git a/gcc/testsuite/g++.dg/modules/gmf-4.C b/gcc/testsuite/g++.dg/modules/gmf-4.C
new file mode 100644
index 00000000000..91368f3cdcc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/gmf-4.C
@@ -0,0 +1,13 @@
+// PR c++/114630
+// { dg-additional-options "-fmodules-ts -Wno-global-module -fdump-lang-module" }
+// { dg-module-cmi M }
+
+module;
+template <typename> struct allocator {
+ allocator() {}
+};
+export module M;
+template class allocator<wchar_t>;
+
+// allocator is reachable because the explicit instantiation is in purview.
+// { dg-final { scan-lang-dump {Wrote declaration[^\n]*allocator} module } }
diff --git a/gcc/testsuite/g++.dg/modules/gmf-xtreme.C b/gcc/testsuite/g++.dg/modules/gmf-xtreme.C
new file mode 100644
index 00000000000..0a01c405291
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/gmf-xtreme.C
@@ -0,0 +1,10 @@
+// PR c++/114630
+// { dg-additional-options "-fmodules-ts -fdump-lang-module" }
+// { dg-module-cmi empty }
+
+module;
+#include "xtreme-header.h"
+export module empty;
+
+// The whole GMF should be discarded here
+// { dg-final { scan-lang-dump "Wrote 0 clusters" module } }
diff --git a/gcc/testsuite/g++.dg/modules/modules.exp b/gcc/testsuite/g++.dg/modules/modules.exp
index 335401b2166..81d0bebdfe5 100644
--- a/gcc/testsuite/g++.dg/modules/modules.exp
+++ b/gcc/testsuite/g++.dg/modules/modules.exp
@@ -269,6 +269,7 @@ proc module-init { src } {
set std_prefix "-std=c++"
global extra_tool_flags
set extra_tool_flags {}
+ global MOD_STD_LIST
foreach op $tmp {
switch [lindex $op 0] {
@@ -288,13 +289,16 @@ proc module-init { src } {
}
}
- if { !$have_std } {
- global MOD_STD_LIST
+ if { $have_std } {
+ lappend option_list ""
+ } elseif { [string match "*xtreme*" $src] } {
+ # Only run the xtreme tests once.
+ set x [lindex $MOD_STD_LIST end]
+ lappend option_list "${std_prefix}$x"
+ } else {
foreach x $MOD_STD_LIST {
lappend option_list "${std_prefix}$x"
}
- } else {
- lappend option_list ""
}
return $option_list
--
2.48.1
