c++/modules: Fix installing a partial spec over an implicit inst [PR124732]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_gcc_build--master-arm |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_build--master-aarch64 |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-arm |
success
|
Test passed
|
| linaro-tcwg-bot/tcwg_gcc_check--master-aarch64 |
success
|
Test passed
|
Commit Message
Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
-- >8 --
We fail a checking-only assertion in the linked PR because we get a
TEMPLATE_DECL that is marked as DECL_MODULE_ENTITY_P but we can't find
its entry in the entity map.
The following sequence of events has occurred:
1. A partial specialisation in the current TU is built.
2. We import a pending instantiation (a TYPE_DECL); this matches the
partial specialisation, we register the existing TYPE_DECL on the
entity_map and mark as DECL_MODULE_ENTITY_P.
3. We import a partial specialisation (a TEMPLATE_DECL); this matches
the same partial specialisation, but we attempt to register the
TEMPLATE_DECL. We see that DECL_MODULE_ENTITY_P is already set
(we only store this on the DECL_TEMPLATE_RESULT) and so assume we
should be able to find it in the entity map, but it's not there.
This is similar to PR c++/120013, except in that case we had swapped
steps #2 and #3, so we attempted to recover the TEMPLATE_DECL of the
partial spec; but I'd gotten it wrong and got the primary TEMPLATE_DECL
rather than the partial one, though I haven't been able to find a way to
cause a failure from that yet.
On reflection, I think r14-9881-g77c0b5b23f was wrong to mess around
with overriding entity_map to indicate partition imports that will need
to be exported, as it seems to be quite fragile and I'm concerned there
are more edge cases we haven't found yet. So this patch instead
introduces a new DECL_MODULE_PARTITION_P flag to indicate entities
imported from a module partition, and then make_dependency can check
this flag to determine what entities can be treated as imported.
PR c++/124732
gcc/cp/ChangeLog:
* cp-tree.h (DECL_MODULE_PARTITION_P): New accessor.
(struct lang_decl_base): New field 'module_partition_p'.
* decl.cc (duplicate_decls): Handle DECL_MODULE_PARTITION_P.
* lex.cc (cxx_dup_lang_specific_decl): Likewise.
* module.cc (trees_in::install_entity): Just set
DECL_MODULE_PARTITION_P rather than attempting to adjust
entity_map.
(depset::hash::make_dependency): Check DECL_MODULE_PARTITION_P
instead of from->remap for marking imported deps.
gcc/testsuite/ChangeLog:
* g++.dg/modules/partial-8_d.C: Supplement test.
Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
gcc/cp/cp-tree.h | 7 ++-
gcc/cp/decl.cc | 2 +
gcc/cp/lex.cc | 1 +
gcc/cp/module.cc | 56 ++++++----------------
gcc/testsuite/g++.dg/modules/partial-8_d.C | 7 ++-
5 files changed, 29 insertions(+), 44 deletions(-)
Comments
On 4/3/26 7:47 PM, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
>
> -- >8 --
>
> We fail a checking-only assertion in the linked PR because we get a
> TEMPLATE_DECL that is marked as DECL_MODULE_ENTITY_P but we can't find
> its entry in the entity map.
>
> The following sequence of events has occurred:
>
> 1. A partial specialisation in the current TU is built.
>
> 2. We import a pending instantiation (a TYPE_DECL); this matches the
> partial specialisation, we register the existing TYPE_DECL on the
> entity_map and mark as DECL_MODULE_ENTITY_P.
>
> 3. We import a partial specialisation (a TEMPLATE_DECL); this matches
> the same partial specialisation, but we attempt to register the
> TEMPLATE_DECL. We see that DECL_MODULE_ENTITY_P is already set
> (we only store this on the DECL_TEMPLATE_RESULT) and so assume we
> should be able to find it in the entity map, but it's not there.
>
> This is similar to PR c++/120013, except in that case we had swapped
> steps #2 and #3, so we attempted to recover the TEMPLATE_DECL of the
> partial spec; but I'd gotten it wrong and got the primary TEMPLATE_DECL
> rather than the partial one, though I haven't been able to find a way to
> cause a failure from that yet.
>
> On reflection, I think r14-9881-g77c0b5b23f was wrong to mess around
> with overriding entity_map to indicate partition imports that will need
> to be exported, as it seems to be quite fragile and I'm concerned there
> are more edge cases we haven't found yet. So this patch instead
> introduces a new DECL_MODULE_PARTITION_P flag to indicate entities
> imported from a module partition, and then make_dependency can check
> this flag to determine what entities can be treated as imported.
Can't make_dependency look at from->partition_p instead of adding
another flag on every decl?
> PR c++/124732
>
> gcc/cp/ChangeLog:
>
> * cp-tree.h (DECL_MODULE_PARTITION_P): New accessor.
> (struct lang_decl_base): New field 'module_partition_p'.
> * decl.cc (duplicate_decls): Handle DECL_MODULE_PARTITION_P.
> * lex.cc (cxx_dup_lang_specific_decl): Likewise.
> * module.cc (trees_in::install_entity): Just set
> DECL_MODULE_PARTITION_P rather than attempting to adjust
> entity_map.
> (depset::hash::make_dependency): Check DECL_MODULE_PARTITION_P
> instead of from->remap for marking imported deps.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/partial-8_d.C: Supplement test.
>
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
> gcc/cp/cp-tree.h | 7 ++-
> gcc/cp/decl.cc | 2 +
> gcc/cp/lex.cc | 1 +
> gcc/cp/module.cc | 56 ++++++----------------
> gcc/testsuite/g++.dg/modules/partial-8_d.C | 7 ++-
> 5 files changed, 29 insertions(+), 44 deletions(-)
>
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index ea3cb049785..d5746af8b9a 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -1848,6 +1848,10 @@ check_constraint_info (tree t)
> #define DECL_MODULE_IMPORT_P(NODE) \
> (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_import_p)
>
> +/* True if this decl was seen from a partition. */
> +#define DECL_MODULE_PARTITION_P(NODE) \
> + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_partition_p)
> +
> /* True if this decl is in the entity hash & array. This means that
> some variant was imported, even if DECL_MODULE_IMPORT_P is false. */
> #define DECL_MODULE_ENTITY_P(NODE) \
> @@ -3142,6 +3146,7 @@ struct GTY(()) lang_decl_base {
> unsigned module_purview_p : 1; /* in named-module purview */
> unsigned module_attach_p : 1; /* attached to named module */
> unsigned module_import_p : 1; /* from an import */
> + unsigned module_partition_p : 1; /* from a partition */
> unsigned module_entity_p : 1; /* is in the entitity ary & hash */
>
> unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */
> @@ -3149,7 +3154,7 @@ struct GTY(()) lang_decl_base {
> /* VAR_DECL being used to represent an OpenMP declared mapper. */
> unsigned omp_declare_mapper_p : 1;
>
> - /* 10 spare bits. */
> + /* 9 spare bits. */
> };
>
> /* True for DECL codes which have template info and access. */
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 8409752aa0c..d5f7dfb765b 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -3364,10 +3364,12 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
> /* Merge module entity mapping information. */
> if (DECL_LANG_SPECIFIC (olddecl)
> && (DECL_MODULE_ENTITY_P (olddecl)
> + || DECL_MODULE_PARTITION_P (olddecl)
> || DECL_MODULE_KEYED_DECLS_P (olddecl)))
> {
> retrofit_lang_decl (newdecl);
> DECL_MODULE_ENTITY_P (newdecl) = DECL_MODULE_ENTITY_P (olddecl);
> + DECL_MODULE_PARTITION_P (newdecl) = DECL_MODULE_PARTITION_P (olddecl);
> DECL_MODULE_KEYED_DECLS_P (newdecl) = DECL_MODULE_KEYED_DECLS_P (olddecl);
> }
>
> diff --git a/gcc/cp/lex.cc b/gcc/cp/lex.cc
> index 88b0b24e097..135ded13047 100644
> --- a/gcc/cp/lex.cc
> +++ b/gcc/cp/lex.cc
> @@ -1111,6 +1111,7 @@ cxx_dup_lang_specific_decl (tree node)
> (module_purview_p still does). */
> ld->u.base.module_entity_p = false;
> ld->u.base.module_import_p = false;
> + ld->u.base.module_partition_p = false;
> ld->u.base.module_keyed_decls_p = false;
>
> if (GATHER_STATISTICS)
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 6958388e454..e2b28e80e51 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -8284,38 +8284,10 @@ trees_in::install_entity (tree decl)
> gcc_checking_assert (!existed);
> slot = ident;
> }
> - else
> - {
> - unsigned *slot = entity_map->get (DECL_UID (decl));
> -
> - /* The entity must be in the entity map already. However, DECL may
> - be the DECL_TEMPLATE_RESULT of an existing partial specialisation
> - if we matched it while streaming another instantiation; in this
> - case we already registered that TEMPLATE_DECL. */
> - if (!slot)
> - {
> - tree type = TREE_TYPE (decl);
> - gcc_checking_assert (TREE_CODE (decl) == TYPE_DECL
> - && CLASS_TYPE_P (type)
> - && CLASSTYPE_TEMPLATE_SPECIALIZATION (type));
> - slot = entity_map->get (DECL_UID (CLASSTYPE_TI_TEMPLATE (type)));
> - }
> - gcc_checking_assert (slot);
>
> - if (state->is_partition ())
> - {
> - /* The decl is already in the entity map, but we see it again now
> - from a partition: we want to overwrite if the original decl
> - wasn't also from a (possibly different) partition. Otherwise,
> - for things like template instantiations, make_dependency might
> - not realise that this is also provided from a partition and
> - should be considered part of this module (and thus always
> - emitted into the primary interface's CMI). */
> - module_state *imp = import_entity_module (*slot);
> - if (!imp->is_partition ())
> - *slot = ident;
> - }
> - }
> + /* Remember to treat this as-if declared in this module later. */
> + if (state->is_partition ())
> + DECL_MODULE_PARTITION_P (not_tmpl) = true;
>
> return true;
> }
> @@ -14648,23 +14620,23 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
> bool imported_from_module_p = false;
>
> if (DECL_LANG_SPECIFIC (not_tmpl)
> - && DECL_MODULE_IMPORT_P (not_tmpl))
> + && DECL_MODULE_IMPORT_P (not_tmpl)
> + /* Treat imports from partitions as part of the primary module. */
> + && (!DECL_MODULE_PARTITION_P (not_tmpl)
> + || module_partition_p ()))
> {
> /* Store the module number and index in cluster/section,
> so we don't have to look them up again. */
> unsigned index = import_entity_index (decl);
> module_state *from = import_entity_module (index);
> - /* Remap will be zero for imports from partitions, which
> - we want to treat as-if declared in this TU. */
> - if (from->remap)
> - {
> - dep->cluster = index - from->entity_lwm;
> - dep->section = from->remap;
> - dep->set_flag_bit<DB_IMPORTED_BIT> ();
> + gcc_checking_assert (from->remap);
>
> - if (!from->is_header ())
> - imported_from_module_p = true;
> - }
> + dep->cluster = index - from->entity_lwm;
> + dep->section = from->remap;
> + dep->set_flag_bit<DB_IMPORTED_BIT> ();
> +
> + if (!from->is_header ())
> + imported_from_module_p = true;
> }
>
> /* Check for TU-local entities. This is unnecessary in header
> diff --git a/gcc/testsuite/g++.dg/modules/partial-8_d.C b/gcc/testsuite/g++.dg/modules/partial-8_d.C
> index 2aedb39670c..8df484cef11 100644
> --- a/gcc/testsuite/g++.dg/modules/partial-8_d.C
> +++ b/gcc/testsuite/g++.dg/modules/partial-8_d.C
> @@ -1,9 +1,14 @@
> // PR c++/120013
> -// { dg-additional-options "-fmodules" }
> +// PR c++/124732
> +// { dg-additional-options "-fmodules -Wno-global-module" }
> // { dg-module-cmi m }
> // Same as partial-8_c.C but in the other order, to ensure
> // that loading a partial spec over an instantiation works
>
> +module;
> +#include "partial-8.h"
> +template <typename T> struct tuple_element<T*>;
> +template <typename T> constexpr int var<T*> = 456;
> export module m;
> import :b;
> import :a;
On Mon, Apr 06, 2026 at 12:52:45PM -0400, Jason Merrill wrote:
> On 4/3/26 7:47 PM, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
> >
> > -- >8 --
> >
> > We fail a checking-only assertion in the linked PR because we get a
> > TEMPLATE_DECL that is marked as DECL_MODULE_ENTITY_P but we can't find
> > its entry in the entity map.
> >
> > The following sequence of events has occurred:
> >
> > 1. A partial specialisation in the current TU is built.
> >
> > 2. We import a pending instantiation (a TYPE_DECL); this matches the
> > partial specialisation, we register the existing TYPE_DECL on the
> > entity_map and mark as DECL_MODULE_ENTITY_P.
> >
> > 3. We import a partial specialisation (a TEMPLATE_DECL); this matches
> > the same partial specialisation, but we attempt to register the
> > TEMPLATE_DECL. We see that DECL_MODULE_ENTITY_P is already set
> > (we only store this on the DECL_TEMPLATE_RESULT) and so assume we
> > should be able to find it in the entity map, but it's not there.
> >
> > This is similar to PR c++/120013, except in that case we had swapped
> > steps #2 and #3, so we attempted to recover the TEMPLATE_DECL of the
> > partial spec; but I'd gotten it wrong and got the primary TEMPLATE_DECL
> > rather than the partial one, though I haven't been able to find a way to
> > cause a failure from that yet.
> >
> > On reflection, I think r14-9881-g77c0b5b23f was wrong to mess around
> > with overriding entity_map to indicate partition imports that will need
> > to be exported, as it seems to be quite fragile and I'm concerned there
> > are more edge cases we haven't found yet. So this patch instead
> > introduces a new DECL_MODULE_PARTITION_P flag to indicate entities
> > imported from a module partition, and then make_dependency can check
> > this flag to determine what entities can be treated as imported.
>
> Can't make_dependency look at from->partition_p instead of adding another
> flag on every decl?
>
The issue that r14-9881 was trying to fix is that if a declaration was
first imported from a non-partition module before being seen in a
partition, we don't reinstall into the entity map and so 'from' in this
case will not be marked 'partition_p' (otherwise the existing behaviour
checking remap would have also worked).
The trickiness is trying to deal with partial specialisations importing
over implicit specialisations (or vice-versa) where we don't have a
stable DECL_UID to search for in the entity map array, because the
former we get TEMPLATE_DECLs but the latter is a regular TYPE_DECL.
If you prefer I could try to spend some time finding a different way to
approach this that works in this case and doesn't incorrectly correspond
the entity_map with a different kind of entity in entity_ary.
> > PR c++/124732
> >
> > gcc/cp/ChangeLog:
> >
> > * cp-tree.h (DECL_MODULE_PARTITION_P): New accessor.
> > (struct lang_decl_base): New field 'module_partition_p'.
> > * decl.cc (duplicate_decls): Handle DECL_MODULE_PARTITION_P.
> > * lex.cc (cxx_dup_lang_specific_decl): Likewise.
> > * module.cc (trees_in::install_entity): Just set
> > DECL_MODULE_PARTITION_P rather than attempting to adjust
> > entity_map.
> > (depset::hash::make_dependency): Check DECL_MODULE_PARTITION_P
> > instead of from->remap for marking imported deps.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/modules/partial-8_d.C: Supplement test.
> >
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> > gcc/cp/cp-tree.h | 7 ++-
> > gcc/cp/decl.cc | 2 +
> > gcc/cp/lex.cc | 1 +
> > gcc/cp/module.cc | 56 ++++++----------------
> > gcc/testsuite/g++.dg/modules/partial-8_d.C | 7 ++-
> > 5 files changed, 29 insertions(+), 44 deletions(-)
> >
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index ea3cb049785..d5746af8b9a 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -1848,6 +1848,10 @@ check_constraint_info (tree t)
> > #define DECL_MODULE_IMPORT_P(NODE) \
> > (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_import_p)
> > +/* True if this decl was seen from a partition. */
> > +#define DECL_MODULE_PARTITION_P(NODE) \
> > + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_partition_p)
> > +
> > /* True if this decl is in the entity hash & array. This means that
> > some variant was imported, even if DECL_MODULE_IMPORT_P is false. */
> > #define DECL_MODULE_ENTITY_P(NODE) \
> > @@ -3142,6 +3146,7 @@ struct GTY(()) lang_decl_base {
> > unsigned module_purview_p : 1; /* in named-module purview */
> > unsigned module_attach_p : 1; /* attached to named module */
> > unsigned module_import_p : 1; /* from an import */
> > + unsigned module_partition_p : 1; /* from a partition */
> > unsigned module_entity_p : 1; /* is in the entitity ary & hash */
> > unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */
> > @@ -3149,7 +3154,7 @@ struct GTY(()) lang_decl_base {
> > /* VAR_DECL being used to represent an OpenMP declared mapper. */
> > unsigned omp_declare_mapper_p : 1;
> > - /* 10 spare bits. */
> > + /* 9 spare bits. */
> > };
> > /* True for DECL codes which have template info and access. */
> > diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> > index 8409752aa0c..d5f7dfb765b 100644
> > --- a/gcc/cp/decl.cc
> > +++ b/gcc/cp/decl.cc
> > @@ -3364,10 +3364,12 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
> > /* Merge module entity mapping information. */
> > if (DECL_LANG_SPECIFIC (olddecl)
> > && (DECL_MODULE_ENTITY_P (olddecl)
> > + || DECL_MODULE_PARTITION_P (olddecl)
> > || DECL_MODULE_KEYED_DECLS_P (olddecl)))
> > {
> > retrofit_lang_decl (newdecl);
> > DECL_MODULE_ENTITY_P (newdecl) = DECL_MODULE_ENTITY_P (olddecl);
> > + DECL_MODULE_PARTITION_P (newdecl) = DECL_MODULE_PARTITION_P (olddecl);
> > DECL_MODULE_KEYED_DECLS_P (newdecl) = DECL_MODULE_KEYED_DECLS_P (olddecl);
> > }
> > diff --git a/gcc/cp/lex.cc b/gcc/cp/lex.cc
> > index 88b0b24e097..135ded13047 100644
> > --- a/gcc/cp/lex.cc
> > +++ b/gcc/cp/lex.cc
> > @@ -1111,6 +1111,7 @@ cxx_dup_lang_specific_decl (tree node)
> > (module_purview_p still does). */
> > ld->u.base.module_entity_p = false;
> > ld->u.base.module_import_p = false;
> > + ld->u.base.module_partition_p = false;
> > ld->u.base.module_keyed_decls_p = false;
> > if (GATHER_STATISTICS)
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 6958388e454..e2b28e80e51 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -8284,38 +8284,10 @@ trees_in::install_entity (tree decl)
> > gcc_checking_assert (!existed);
> > slot = ident;
> > }
> > - else
> > - {
> > - unsigned *slot = entity_map->get (DECL_UID (decl));
> > -
> > - /* The entity must be in the entity map already. However, DECL may
> > - be the DECL_TEMPLATE_RESULT of an existing partial specialisation
> > - if we matched it while streaming another instantiation; in this
> > - case we already registered that TEMPLATE_DECL. */
> > - if (!slot)
> > - {
> > - tree type = TREE_TYPE (decl);
> > - gcc_checking_assert (TREE_CODE (decl) == TYPE_DECL
> > - && CLASS_TYPE_P (type)
> > - && CLASSTYPE_TEMPLATE_SPECIALIZATION (type));
> > - slot = entity_map->get (DECL_UID (CLASSTYPE_TI_TEMPLATE (type)));
> > - }
> > - gcc_checking_assert (slot);
> > - if (state->is_partition ())
> > - {
> > - /* The decl is already in the entity map, but we see it again now
> > - from a partition: we want to overwrite if the original decl
> > - wasn't also from a (possibly different) partition. Otherwise,
> > - for things like template instantiations, make_dependency might
> > - not realise that this is also provided from a partition and
> > - should be considered part of this module (and thus always
> > - emitted into the primary interface's CMI). */
> > - module_state *imp = import_entity_module (*slot);
> > - if (!imp->is_partition ())
> > - *slot = ident;
> > - }
> > - }
> > + /* Remember to treat this as-if declared in this module later. */
> > + if (state->is_partition ())
> > + DECL_MODULE_PARTITION_P (not_tmpl) = true;
> > return true;
> > }
> > @@ -14648,23 +14620,23 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
> > bool imported_from_module_p = false;
> > if (DECL_LANG_SPECIFIC (not_tmpl)
> > - && DECL_MODULE_IMPORT_P (not_tmpl))
> > + && DECL_MODULE_IMPORT_P (not_tmpl)
> > + /* Treat imports from partitions as part of the primary module. */
> > + && (!DECL_MODULE_PARTITION_P (not_tmpl)
> > + || module_partition_p ()))
> > {
> > /* Store the module number and index in cluster/section,
> > so we don't have to look them up again. */
> > unsigned index = import_entity_index (decl);
> > module_state *from = import_entity_module (index);
> > - /* Remap will be zero for imports from partitions, which
> > - we want to treat as-if declared in this TU. */
> > - if (from->remap)
> > - {
> > - dep->cluster = index - from->entity_lwm;
> > - dep->section = from->remap;
> > - dep->set_flag_bit<DB_IMPORTED_BIT> ();
> > + gcc_checking_assert (from->remap);
> > - if (!from->is_header ())
> > - imported_from_module_p = true;
> > - }
> > + dep->cluster = index - from->entity_lwm;
> > + dep->section = from->remap;
> > + dep->set_flag_bit<DB_IMPORTED_BIT> ();
> > +
> > + if (!from->is_header ())
> > + imported_from_module_p = true;
> > }
> > /* Check for TU-local entities. This is unnecessary in header
> > diff --git a/gcc/testsuite/g++.dg/modules/partial-8_d.C b/gcc/testsuite/g++.dg/modules/partial-8_d.C
> > index 2aedb39670c..8df484cef11 100644
> > --- a/gcc/testsuite/g++.dg/modules/partial-8_d.C
> > +++ b/gcc/testsuite/g++.dg/modules/partial-8_d.C
> > @@ -1,9 +1,14 @@
> > // PR c++/120013
> > -// { dg-additional-options "-fmodules" }
> > +// PR c++/124732
> > +// { dg-additional-options "-fmodules -Wno-global-module" }
> > // { dg-module-cmi m }
> > // Same as partial-8_c.C but in the other order, to ensure
> > // that loading a partial spec over an instantiation works
> > +module;
> > +#include "partial-8.h"
> > +template <typename T> struct tuple_element<T*>;
> > +template <typename T> constexpr int var<T*> = 456;
> > export module m;
> > import :b;
> > import :a;
>
On 4/6/26 5:32 PM, Nathaniel Shead wrote:
> On Mon, Apr 06, 2026 at 12:52:45PM -0400, Jason Merrill wrote:
>> On 4/3/26 7:47 PM, Nathaniel Shead wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/15?
>>>
>>> -- >8 --
>>>
>>> We fail a checking-only assertion in the linked PR because we get a
>>> TEMPLATE_DECL that is marked as DECL_MODULE_ENTITY_P but we can't find
>>> its entry in the entity map.
>>>
>>> The following sequence of events has occurred:
>>>
>>> 1. A partial specialisation in the current TU is built.
>>>
>>> 2. We import a pending instantiation (a TYPE_DECL); this matches the
>>> partial specialisation, we register the existing TYPE_DECL on the
>>> entity_map and mark as DECL_MODULE_ENTITY_P.
>>>
>>> 3. We import a partial specialisation (a TEMPLATE_DECL); this matches
>>> the same partial specialisation, but we attempt to register the
>>> TEMPLATE_DECL. We see that DECL_MODULE_ENTITY_P is already set
>>> (we only store this on the DECL_TEMPLATE_RESULT) and so assume we
>>> should be able to find it in the entity map, but it's not there.
>>>
>>> This is similar to PR c++/120013, except in that case we had swapped
>>> steps #2 and #3, so we attempted to recover the TEMPLATE_DECL of the
>>> partial spec; but I'd gotten it wrong and got the primary TEMPLATE_DECL
>>> rather than the partial one, though I haven't been able to find a way to
>>> cause a failure from that yet.
>>>
>>> On reflection, I think r14-9881-g77c0b5b23f was wrong to mess around
>>> with overriding entity_map to indicate partition imports that will need
>>> to be exported, as it seems to be quite fragile and I'm concerned there
>>> are more edge cases we haven't found yet. So this patch instead
>>> introduces a new DECL_MODULE_PARTITION_P flag to indicate entities
>>> imported from a module partition, and then make_dependency can check
>>> this flag to determine what entities can be treated as imported.
>>
>> Can't make_dependency look at from->partition_p instead of adding another
>> flag on every decl?
>
> The issue that r14-9881 was trying to fix is that if a declaration was
> first imported from a non-partition module before being seen in a
> partition, we don't reinstall into the entity map and so 'from' in this
> case will not be marked 'partition_p' (otherwise the existing behaviour
> checking remap would have also worked).
>
> The trickiness is trying to deal with partial specialisations importing
> over implicit specialisations (or vice-versa) where we don't have a
> stable DECL_UID to search for in the entity map array, because the
> former we get TEMPLATE_DECLs but the latter is a regular TYPE_DECL.
> If you prefer I could try to spend some time finding a different way to
> approach this that works in this case and doesn't incorrectly correspond
> the entity_map with a different kind of entity in entity_ary.
I imagine using the TYPE_DECL for lookup in both cases might work?
Jason
@@ -1848,6 +1848,10 @@ check_constraint_info (tree t)
#define DECL_MODULE_IMPORT_P(NODE) \
(DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_import_p)
+/* True if this decl was seen from a partition. */
+#define DECL_MODULE_PARTITION_P(NODE) \
+ (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_partition_p)
+
/* True if this decl is in the entity hash & array. This means that
some variant was imported, even if DECL_MODULE_IMPORT_P is false. */
#define DECL_MODULE_ENTITY_P(NODE) \
@@ -3142,6 +3146,7 @@ struct GTY(()) lang_decl_base {
unsigned module_purview_p : 1; /* in named-module purview */
unsigned module_attach_p : 1; /* attached to named module */
unsigned module_import_p : 1; /* from an import */
+ unsigned module_partition_p : 1; /* from a partition */
unsigned module_entity_p : 1; /* is in the entitity ary & hash */
unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */
@@ -3149,7 +3154,7 @@ struct GTY(()) lang_decl_base {
/* VAR_DECL being used to represent an OpenMP declared mapper. */
unsigned omp_declare_mapper_p : 1;
- /* 10 spare bits. */
+ /* 9 spare bits. */
};
/* True for DECL codes which have template info and access. */
@@ -3364,10 +3364,12 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
/* Merge module entity mapping information. */
if (DECL_LANG_SPECIFIC (olddecl)
&& (DECL_MODULE_ENTITY_P (olddecl)
+ || DECL_MODULE_PARTITION_P (olddecl)
|| DECL_MODULE_KEYED_DECLS_P (olddecl)))
{
retrofit_lang_decl (newdecl);
DECL_MODULE_ENTITY_P (newdecl) = DECL_MODULE_ENTITY_P (olddecl);
+ DECL_MODULE_PARTITION_P (newdecl) = DECL_MODULE_PARTITION_P (olddecl);
DECL_MODULE_KEYED_DECLS_P (newdecl) = DECL_MODULE_KEYED_DECLS_P (olddecl);
}
@@ -1111,6 +1111,7 @@ cxx_dup_lang_specific_decl (tree node)
(module_purview_p still does). */
ld->u.base.module_entity_p = false;
ld->u.base.module_import_p = false;
+ ld->u.base.module_partition_p = false;
ld->u.base.module_keyed_decls_p = false;
if (GATHER_STATISTICS)
@@ -8284,38 +8284,10 @@ trees_in::install_entity (tree decl)
gcc_checking_assert (!existed);
slot = ident;
}
- else
- {
- unsigned *slot = entity_map->get (DECL_UID (decl));
-
- /* The entity must be in the entity map already. However, DECL may
- be the DECL_TEMPLATE_RESULT of an existing partial specialisation
- if we matched it while streaming another instantiation; in this
- case we already registered that TEMPLATE_DECL. */
- if (!slot)
- {
- tree type = TREE_TYPE (decl);
- gcc_checking_assert (TREE_CODE (decl) == TYPE_DECL
- && CLASS_TYPE_P (type)
- && CLASSTYPE_TEMPLATE_SPECIALIZATION (type));
- slot = entity_map->get (DECL_UID (CLASSTYPE_TI_TEMPLATE (type)));
- }
- gcc_checking_assert (slot);
- if (state->is_partition ())
- {
- /* The decl is already in the entity map, but we see it again now
- from a partition: we want to overwrite if the original decl
- wasn't also from a (possibly different) partition. Otherwise,
- for things like template instantiations, make_dependency might
- not realise that this is also provided from a partition and
- should be considered part of this module (and thus always
- emitted into the primary interface's CMI). */
- module_state *imp = import_entity_module (*slot);
- if (!imp->is_partition ())
- *slot = ident;
- }
- }
+ /* Remember to treat this as-if declared in this module later. */
+ if (state->is_partition ())
+ DECL_MODULE_PARTITION_P (not_tmpl) = true;
return true;
}
@@ -14648,23 +14620,23 @@ depset::hash::make_dependency (tree decl, entity_kind ek)
bool imported_from_module_p = false;
if (DECL_LANG_SPECIFIC (not_tmpl)
- && DECL_MODULE_IMPORT_P (not_tmpl))
+ && DECL_MODULE_IMPORT_P (not_tmpl)
+ /* Treat imports from partitions as part of the primary module. */
+ && (!DECL_MODULE_PARTITION_P (not_tmpl)
+ || module_partition_p ()))
{
/* Store the module number and index in cluster/section,
so we don't have to look them up again. */
unsigned index = import_entity_index (decl);
module_state *from = import_entity_module (index);
- /* Remap will be zero for imports from partitions, which
- we want to treat as-if declared in this TU. */
- if (from->remap)
- {
- dep->cluster = index - from->entity_lwm;
- dep->section = from->remap;
- dep->set_flag_bit<DB_IMPORTED_BIT> ();
+ gcc_checking_assert (from->remap);
- if (!from->is_header ())
- imported_from_module_p = true;
- }
+ dep->cluster = index - from->entity_lwm;
+ dep->section = from->remap;
+ dep->set_flag_bit<DB_IMPORTED_BIT> ();
+
+ if (!from->is_header ())
+ imported_from_module_p = true;
}
/* Check for TU-local entities. This is unnecessary in header
@@ -1,9 +1,14 @@
// PR c++/120013
-// { dg-additional-options "-fmodules" }
+// PR c++/124732
+// { dg-additional-options "-fmodules -Wno-global-module" }
// { dg-module-cmi m }
// Same as partial-8_c.C but in the other order, to ensure
// that loading a partial spec over an instantiation works
+module;
+#include "partial-8.h"
+template <typename T> struct tuple_element<T*>;
+template <typename T> constexpr int var<T*> = 456;
export module m;
import :b;
import :a;