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?
Or go with this patch but only look at the saved module_kind if
DECL_EXPLICIT_INSTANTIATION, but that seems a bit of a waste of space. Might
be safer at this point, though.
Part of the issue is handling things that are "implicit instantiations"
that we don't track were as such; the obvious case here is friend decls,
which get instantiated with the current purviewness but might only have
been instantiated due to a deferred implicit instantiation, and for
which we remove any evidence that they were created due to an implicit
instantiation.
So maybe looking at the problem from that side would also work, but I'm
not sure exactly how best to do that just yet.
Another approach would be to maybe lower the error to a permerror, so
that users that 'know better' could compile such modules anyway (at risk
of various link errors and ODR issues), though I would also need to
adjust the streaming logic to handle this better. Thoughts?
This also sounds desirable. With a warning flag so it can be disabled for
certain deliberate cases like the gthr stuff.
OK, I'll try and took a look if I get a chance (unfortunately life has
interrupted me again for the time being...). I'm not sure that this
would work for the gthr case though (since the error occurs at the point
of use not at the point of declaration); for that we would probably need
some kind of attribute I guess?
We frequently use warning_enabled_at to suppress a diagnostic based on
whether the diagnostic is enabled at the point of definition of some entity.
Jason
From c4b03fed1b6f1bad193a90ddfc64b175fdb92f7b 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'.
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.
gcc/testsuite/ChangeLog:
* g++.dg/modules/gmf-3.C: New test.
* g++.dg/modules/gmf-4.C: New test.
Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
Co-authored-by: Nathaniel Shead <nathanielosh...@gmail.com>
---
gcc/cp/pt.cc | 12 +++++++++++-
gcc/testsuite/g++.dg/modules/gmf-3.C | 13 +++++++++++++
gcc/testsuite/g++.dg/modules/gmf-4.C | 13 +++++++++++++
3 files changed, 37 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/gmf-3.C
create mode 100644 gcc/testsuite/g++.dg/modules/gmf-4.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index c09a934580f..05a7ec29604 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11531,7 +11531,16 @@ 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 ())
+ {
+ 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
@@ -26023,6 +26032,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);
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..e52ae904ea9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/gmf-3.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() {}
+};
+template class allocator<wchar_t>;
+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 } }
--
2.48.1
