On Fri, 21 Feb 2025, 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?).
>
> 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?
+1
>
> 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?
>
> Nathaniel
>
> On Wed, May 01, 2024 at 08:00:22PM +1000, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> >
> > -- >8 --
> >
> > 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 also remembering the value of module_kind when
> > the instantiation was deferred, and restoring it when doing this
> > deferred instantiation. That way newly instantiated declarations
> > appropriately get a DECL_MODULE_PURVIEW_P appropriate for where the
> > instantiation was required, 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 a
> > specialisation was declared 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:
> >
> > * cp-tree.h (struct tinst_level): Add field for tracking
> > module_kind.
> > * pt.cc (push_tinst_level_loc): Cache module_kind in new_level.
> > (reopen_tinst_level): Restore module_kind from level.
> > (instantiate_pending_templates): Save and restore module_kind so
> > it isn't affected by reopen_tinst_level.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/gmf-3.C: New test.
> >
> > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> > ---
> > gcc/cp/cp-tree.h | 3 +++
> > gcc/cp/pt.cc | 4 ++++
> > gcc/testsuite/g++.dg/modules/gmf-3.C | 13 +++++++++++++
> > 3 files changed, 20 insertions(+)
> > create mode 100644 gcc/testsuite/g++.dg/modules/gmf-3.C
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index 1938ada0268..0e619120ccc 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -6626,6 +6626,9 @@ struct GTY((chain_next ("%h.next"))) tinst_level {
> > /* The location where the template is instantiated. */
> > location_t locus;
> >
> > + /* The module kind where the template is instantiated. */
> > + unsigned module_kind;
> > +
> > /* errorcount + sorrycount when we pushed this level. */
> > unsigned short errors;
> >
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 1c3eef60c06..401aa92bc3e 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -11277,6 +11277,7 @@ push_tinst_level_loc (tree tldcl, tree targs,
> > location_t loc)
> > new_level->tldcl = tldcl;
> > new_level->targs = targs;
> > new_level->locus = loc;
> > + new_level->module_kind = module_kind;
> > new_level->errors = errorcount + sorrycount;
> > new_level->next = NULL;
> > new_level->refcount = 0;
> > @@ -11345,6 +11346,7 @@ reopen_tinst_level (struct tinst_level *level)
> > for (t = level; t; t = t->next)
> > ++tinst_depth;
> >
> > + module_kind = level->module_kind;
> > set_refcount_ptr (current_tinst_level, level);
> > pop_tinst_level ();
> > if (current_tinst_level)
> > @@ -27442,6 +27444,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
> > @@ -27532,6 +27535,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..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 } }
> > --
> > 2.43.2
> >
>
>
