On Thu, 2 May 2024, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk/14.2? > > Another alternative would be to stream such !DECL_NAME temporaries with > a merge key of MK_unique rather than attempting to find the matching > (nonexistant) field of the class context.
Both approaches sound good to me, hard to say which one is preferable.. The handling of function-scope vs class-scope temporaries seems to start diverging in: @@ -8861,28 +8861,6 @@ trees_out::decl_node (tree decl, walk_kind ref) return false; } ! tree ctx = CP_DECL_CONTEXT (decl); ! depset *dep = NULL; ! if (streaming_p ()) ! dep = dep_hash->find_dependency (decl); ! else if (TREE_CODE (ctx) != FUNCTION_DECL ! || TREE_CODE (decl) == TEMPLATE_DECL ! || DECL_IMPLICIT_TYPEDEF_P (decl) ! || (DECL_LANG_SPECIFIC (decl) ! && DECL_MODULE_IMPORT_P (decl))) ! { ! auto kind = (TREE_CODE (decl) == NAMESPACE_DECL ! && !DECL_NAMESPACE_ALIAS (decl) ! ? depset::EK_NAMESPACE : depset::EK_DECL); ! dep = dep_hash->add_dependency (decl, kind); ! } ! ! if (!dep) ! { ! /* Some internal entity of context. Do by value. */ ! decl_value (decl, NULL); ! return false; ! } if (dep->get_entity_kind () == depset::EK_REDIRECT) { where for a class-scope temporary we add a dependency for it, stream it by reference, and then stream it by value separately, which seems unnecessary. So if we decide to keep the create_temporary_var change, we probably would want to unify this code path's handling of temporaries (i.e. don't add_dependency a temporary regardless of its context). If we decide your partially revert the create_temporary_var change, your patch LGTM. > > -- >8 -- > > In r14-9266-g2823b4d96d9ec4 I gave all temporary vars a DECL_CONTEXT, > including those at namespace or global scope, so that they could be > properly merged across importers. However, not all of these temporary > vars are actually supposed to be mergeable. > > For instance, in the attached testcase we have an unnamed temporary var > used in the NSDMI of a class member, which cannot properly merged -- but > it also doesn't need to be, as it'll be thrown away when the class type > itself is merged anyway. > > This patch reverts the change made above and instead makes a weaker > adjustment that only causes temporary vars with linkage have a > DECL_CONTEXT to merge from. This way these unnamed, "unmergeable" > temporaries are properly streamed by value again. > > PR c++/114856 > > gcc/cp/ChangeLog: > > * call.cc (make_temporary_var_for_ref_to_temp): Set context for > temporaries with linkage. > * init.cc (create_temporary_var): Revert to only set context > when in a function decl. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/pr114856.h: New test. > * g++.dg/modules/pr114856_a.H: New test. > * g++.dg/modules/pr114856_b.C: New test. > > Signed-off-by: Nathaniel Shead <nathanielosh...@gmail.com> > --- > gcc/cp/call.cc | 1 + > gcc/cp/init.cc | 2 +- > gcc/testsuite/g++.dg/modules/pr114856.h | 12 ++++++++++++ > gcc/testsuite/g++.dg/modules/pr114856_a.H | 5 +++++ > gcc/testsuite/g++.dg/modules/pr114856_b.C | 5 +++++ > 5 files changed, 24 insertions(+), 1 deletion(-) > create mode 100644 gcc/testsuite/g++.dg/modules/pr114856.h > create mode 100644 gcc/testsuite/g++.dg/modules/pr114856_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/pr114856_b.C > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc > index dbdd7c29fe8..3b8889ac301 100644 > --- a/gcc/cp/call.cc > +++ b/gcc/cp/call.cc > @@ -13799,6 +13799,7 @@ make_temporary_var_for_ref_to_temp (tree decl, tree > type) > > tree name = mangle_ref_init_variable (decl); > DECL_NAME (var) = name; > + DECL_CONTEXT (var) = current_scope (); > SET_DECL_ASSEMBLER_NAME (var, name); > } > else > diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc > index a93ce00800c..e758a8c8568 100644 > --- a/gcc/cp/init.cc > +++ b/gcc/cp/init.cc > @@ -4287,7 +4287,7 @@ create_temporary_var (tree type) > TREE_USED (decl) = 1; > DECL_ARTIFICIAL (decl) = 1; > DECL_IGNORED_P (decl) = 1; > - DECL_CONTEXT (decl) = current_scope (); > + DECL_CONTEXT (decl) = current_function_decl; > > return decl; > } > diff --git a/gcc/testsuite/g++.dg/modules/pr114856.h > b/gcc/testsuite/g++.dg/modules/pr114856.h > new file mode 100644 > index 00000000000..b1a3c2cd834 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr114856.h > @@ -0,0 +1,12 @@ > +// PR c++/114856 > + > +#include <initializer_list> > +struct A { > + ~A(); > +}; > +struct V { > + V(std::initializer_list<A>); > +}; > +struct data { > + V v{{}}; > +}; > diff --git a/gcc/testsuite/g++.dg/modules/pr114856_a.H > b/gcc/testsuite/g++.dg/modules/pr114856_a.H > new file mode 100644 > index 00000000000..6195277dbde > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr114856_a.H > @@ -0,0 +1,5 @@ > +// PR c++/114856 > +// { dg-additional-options "-fmodule-header" } > +// { dg-module-cmi {} } > + > +#include "pr114856.h" > diff --git a/gcc/testsuite/g++.dg/modules/pr114856_b.C > b/gcc/testsuite/g++.dg/modules/pr114856_b.C > new file mode 100644 > index 00000000000..f81dc8b81d5 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr114856_b.C > @@ -0,0 +1,5 @@ > +// PR c++/114856 > +// { dg-additional-options "-fmodules-ts" } > + > +#include "pr114856.h" > +import "pr114856_a.H"; > -- > 2.43.2 > >