On Mon, 8 Jul 2024, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/14?
>
> -- >8 --
>
> When duplicate_decls finds a match with an existing imported
> declaration, it clears DECL_LANG_SPECIFIC of the olddecl and replaces it
> with the contents of newdecl; this clears DECL_MODULE_ENTITY_P causing
> an ICE if the same declaration is imported again later.
>
> This fixes the issue by ensuring that the flag is transferred to newdecl
> before clearing so that it ends up on olddecl again.
>
> For future-proofing we also do the same with DECL_MODULE_KEYED_DECLS_P,
> though because we don't yet support textual redefinition merging we
> can't yet test this works as intended. I don't expect it's possible for
> a new declaration already to have extra keyed decls mismatching that of
> the old declaration though, so I don't do anything with 'keyed_map' at
> this time.
Makes sense, thanks for tracking this down!
>
> PR c++/99241
>
> gcc/cp/ChangeLog:
>
> * decl.cc (duplicate_decls): Merge module entity information.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/pr99241_a.H: New test.
> * g++.dg/modules/pr99241_b.H: New test.
> * g++.dg/modules/pr99241_c.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> ---
> gcc/cp/decl.cc | 17 +++++++++++++++++
> gcc/testsuite/g++.dg/modules/pr99241_a.H | 3 +++
> gcc/testsuite/g++.dg/modules/pr99241_b.H | 3 +++
> gcc/testsuite/g++.dg/modules/pr99241_c.C | 5 +++++
> 4 files changed, 28 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_b.H
> create mode 100644 gcc/testsuite/g++.dg/modules/pr99241_c.C
>
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 29616100cfe..b3a770df926 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -3149,6 +3149,23 @@ duplicate_decls (tree newdecl, tree olddecl, bool
> hiding, bool was_hidden)
> if (TREE_CODE (newdecl) == FIELD_DECL)
> DECL_PACKED (olddecl) = DECL_PACKED (newdecl);
>
> + /* Merge module entity mapping information. */
> + if (modules_p ())
> + {
> + tree old_nontmpl = STRIP_TEMPLATE (olddecl);
It seems duplicate_decls has an unconditional early exit code path for
TEMPLATE_DECL
if (TREE_CODE (newdecl) == TEMPLATE_DECL)
{
tree old_result = DECL_TEMPLATE_RESULT (olddecl);
...
return olddecl;
}
so I think these STRIP_TEMPLATEs are unneeded? (And I guess this early
exit explains why the testcase doesn't ICE when 'terminate' is a
function template.)
> + if (DECL_LANG_SPECIFIC (old_nontmpl)
> + && (DECL_MODULE_ENTITY_P (old_nontmpl)
> + || DECL_MODULE_KEYED_DECLS_P (old_nontmpl)))
> + {
> + tree new_nontmpl = STRIP_TEMPLATE (newdecl);
> + retrofit_lang_decl (new_nontmpl);
> + DECL_MODULE_ENTITY_P (new_nontmpl)
> + = DECL_MODULE_ENTITY_P (old_nontmpl);
> + DECL_MODULE_KEYED_DECLS_P (new_nontmpl)
> + = DECL_MODULE_KEYED_DECLS_P (old_nontmpl);
> + }
> + }
> +
> /* The DECL_LANG_SPECIFIC information in OLDDECL will be replaced
> with that from NEWDECL below. */
> if (DECL_LANG_SPECIFIC (olddecl))
> diff --git a/gcc/testsuite/g++.dg/modules/pr99241_a.H
> b/gcc/testsuite/g++.dg/modules/pr99241_a.H
> new file mode 100644
> index 00000000000..c7031f08eb5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99241_a.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +void terminate();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99241_b.H
> b/gcc/testsuite/g++.dg/modules/pr99241_b.H
> new file mode 100644
> index 00000000000..c7031f08eb5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99241_b.H
> @@ -0,0 +1,3 @@
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +void terminate();
> diff --git a/gcc/testsuite/g++.dg/modules/pr99241_c.C
> b/gcc/testsuite/g++.dg/modules/pr99241_c.C
> new file mode 100644
> index 00000000000..7f2b1bb43ea
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/pr99241_c.C
> @@ -0,0 +1,5 @@
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +import "pr99241_a.H";
> +void terminate();
> +import "pr99241_b.H";
> --
> 2.43.2
>
>
