Re: [PATCH] c++/modules: Prevent overwriting arguments when merging duplicates [PR112588]

2024-01-02 Thread Nathaniel Shead
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637768.html.

On Sat, Dec 16, 2023 at 09:50:10PM +1100, Nathaniel Shead wrote:
> Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637768.html.
> (I've changed the summary message a little from that email but the patch
> is otherwise unchanged.)
> 
> On Wed, Nov 22, 2023 at 10:33:15PM +1100, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
> > access.
> > 
> > -- >8 --
> > 
> > When merging duplicate instantiations of function templates, currently
> > read_function_def overwrites the arguments with that of the existing
> > duplicate. This is problematic, however, since this means that the
> > PARM_DECLs in the body of the function definition no longer match with
> > the PARM_DECLs in the argument list, which causes issues when it comes
> > to generating RTL.
> > 
> > There doesn't seem to be any reason to do this replacement, so this
> > patch removes that logic.
> > 
> > PR c++/112588
> > 
> > gcc/cp/ChangeLog:
> > 
> > * module.cc (trees_in::read_function_def): Don't overwrite
> > arguments.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > * g++.dg/modules/merge-16.h: New test.
> > * g++.dg/modules/merge-16_a.C: New test.
> > * g++.dg/modules/merge-16_b.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead 
> > ---
> >  gcc/cp/module.cc  |  2 --
> >  gcc/testsuite/g++.dg/modules/merge-16.h   | 10 ++
> >  gcc/testsuite/g++.dg/modules/merge-16_a.C |  7 +++
> >  gcc/testsuite/g++.dg/modules/merge-16_b.C |  5 +
> >  4 files changed, 22 insertions(+), 2 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16.h
> >  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16_a.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16_b.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 4f5b6e2747a..2520ab659cc 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -11665,8 +11665,6 @@ trees_in::read_function_def (tree decl, tree 
> > maybe_template)
> >DECL_RESULT (decl) = result;
> >DECL_INITIAL (decl) = initial;
> >DECL_SAVED_TREE (decl) = saved;
> > -  if (maybe_dup)
> > -   DECL_ARGUMENTS (decl) = DECL_ARGUMENTS (maybe_dup);
> >  
> >if (context)
> > SET_DECL_FRIEND_CONTEXT (decl, context);
> > diff --git a/gcc/testsuite/g++.dg/modules/merge-16.h 
> > b/gcc/testsuite/g++.dg/modules/merge-16.h
> > new file mode 100644
> > index 000..fdb38551103
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/merge-16.h
> > @@ -0,0 +1,10 @@
> > +// PR c++/112588
> > +
> > +void f(int*);
> > +
> > +template 
> > +struct S {
> > +  void g(int n) { f(); }
> > +};
> > +
> > +template struct S;
> > diff --git a/gcc/testsuite/g++.dg/modules/merge-16_a.C 
> > b/gcc/testsuite/g++.dg/modules/merge-16_a.C
> > new file mode 100644
> > index 000..c243224c875
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/merge-16_a.C
> > @@ -0,0 +1,7 @@
> > +// PR c++/112588
> > +// { dg-additional-options "-fmodules-ts" }
> > +// { dg-module-cmi merge16 }
> > +
> > +module;
> > +#include "merge-16.h"
> > +export module merge16;
> > diff --git a/gcc/testsuite/g++.dg/modules/merge-16_b.C 
> > b/gcc/testsuite/g++.dg/modules/merge-16_b.C
> > new file mode 100644
> > index 000..8c7b1f0511f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/merge-16_b.C
> > @@ -0,0 +1,5 @@
> > +// PR c++/112588
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +#include "merge-16.h"
> > +import merge16;
> > -- 
> > 2.42.0
> > 


[PATCH] c++/modules: Prevent overwriting arguments when merging duplicates [PR112588]

2023-12-16 Thread Nathaniel Shead
Ping for https://gcc.gnu.org/pipermail/gcc-patches/2023-November/637768.html.
(I've changed the summary message a little from that email but the patch
is otherwise unchanged.)

On Wed, Nov 22, 2023 at 10:33:15PM +1100, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu. I don't have write
> access.
> 
> -- >8 --
> 
> When merging duplicate instantiations of function templates, currently
> read_function_def overwrites the arguments with that of the existing
> duplicate. This is problematic, however, since this means that the
> PARM_DECLs in the body of the function definition no longer match with
> the PARM_DECLs in the argument list, which causes issues when it comes
> to generating RTL.
> 
> There doesn't seem to be any reason to do this replacement, so this
> patch removes that logic.
> 
>   PR c++/112588
> 
> gcc/cp/ChangeLog:
> 
>   * module.cc (trees_in::read_function_def): Don't overwrite
>   arguments.
> 
> gcc/testsuite/ChangeLog:
> 
>   * g++.dg/modules/merge-16.h: New test.
>   * g++.dg/modules/merge-16_a.C: New test.
>   * g++.dg/modules/merge-16_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead 
> ---
>  gcc/cp/module.cc  |  2 --
>  gcc/testsuite/g++.dg/modules/merge-16.h   | 10 ++
>  gcc/testsuite/g++.dg/modules/merge-16_a.C |  7 +++
>  gcc/testsuite/g++.dg/modules/merge-16_b.C |  5 +
>  4 files changed, 22 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16.h
>  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16_a.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/merge-16_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 4f5b6e2747a..2520ab659cc 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -11665,8 +11665,6 @@ trees_in::read_function_def (tree decl, tree 
> maybe_template)
>DECL_RESULT (decl) = result;
>DECL_INITIAL (decl) = initial;
>DECL_SAVED_TREE (decl) = saved;
> -  if (maybe_dup)
> - DECL_ARGUMENTS (decl) = DECL_ARGUMENTS (maybe_dup);
>  
>if (context)
>   SET_DECL_FRIEND_CONTEXT (decl, context);
> diff --git a/gcc/testsuite/g++.dg/modules/merge-16.h 
> b/gcc/testsuite/g++.dg/modules/merge-16.h
> new file mode 100644
> index 000..fdb38551103
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-16.h
> @@ -0,0 +1,10 @@
> +// PR c++/112588
> +
> +void f(int*);
> +
> +template 
> +struct S {
> +  void g(int n) { f(); }
> +};
> +
> +template struct S;
> diff --git a/gcc/testsuite/g++.dg/modules/merge-16_a.C 
> b/gcc/testsuite/g++.dg/modules/merge-16_a.C
> new file mode 100644
> index 000..c243224c875
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-16_a.C
> @@ -0,0 +1,7 @@
> +// PR c++/112588
> +// { dg-additional-options "-fmodules-ts" }
> +// { dg-module-cmi merge16 }
> +
> +module;
> +#include "merge-16.h"
> +export module merge16;
> diff --git a/gcc/testsuite/g++.dg/modules/merge-16_b.C 
> b/gcc/testsuite/g++.dg/modules/merge-16_b.C
> new file mode 100644
> index 000..8c7b1f0511f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/merge-16_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/112588
> +// { dg-additional-options "-fmodules-ts" }
> +
> +#include "merge-16.h"
> +import merge16;
> -- 
> 2.42.0
>