[v3,3/6] c++: Fix ABI for lambdas declared in alias templates [PR116568]

Message ID 677bcb0c.170a0220.3b0b63.0c69@mx.google.com
State New
Headers
Series c++: Add some missing LAMBDA_EXPR_EXTRA_SCOPEs |

Commit Message

Nathaniel Shead Jan. 6, 2025, 12:22 p.m. UTC
  I'm not 100% sure I've handled this properly, any feedback welcome.
In particular, maybe should I use `DECL_IMPLICIT_TYPEDEF_P` in the
mangling logic instead of `!TYPE_DECL_ALIAS_P`?  They both seem to work
in this case but not sure which would be clearer.

I also looked into trying do a limited form of 'start_decl' before
parsing the type but there were too many circular dependencies for me to
work through, so I think any such changes would have to wait till GCC16
(if they're even possible at all).

-- >8 --

This adds mangling support for lambdas with a mangling context of an
alias template, and gives that context when instantiating such a lambda.

This only currently works for class-scope alias templates, however, due
to the

  if (LAMBDA_EXPR_EXTRA_SCOPE (t))
    record_lambda_scope (r);

condition in 'tsubst_lambda_scope'.

For namespace-scope alias templates, we can't easily add the mangling
context: we can't build the TYPE_DECL to record against until after
we've parsed the type (and already recorded lambda scope), as
`start_decl` relies on the type being passed in correctly, and setting
the mangling scope after parsing is too late because e.g.
'template_class_depth' (called from grokfndecl when building the lambda
functions while parsing the type) relies on the LAMBDA_EXPR_EXTRA_SCOPE
already being properly set.  This will also likely matter for
'determine_visibility'.  I'm not sure what a good way to break this
recursive dependency is.

This change also requires a slight adjustment to the late-ret-3
testcase, as changing the order of creating the node for the alias
adjusted the tiebreak sorting of cluster members when logging.

	PR c++/116568

gcc/cp/ChangeLog:

	* mangle.cc (maybe_template_info): Support getting template info
	of alias templates.
	(canonicalize_for_substitution): Don't canonicalise aliases.
	(decl_mangling_context): Don't treat aliases as lambda closure
	types.
	(write_unqualified_name): Likewise.
	* pt.cc (tsubst_decl): Start lambda scope for alias templates.
	(instantiate_template): No longer need to special case alias
	templates here.

gcc/testsuite/ChangeLog:

	* g++.dg/abi/lambda-ctx4.C: Adjust mangling, include namespace
	scope alias templates (XFAILed for now).
	* g++.dg/modules/late-ret-3_a.H: Adjust cluster order.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/mangle.cc                            | 13 +++++++-----
 gcc/cp/pt.cc                                | 23 +++++++++------------
 gcc/testsuite/g++.dg/abi/lambda-ctx4.C      | 21 ++++++++++++-------
 gcc/testsuite/g++.dg/modules/late-ret-3_a.H |  2 +-
 4 files changed, 33 insertions(+), 26 deletions(-)
  

Comments

Jason Merrill Jan. 17, 2025, 12:09 a.m. UTC | #1
On 1/6/25 7:22 AM, Nathaniel Shead wrote:
> I'm not 100% sure I've handled this properly, any feedback welcome.
> In particular, maybe should I use `DECL_IMPLICIT_TYPEDEF_P` in the
> mangling logic instead of `!TYPE_DECL_ALIAS_P`?  They both seem to work
> in this case but not sure which would be clearer.
> 
> I also looked into trying do a limited form of 'start_decl' before
> parsing the type but there were too many circular dependencies for me to
> work through, so I think any such changes would have to wait till GCC16
> (if they're even possible at all).
> 
> -- >8 --
> 
> This adds mangling support for lambdas with a mangling context of an
> alias template, and gives that context when instantiating such a lambda.

I think this is wrong, an alias is not an entity so it is not a 
definable item.

The ABI change proposal also doesn't mention aliases.

Jason
  
Nathaniel Shead Jan. 17, 2025, 12:24 a.m. UTC | #2
On Thu, Jan 16, 2025 at 07:09:33PM -0500, Jason Merrill wrote:
> On 1/6/25 7:22 AM, Nathaniel Shead wrote:
> > I'm not 100% sure I've handled this properly, any feedback welcome.
> > In particular, maybe should I use `DECL_IMPLICIT_TYPEDEF_P` in the
> > mangling logic instead of `!TYPE_DECL_ALIAS_P`?  They both seem to work
> > in this case but not sure which would be clearer.
> > 
> > I also looked into trying do a limited form of 'start_decl' before
> > parsing the type but there were too many circular dependencies for me to
> > work through, so I think any such changes would have to wait till GCC16
> > (if they're even possible at all).
> > 
> > -- >8 --
> > 
> > This adds mangling support for lambdas with a mangling context of an
> > alias template, and gives that context when instantiating such a lambda.
> 
> I think this is wrong, an alias is not an entity so it is not a definable
> item.
> 
> The ABI change proposal also doesn't mention aliases.
> 
> Jason
> 

Ah right, I see; I'd treated https://eel.is/c++draft/basic.def.odr#1.5
as being any template, but I see now it's "any templated entity" which
is different (since as you say an alias isn't an entity).

In that case, how do you think we should handle class-scope alias
templates of lambdas?  Such a class is surely a definable item, and so
e.g. 

  struct S {
    template <int I>
    using X = decltype([]{ return I; });
  };
  using L1 = S::X<1>;
  using L2 = S::X<2>;

should this work and declare L1 to be the same type across TUs?
In which case it would need mangling to include the template arguments.
Or because this is a template instantiation are there different rules?

The alternative would of course be that such lambdas are TU-local, which
is what I believe Clang currently does.

Nathaniel
  
Jason Merrill Jan. 22, 2025, 11:11 p.m. UTC | #3
On 1/16/25 7:24 PM, Nathaniel Shead wrote:
> On Thu, Jan 16, 2025 at 07:09:33PM -0500, Jason Merrill wrote:
>> On 1/6/25 7:22 AM, Nathaniel Shead wrote:
>>> I'm not 100% sure I've handled this properly, any feedback welcome.
>>> In particular, maybe should I use `DECL_IMPLICIT_TYPEDEF_P` in the
>>> mangling logic instead of `!TYPE_DECL_ALIAS_P`?  They both seem to work
>>> in this case but not sure which would be clearer.
>>>
>>> I also looked into trying do a limited form of 'start_decl' before
>>> parsing the type but there were too many circular dependencies for me to
>>> work through, so I think any such changes would have to wait till GCC16
>>> (if they're even possible at all).
>>>
>>> -- >8 --
>>>
>>> This adds mangling support for lambdas with a mangling context of an
>>> alias template, and gives that context when instantiating such a lambda.
>>
>> I think this is wrong, an alias is not an entity so it is not a definable
>> item.
>>
>> The ABI change proposal also doesn't mention aliases.
> 
> Ah right, I see; I'd treated https://eel.is/c++draft/basic.def.odr#1.5
> as being any template, but I see now it's "any templated entity" which
> is different (since as you say an alias isn't an entity).
> 
> In that case, how do you think we should handle class-scope alias
> templates of lambdas?  Such a class is surely a definable item, and so
> e.g.
> 
>    struct S {
>      template <int I>
>      using X = decltype([]{ return I; });
>    };
>    using L1 = S::X<1>;
>    using L2 = S::X<2>;
> 
> should this work and declare L1 to be the same type across TUs?

Hmm, I suppose it should.  So then using the alias template name in the 
mangling is then not because it's a definable item, but just as a 
convenient label to indicate where it appears in the class and what the 
template arguments apply to.

But even with that understanding, many of the changes in this patch to 
make aliases more special seem wrong, we shouldn't need those just to 
push/pop lambda scope?

Jason
  
Jason Merrill Jan. 23, 2025, 3:51 p.m. UTC | #4
On 1/22/25 6:11 PM, Jason Merrill wrote:
> On 1/16/25 7:24 PM, Nathaniel Shead wrote:
>> On Thu, Jan 16, 2025 at 07:09:33PM -0500, Jason Merrill wrote:
>>> On 1/6/25 7:22 AM, Nathaniel Shead wrote:
>>>> I'm not 100% sure I've handled this properly, any feedback welcome.
>>>> In particular, maybe should I use `DECL_IMPLICIT_TYPEDEF_P` in the
>>>> mangling logic instead of `!TYPE_DECL_ALIAS_P`?  They both seem to work
>>>> in this case but not sure which would be clearer.
>>>>
>>>> I also looked into trying do a limited form of 'start_decl' before
>>>> parsing the type but there were too many circular dependencies for 
>>>> me to
>>>> work through, so I think any such changes would have to wait till GCC16
>>>> (if they're even possible at all).
>>>>
>>>> -- >8 --
>>>>
>>>> This adds mangling support for lambdas with a mangling context of an
>>>> alias template, and gives that context when instantiating such a 
>>>> lambda.
>>>
>>> I think this is wrong, an alias is not an entity so it is not a 
>>> definable
>>> item.
>>>
>>> The ABI change proposal also doesn't mention aliases.
>>
>> Ah right, I see; I'd treated https://eel.is/c++draft/basic.def.odr#1.5
>> as being any template, but I see now it's "any templated entity" which
>> is different (since as you say an alias isn't an entity).
>>
>> In that case, how do you think we should handle class-scope alias
>> templates of lambdas?  Such a class is surely a definable item, and so
>> e.g.
>>
>>    struct S {
>>      template <int I>
>>      using X = decltype([]{ return I; });
>>    };
>>    using L1 = S::X<1>;
>>    using L2 = S::X<2>;
>>
>> should this work and declare L1 to be the same type across TUs?
> 
> Hmm, I suppose it should.  So then using the alias template name in the 
> mangling is then not because it's a definable item, but just as a 
> convenient label to indicate where it appears in the class and what the 
> template arguments apply to.

Actually, on rereading I think my interpretation was wrong:

https://eel.is/c++draft/basic#pre-3 says a template is an entity.
https://eel.is/c++draft/temp.pre#8.1 says an entity is templated if it 
is a template.
https://eel.is/c++draft/basic#def.odr-1.5 says a templated entity is a 
definable item.

So, an alias template is a definable item even if a non-template alias 
is not.

But a lambda in a namespace-scope alias template is still clearly 
TU-local under https://eel.is/c++draft/basic#link-15.2 .

Jason
  

Patch

diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 170dafd52c1..9d457e2a2f3 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -292,7 +292,7 @@  abi_check (int ver)
 static tree
 maybe_template_info (const tree decl)
 {
-  if (TREE_CODE (decl) == TYPE_DECL)
+  if (TREE_CODE (decl) == TYPE_DECL && !TYPE_DECL_ALIAS_P (decl))
     {
       /* TYPE_DECLs are handled specially.  Look at its type to decide
 	 if this is a template instantiation.  */
@@ -305,7 +305,7 @@  maybe_template_info (const tree decl)
     {
       /* Check if the template is a primary template.  */
       if (DECL_LANG_SPECIFIC (decl) != NULL
-	  && VAR_OR_FUNCTION_DECL_P (decl)
+	  && (VAR_OR_FUNCTION_DECL_P (decl) || TREE_CODE (decl) == TYPE_DECL)
 	  && DECL_TEMPLATE_INFO (decl)
 	  && PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (decl)))
 	return DECL_TEMPLATE_INFO (decl);
@@ -402,8 +402,8 @@  write_exception_spec (tree spec)
 static inline tree
 canonicalize_for_substitution (tree node)
 {
-  /* For a TYPE_DECL, use the type instead.  */
-  if (TREE_CODE (node) == TYPE_DECL)
+  /* For a non-alias TYPE_DECL, use the type instead.  */
+  if (TREE_CODE (node) == TYPE_DECL && !TYPE_DECL_ALIAS_P (node))
     node = TREE_TYPE (node);
   if (TYPE_P (node)
       && TYPE_CANONICAL (node) != node
@@ -1044,6 +1044,7 @@  decl_mangling_context (tree decl)
     decl = DECL_TEMPLATE_RESULT (decl);
 
   if (TREE_CODE (decl) == TYPE_DECL
+      && !TYPE_DECL_ALIAS_P (decl)
       && LAMBDA_TYPE_P (TREE_TYPE (decl)))
     {
       tree extra = LAMBDA_TYPE_EXTRA_SCOPE (TREE_TYPE (decl));
@@ -1588,7 +1589,9 @@  write_unqualified_name (tree decl)
       if (TREE_CODE (decl) == TYPE_DECL
           && TYPE_UNNAMED_P (type))
         write_unnamed_type_name (type);
-      else if (TREE_CODE (decl) == TYPE_DECL && LAMBDA_TYPE_P (type))
+      else if (TREE_CODE (decl) == TYPE_DECL
+	       && !TYPE_DECL_ALIAS_P (decl)
+	       && LAMBDA_TYPE_P (type))
         write_closure_type_name (type);
       else
         write_source_name (DECL_NAME (decl));
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index cb234b53a55..cba7b97ef70 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -15753,6 +15753,7 @@  tsubst_decl (tree t, tree args, tsubst_flags_t complain,
 	  }
 
 	/* Create a new node for the specialization we need.  */
+	r = copy_decl (t);
 	if (type == NULL_TREE)
 	  {
 	    if (is_typedef_decl (t))
@@ -15766,19 +15767,17 @@  tsubst_decl (tree t, tree args, tsubst_flags_t complain,
 	    tsubst_flags_t tcomplain = complain;
 	    if (VAR_P (t))
 	      tcomplain |= tf_tst_ok;
+	    bool is_alias
+	      = (is_typedef_decl (t)
+		 && alias_template_specialization_p (TREE_TYPE (t), nt_opaque));
+	    if (is_alias)
+	      start_lambda_scope (r);
 	    type = tsubst (type, args, tcomplain, in_decl);
-	    /* Substituting the type might have recursively instantiated this
-	       same alias (c++/86171).  */
-	    if (use_spec_table && gen_tmpl && DECL_ALIAS_TEMPLATE_P (gen_tmpl)
-		&& (spec = retrieve_specialization (gen_tmpl, argvec, hash)))
-	      {
-		r = spec;
-		break;
-	      }
+	    if (is_alias)
+	      finish_lambda_scope ();
 	  }
 	if (type == error_mark_node && !(complain & tf_error))
 	  RETURN (error_mark_node);
-	r = copy_decl (t);
 	if (VAR_P (r))
 	  {
 	    DECL_INITIALIZED_P (r) = 0;
@@ -22536,8 +22535,7 @@  instantiate_template (tree tmpl, tree orig_args, tsubst_flags_t complain)
 	ctx = tsubst_entering_scope (DECL_CONTEXT (gen_tmpl), targ_ptr,
 				     complain, gen_tmpl);
       push_nested_class (ctx);
-      if (!DECL_ALIAS_TEMPLATE_P (gen_tmpl))
-	start_lambda_scope (TYPE_NAME (ctx));
+      start_lambda_scope (TYPE_NAME (ctx));
     }
 
   tree pattern = DECL_TEMPLATE_RESULT (gen_tmpl);
@@ -22568,8 +22566,7 @@  instantiate_template (tree tmpl, tree orig_args, tsubst_flags_t complain)
     fndecl = tsubst_decl (pattern, targ_ptr, complain, /*use_spec_table=*/false);
   if (DECL_CLASS_SCOPE_P (gen_tmpl))
     {
-      if (!DECL_ALIAS_TEMPLATE_P (gen_tmpl))
-	finish_lambda_scope ();
+      finish_lambda_scope ();
       pop_nested_class ();
     }
   pop_from_top_level ();
diff --git a/gcc/testsuite/g++.dg/abi/lambda-ctx4.C b/gcc/testsuite/g++.dg/abi/lambda-ctx4.C
index d6544a84652..809cace8574 100644
--- a/gcc/testsuite/g++.dg/abi/lambda-ctx4.C
+++ b/gcc/testsuite/g++.dg/abi/lambda-ctx4.C
@@ -1,5 +1,4 @@ 
 // { dg-do compile { target c++20 } }
-// { dg-additional-options "-fkeep-inline-functions" }
 
 struct S {
   template <int I>
@@ -9,14 +8,22 @@  struct S {
 S::T<0> a;
 S::T<1> b;
 
+template <int I>
+using L = decltype([]{ return I; });
+
+L<0> c;
+L<1> d;
+
 int main() {
   a();
   b();
+  c();
+  d();
 }
 
-// Currently we don't implement any special mangling rules for template aliases
-// (though we probably should; an alias template is a definable item by
-// [basic.def.odr] p1.5 and as such contained lambdas in different TUs should have
-// the same type, see [basic.def.odr] p15.6)
-// { scan_assembler {_ZNK1SUlvE_clEv:} }
-// { scan_assembler {_ZNK1SUlvE0_clEv:} }
+// { dg-final { scan-assembler {_ZNK1S1TILi0EEUlvE_clEv:} } }
+// { dg-final { scan-assembler {_ZNK1S1TILi1EEUlvE_clEv:} } }
+
+// namespace-scope aliases don't have LAMBDA_EXPR_EXTRA_CONTEXT properly set
+// { dg-final { scan-assembler {_ZNK1LILi0EEUlvE_clEv:} { xfail *-*-* } } }
+// { dg-final { scan-assembler {_ZNK1LILi1EEUlvE_clEv:} { xfail *-*-* } } }
diff --git a/gcc/testsuite/g++.dg/modules/late-ret-3_a.H b/gcc/testsuite/g++.dg/modules/late-ret-3_a.H
index 54f95db0456..e64a6f340c0 100644
--- a/gcc/testsuite/g++.dg/modules/late-ret-3_a.H
+++ b/gcc/testsuite/g++.dg/modules/late-ret-3_a.H
@@ -17,4 +17,4 @@  auto Bar (const A& arg)
   -> TPL_3<typename TPL_1<decltype(arg)>::type>
   {return 3;}
 
-// { dg-final { scan-lang-dump { Cluster members:\n  \[0\]=decl definition '::template Foo'\n  \[1\]=specialization declaration '::TPL_1<#null#>'\n  \[2\]=specialization declaration '::TPL_3<::TPL_1<#null#>::type>'\n  \[3\]=specialization declaration '::TPL_2<::TPL_1<#null#>::type>'\n  \[4\]=binding '::Foo'\n} module } }
+// { dg-final { scan-lang-dump { Cluster members:\n  \[0\]=decl definition '::template Foo'\n  \[1\]=specialization declaration '::TPL_3<::TPL_1<#null#>::type>'\n  \[2\]=specialization declaration '::TPL_1<#null#>'\n  \[3\]=specialization declaration '::TPL_2<::TPL_1<#null#>::type>'\n  \[4\]=binding '::Foo'\n} module } }