| Message ID | 677bcb0c.170a0220.3b0b63.0c69@mx.google.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Add some missing LAMBDA_EXPR_EXTRA_SCOPEs | expand |
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
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 } }
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(-)