[v3,3/6] c++: Fix ABI for lambdas declared in alias templates [PR116568]
Commit Message
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
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
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
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
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
@@ -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));
@@ -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 ();
@@ -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 *-*-* } } }
@@ -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 } }