On Thu, 31 Jul 2025, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> And maybe 15?
Good catch re the invisiref handling! This LGTM, Jason what do you
think?
>
> -- >8 --
>
> When we merge a function definition, if there already exists a forward
> declaration in the importing TU we use the PARM_DECLs belonging to that
> decl. This usually works fine, except as noted in the linked PR there
> are some flags (such as TREE_ADDRESSABLE) that only get set on a
> PARM_DECL once a definition is provided.
>
> This patch fixes the wrong-code issues by propagating any properties on
> PARM_DECLs I could find that may affect codegen.
>
> PR c++/121238
>
> gcc/cp/ChangeLog:
>
> * module.cc (trees_in::fn_parms_fini): Merge properties for
> definitions.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/merge-19.h: New test.
> * g++.dg/modules/merge-19_a.H: New test.
> * g++.dg/modules/merge-19_b.C: New test.
>
> Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com>
> ---
> gcc/cp/module.cc | 11 +++++++++++
> gcc/testsuite/g++.dg/modules/merge-19.h | 21 +++++++++++++++++++++
> gcc/testsuite/g++.dg/modules/merge-19_a.H | 5 +++++
> gcc/testsuite/g++.dg/modules/merge-19_b.C | 16 ++++++++++++++++
> 4 files changed, 53 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/modules/merge-19.h
> create mode 100644 gcc/testsuite/g++.dg/modules/merge-19_a.H
> create mode 100644 gcc/testsuite/g++.dg/modules/merge-19_b.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 2f6a8ab98fa..07a1dd85207 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -11164,6 +11164,17 @@ trees_in::fn_parms_fini (int tag, tree fn, tree
> existing, bool is_defn)
> names of the parms from us. */
> DECL_NAME (existing_parm) = DECL_NAME (parm);
> DECL_SOURCE_LOCATION (existing_parm) = DECL_SOURCE_LOCATION
> (parm);
> +
> + /* And some other flags important for codegen only set on
> + a parameter definition. */
> + TREE_ADDRESSABLE (existing_parm) = TREE_ADDRESSABLE (parm);
> + DECL_NONLOCAL (existing_parm) = DECL_NONLOCAL (parm);
> + DECL_ARG_TYPE (existing_parm) = DECL_ARG_TYPE (parm);
> + DECL_BY_REFERENCE (existing_parm) = DECL_BY_REFERENCE (parm);
> +
> + /* Invisref parms had their types adjusted by cp_genericize. */
> + TREE_TYPE (existing_parm) = TREE_TYPE (parm);
> + relayout_decl (existing_parm);
> }
>
> back_refs[~tag] = existing_parm;
> diff --git a/gcc/testsuite/g++.dg/modules/merge-19.h
> b/gcc/testsuite/g++.dg/modules/merge-19.h
> new file mode 100644
> index 00000000000..c3faadce07a
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-19.h
> @@ -0,0 +1,21 @@
> +// PR c++/121238
> +
> +inline void inc(const char*& __first) {
> + ++__first;
> +}
> +
> +template <typename = void>
> +bool parse_integer(const char *first) {
> + const char *start = first;
> + inc(first);
> + return first != start;
> +}
> +template bool parse_integer<void>(const char*);
> +
> +
> +struct S { ~S() {} int x; };
> +template <typename = void>
> +bool take_by_invisiref(S s) {
> + return s.x == 5;
> +}
> +template bool take_by_invisiref<void>(S);
> diff --git a/gcc/testsuite/g++.dg/modules/merge-19_a.H
> b/gcc/testsuite/g++.dg/modules/merge-19_a.H
> new file mode 100644
> index 00000000000..149a447f266
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-19_a.H
> @@ -0,0 +1,5 @@
> +// PR c++/121238
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +#include "merge-19.h"
> diff --git a/gcc/testsuite/g++.dg/modules/merge-19_b.C
> b/gcc/testsuite/g++.dg/modules/merge-19_b.C
> new file mode 100644
> index 00000000000..345e7fe1342
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-19_b.C
> @@ -0,0 +1,16 @@
> +// PR c++/121238
> +// { dg-module-do run }
> +// { dg-additional-options "-fmodules -fno-module-lazy" }
> +
> +#include "merge-19.h"
> +import "merge-19_a.H";
> +
> +int main() {
> + const char fmt[] = "5";
> + if (!parse_integer<void>(fmt))
> + __builtin_abort();
> +
> + S s{ 5 };
> + if (!take_by_invisiref(s))
> + __builtin_abort();
> +}
> --
> 2.47.0
>
>
