* [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] @ 2024-02-10 22:57 Nathaniel Shead 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead 2024-02-14 0:52 ` [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Jason Merrill 0 siblings, 2 replies; 12+ messages in thread From: Nathaniel Shead @ 2024-02-10 22:57 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? Or should I re-introduce the tree checking and just add checks for the new kinds of declarations that could be have keyed decls? -- >8 -- The fix for PR107398 weakened the restrictions that lambdas must belong to namespace scope. However this was not sufficient: we also need to allow lambdas keyed to FIELD_DECLs or PARM_DECLs. Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a fairly large number of different kinds of DECLs, and that in general it's safe to just get 'false' as a result of a check on an unexpected DECL type, this patch also removes the tree checking from the accessor. gcc/cp/ChangeLog: * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. (struct lang_decl_base): Update comments and fix whitespace. * module.cc (trees_out::lang_decl_bools): Always write module_keyed_decls_p flag... (trees_in::lang_decl_bools): ...and always read it. (trees_out::decl_value): Also handle keyed decls on decls other than VAR_DECL or FUNCTION_DECL. (trees_in::decl_value): Likewise. (trees_out::get_merge_kind): Likewise. (maybe_key_decl): Also handle lambdas attached to FIELD_DECLs and PARM_DECLS. (trees_out::key_mergeable): Likewise. (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL container. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7_a.C: New test. * g++.dg/modules/lambda-7_b.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/cp-tree.h | 23 ++++---- gcc/cp/module.cc | 70 +++++++++++------------ gcc/testsuite/g++.dg/modules/lambda-7_a.C | 20 +++++++ gcc/testsuite/g++.dg/modules/lambda-7_b.C | 16 ++++++ 4 files changed, 81 insertions(+), 48 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.C create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 334c11396c2..6ab82dc2d0f 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) /* DECL that has attached decls for ODR-relatedness. */ -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ - ->u.base.module_keyed_decls_p) +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) /* Whether this is an exported DECL. Held on any decl that can appear at namespace scope (function, var, type, template, const or @@ -2887,21 +2886,19 @@ struct GTY(()) lang_decl_base { unsigned friend_or_tls : 1; /* var, fn, type or template */ unsigned unknown_bound_p : 1; /* var */ unsigned odr_used : 1; /* var or fn */ - unsigned concept_p : 1; /* applies to vars and functions */ + unsigned concept_p : 1; /* applies to vars and functions */ unsigned var_declared_inline_p : 1; /* var */ unsigned dependent_init_p : 1; /* var */ /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE decls. */ - unsigned module_purview_p : 1; // in named-module purview - unsigned module_attach_p : 1; // attached to named module - unsigned module_import_p : 1; /* from an import */ - unsigned module_entity_p : 1; /* is in the entitity ary & - hash. */ - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ - unsigned module_keyed_decls_p : 1; - - /* 12 spare bits. */ + unsigned module_purview_p : 1; /* in named-module purview */ + unsigned module_attach_p : 1; /* attached to named module */ + unsigned module_import_p : 1; /* from an import */ + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ + + /* 11 spare bits. */ }; /* True for DECL codes which have template info and access. */ diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 560d8f3b614..9742bca922c 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t) want to mark them as in module purview. */ WB (lang->u.base.module_purview_p && !header_module_p ()); WB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - WB (lang->u.base.module_keyed_decls_p); + WB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -5738,8 +5737,7 @@ trees_in::lang_decl_bools (tree t) RB (lang->u.base.dependent_init_p); RB (lang->u.base.module_purview_p); RB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - RB (lang->u.base.module_keyed_decls_p); + RB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -7869,8 +7867,7 @@ trees_out::decl_value (tree decl, depset *dep) install_entity (decl, dep); } - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner) && !is_key_order ()) { @@ -8170,8 +8167,7 @@ trees_in::decl_value () bool installed = install_entity (existing); bool is_new = existing == decl; - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner)) { /* Read and maybe install the attached entities. */ @@ -10482,8 +10478,7 @@ trees_out::get_merge_kind (tree decl, depset *dep) if (tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (decl)))) - if (TREE_CODE (scope) == VAR_DECL - && DECL_MODULE_KEYED_DECLS_P (scope)) + if (DECL_MODULE_KEYED_DECLS_P (scope)) { mk = MK_keyed; break; @@ -10787,7 +10782,9 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (inner))); - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL + || TREE_CODE (scope) == FIELD_DECL + || TREE_CODE (scope) == PARM_DECL); auto *root = keyed_table->get (scope); unsigned ix = root->length (); /* If we don't find it, we'll write a really big number @@ -11065,6 +11062,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, } } } + else if (mk == MK_keyed + && DECL_LANG_SPECIFIC (name) + && DECL_MODULE_KEYED_DECLS_P (name)) + { + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL + || TREE_CODE (container) == TYPE_DECL); + if (auto *set = keyed_table->get (name)) + if (key.index < set->length ()) + { + existing = (*set)[key.index]; + if (existing) + { + gcc_checking_assert + (DECL_IMPLICIT_TYPEDEF_P (existing)); + if (inner != decl) + existing + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); + } + } + } else switch (TREE_CODE (container)) { @@ -11072,27 +11089,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_unreachable (); case NAMESPACE_DECL: - if (mk == MK_keyed) - { - if (DECL_LANG_SPECIFIC (name) - && VAR_OR_FUNCTION_DECL_P (name) - && DECL_MODULE_KEYED_DECLS_P (name)) - if (auto *set = keyed_table->get (name)) - if (key.index < set->length ()) - { - existing = (*set)[key.index]; - if (existing) - { - gcc_checking_assert - (DECL_IMPLICIT_TYPEDEF_P (existing)); - if (inner != decl) - existing - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); - } - } - } - else if (is_attached - && !(state->is_module () || state->is_partition ())) + if (is_attached + && !(state->is_module () || state->is_partition ())) kind = "unique"; else { @@ -18981,9 +18979,11 @@ maybe_key_decl (tree ctx, tree decl) if (!modules_p ()) return; - // FIXME: For now just deal with lambdas attached to var decls. - // This might be sufficient? - if (TREE_CODE (ctx) != VAR_DECL) + /* We only need to deal with lambdas attached to var, field, + or parm decls. */ + if (TREE_CODE (ctx) != VAR_DECL + && TREE_CODE (ctx) != FIELD_DECL + && TREE_CODE (ctx) != PARM_DECL) return; if (!keyed_table) diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C new file mode 100644 index 00000000000..289285cd926 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C @@ -0,0 +1,20 @@ +// { dg-additional-options "-fmodules-ts" } +// { dg-module-cmi foo } + +export module foo; + +export struct S { + int (*a)(int) = [](int x) { return x * 2; }; + + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { + return f(x); + } + + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { + return f(x); + } +}; + +export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { + return f(x); +} diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C new file mode 100644 index 00000000000..a8762399ee1 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C @@ -0,0 +1,16 @@ +// { dg-module-do run } +// { dg-additional-options "-fmodules-ts" } + +import foo; + +int main() { + S s; + if (s.a(10) != 20) + __builtin_abort(); + if (s.b(10) != 30) + __builtin_abort(); + if (s.c(10) != 40) + __builtin_abort(); + if (d(10) != 50) + __builtin_abort(); +} -- 2.43.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules 2024-02-10 22:57 [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Nathaniel Shead @ 2024-02-11 3:54 ` Nathaniel Shead 2024-02-11 4:01 ` Andrew Pinski ` (2 more replies) 2024-02-14 0:52 ` [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Jason Merrill 1 sibling, 3 replies; 12+ messages in thread From: Nathaniel Shead @ 2024-02-11 3:54 UTC (permalink / raw) To: gcc-patches; +Cc: Jason Merrill, Nathan Sidwell Bootstrapped and regtested (so far just modules.exp and dg.exp) on x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? (Also I noticed I forgot to add the PR to the changelog in my last patch, I've fixed that locally.) -- >8 -- After fixing PR111710, I noticed that we currently ICE when declaring a type that derives from 'decltype([]{})'. As far as I can tell this should be legal code, since by [basic.link] p15.2 a lambda defined in a class-specifier should not be TU-local. This patch also adds a bunch of tests for unevaluated lambdas in other contexts, which generally seem to work now. One interesting case is 'E::f' in the attached testcase: it appears to get a merge kind of 'MK_field', rather than 'MK_keyed' as most other lambdas do. I'm not entirely sure if this will cause issues in the future, but I haven't been able to construct a testcase that causes problems with this, and conversely wrapping the class body in 'start_lambda_scope' causes issues with symbol duplication in COMDAT groups, so I've left it as-is for now. gcc/cp/ChangeLog: * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. (maybe_key_decl): Likewise. * parser.cc (cp_parser_class_head): Start a lambda scope when parsing base classes. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7_a.C: * g++.dg/modules/lambda-7_b.C: Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/module.cc | 8 +++++--- gcc/cp/parser.cc | 10 ++++++++-- gcc/testsuite/g++.dg/modules/lambda-7_a.C | 19 +++++++++++++++++++ gcc/testsuite/g++.dg/modules/lambda-7_b.C | 23 +++++++++++++++++++++++ 4 files changed, 55 insertions(+), 5 deletions(-) diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 9742bca922c..cceec79b26b 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -10784,7 +10784,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, (TREE_TYPE (inner))); gcc_checking_assert (TREE_CODE (scope) == VAR_DECL || TREE_CODE (scope) == FIELD_DECL - || TREE_CODE (scope) == PARM_DECL); + || TREE_CODE (scope) == PARM_DECL + || TREE_CODE (scope) == TYPE_DECL); auto *root = keyed_table->get (scope); unsigned ix = root->length (); /* If we don't find it, we'll write a really big number @@ -18980,10 +18981,11 @@ maybe_key_decl (tree ctx, tree decl) return; /* We only need to deal with lambdas attached to var, field, - or parm decls. */ + parm, or type decls. */ if (TREE_CODE (ctx) != VAR_DECL && TREE_CODE (ctx) != FIELD_DECL - && TREE_CODE (ctx) != PARM_DECL) + && TREE_CODE (ctx) != PARM_DECL + && TREE_CODE (ctx) != TYPE_DECL) return; if (!keyed_table) diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 09ecfa23b5d..151e724ed66 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27663,10 +27663,16 @@ cp_parser_class_head (cp_parser* parser, if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) { if (type) - pushclass (type); + { + pushclass (type); + start_lambda_scope (TYPE_NAME (type)); + } bases = cp_parser_base_clause (parser); if (type) - popclass (); + { + finish_lambda_scope (); + popclass (); + } } else bases = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C index 289285cd926..9a23827a280 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-7_a.C +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C @@ -18,3 +18,22 @@ export struct S { export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { return f(x); } + +// unevaluated lambdas +#if __cplusplus >= 202002L +export struct E : decltype([](int x) { return x * 6; }) { + decltype([](int x) { return x * 7; }) f; +}; + +export template <typename T> +struct G : decltype([](int x) { return x * 8; }) { + decltype([](int x) { return x * 9; }) h; +}; + +template <> +struct G<double> : decltype([](int x) { return x * 10; }) { + decltype([](int x) { return x * 11; }) i; +}; + +export decltype([](int x) { return x * 12; }) j; +#endif diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C index a8762399ee1..59a82e05cbf 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-7_b.C +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C @@ -13,4 +13,27 @@ int main() { __builtin_abort(); if (d(10) != 50) __builtin_abort(); + +#if __cplusplus >= 202002L + E e; + if (e(10) != 60) + __builtin_abort(); + if (e.f(10) != 70) + __builtin_abort(); + + G<int> g1; + if (g1(10) != 80) + __builtin_abort(); + if (g1.h(10) != 90) + __builtin_abort(); + + G<double> g2; + if (g2(10) != 100) + __builtin_abort(); + if (g2.i(10) != 110) + __builtin_abort(); + + if (j(10) != 120) + __builtin_abort(); +#endif } -- 2.43.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead @ 2024-02-11 4:01 ` Andrew Pinski 2024-02-11 4:24 ` Nathaniel Shead 2024-02-12 13:12 ` Marek Polacek 2024-02-14 2:44 ` Jason Merrill 2 siblings, 1 reply; 12+ messages in thread From: Andrew Pinski @ 2024-02-11 4:01 UTC (permalink / raw) To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell On Sat, Feb 10, 2024 at 7:55 PM Nathaniel Shead <nathanieloshead@gmail.com> wrote: > > Bootstrapped and regtested (so far just modules.exp and dg.exp) on > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? > > (Also I noticed I forgot to add the PR to the changelog in my last > patch, I've fixed that locally.) > > -- >8 -- > > After fixing PR111710, I noticed that we currently ICE when declaring a > type that derives from 'decltype([]{})'. As far as I can tell this > should be legal code, since by [basic.link] p15.2 a lambda defined in a > class-specifier should not be TU-local. > > This patch also adds a bunch of tests for unevaluated lambdas in other > contexts, which generally seem to work now. There are many unevaluated lambdas (non-modules related) bugs report (all linked to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107430). Do you know if this fixes any of the non-module related ones too? Thanks, Andrew Pinski > > One interesting case is 'E::f' in the attached testcase: it appears to > get a merge kind of 'MK_field', rather than 'MK_keyed' as most other > lambdas do. I'm not entirely sure if this will cause issues in the > future, but I haven't been able to construct a testcase that causes > problems with this, and conversely wrapping the class body in > 'start_lambda_scope' causes issues with symbol duplication in COMDAT > groups, so I've left it as-is for now. > > gcc/cp/ChangeLog: > > * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. > (maybe_key_decl): Likewise. > * parser.cc (cp_parser_class_head): Start a lambda scope when > parsing base classes. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7_a.C: > * g++.dg/modules/lambda-7_b.C: > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/module.cc | 8 +++++--- > gcc/cp/parser.cc | 10 ++++++++-- > gcc/testsuite/g++.dg/modules/lambda-7_a.C | 19 +++++++++++++++++++ > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 23 +++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 5 deletions(-) > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 9742bca922c..cceec79b26b 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -10784,7 +10784,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > (TREE_TYPE (inner))); > gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > || TREE_CODE (scope) == FIELD_DECL > - || TREE_CODE (scope) == PARM_DECL); > + || TREE_CODE (scope) == PARM_DECL > + || TREE_CODE (scope) == TYPE_DECL); > auto *root = keyed_table->get (scope); > unsigned ix = root->length (); > /* If we don't find it, we'll write a really big number > @@ -18980,10 +18981,11 @@ maybe_key_decl (tree ctx, tree decl) > return; > > /* We only need to deal with lambdas attached to var, field, > - or parm decls. */ > + parm, or type decls. */ > if (TREE_CODE (ctx) != VAR_DECL > && TREE_CODE (ctx) != FIELD_DECL > - && TREE_CODE (ctx) != PARM_DECL) > + && TREE_CODE (ctx) != PARM_DECL > + && TREE_CODE (ctx) != TYPE_DECL) > return; > > if (!keyed_table) > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 09ecfa23b5d..151e724ed66 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -27663,10 +27663,16 @@ cp_parser_class_head (cp_parser* parser, > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > { > if (type) > - pushclass (type); > + { > + pushclass (type); > + start_lambda_scope (TYPE_NAME (type)); > + } > bases = cp_parser_base_clause (parser); > if (type) > - popclass (); > + { > + finish_lambda_scope (); > + popclass (); > + } > } > else > bases = NULL_TREE; > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > index 289285cd926..9a23827a280 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-7_a.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > @@ -18,3 +18,22 @@ export struct S { > export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > return f(x); > } > + > +// unevaluated lambdas > +#if __cplusplus >= 202002L > +export struct E : decltype([](int x) { return x * 6; }) { > + decltype([](int x) { return x * 7; }) f; > +}; > + > +export template <typename T> > +struct G : decltype([](int x) { return x * 8; }) { > + decltype([](int x) { return x * 9; }) h; > +}; > + > +template <> > +struct G<double> : decltype([](int x) { return x * 10; }) { > + decltype([](int x) { return x * 11; }) i; > +}; > + > +export decltype([](int x) { return x * 12; }) j; > +#endif > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > index a8762399ee1..59a82e05cbf 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-7_b.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > @@ -13,4 +13,27 @@ int main() { > __builtin_abort(); > if (d(10) != 50) > __builtin_abort(); > + > +#if __cplusplus >= 202002L > + E e; > + if (e(10) != 60) > + __builtin_abort(); > + if (e.f(10) != 70) > + __builtin_abort(); > + > + G<int> g1; > + if (g1(10) != 80) > + __builtin_abort(); > + if (g1.h(10) != 90) > + __builtin_abort(); > + > + G<double> g2; > + if (g2(10) != 100) > + __builtin_abort(); > + if (g2.i(10) != 110) > + __builtin_abort(); > + > + if (j(10) != 120) > + __builtin_abort(); > +#endif > } > -- > 2.43.0 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules 2024-02-11 4:01 ` Andrew Pinski @ 2024-02-11 4:24 ` Nathaniel Shead 0 siblings, 0 replies; 12+ messages in thread From: Nathaniel Shead @ 2024-02-11 4:24 UTC (permalink / raw) To: Andrew Pinski; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell On Sat, Feb 10, 2024 at 08:01:46PM -0800, Andrew Pinski wrote: > On Sat, Feb 10, 2024 at 7:55 PM Nathaniel Shead > <nathanieloshead@gmail.com> wrote: > > > > Bootstrapped and regtested (so far just modules.exp and dg.exp) on > > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? > > > > (Also I noticed I forgot to add the PR to the changelog in my last > > patch, I've fixed that locally.) > > > > -- >8 -- > > > > After fixing PR111710, I noticed that we currently ICE when declaring a > > type that derives from 'decltype([]{})'. As far as I can tell this > > should be legal code, since by [basic.link] p15.2 a lambda defined in a > > class-specifier should not be TU-local. > > > > This patch also adds a bunch of tests for unevaluated lambdas in other > > contexts, which generally seem to work now. > > There are many unevaluated lambdas (non-modules related) bugs report > (all linked to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107430). > Do you know if this fixes any of the non-module related ones too? > > Thanks, > Andrew Pinski > I took a quick look at the issues linked to the above. I tried a few and they don't seem to be fixed with these patches; in general I would only expect changes in modules or potentially with lambdas being used in base class specifiers, neither of which seem particularly relevant to those issues. Nathaniel > > > > One interesting case is 'E::f' in the attached testcase: it appears to > > get a merge kind of 'MK_field', rather than 'MK_keyed' as most other > > lambdas do. I'm not entirely sure if this will cause issues in the > > future, but I haven't been able to construct a testcase that causes > > problems with this, and conversely wrapping the class body in > > 'start_lambda_scope' causes issues with symbol duplication in COMDAT > > groups, so I've left it as-is for now. > > > > gcc/cp/ChangeLog: > > > > * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. > > (maybe_key_decl): Likewise. > > * parser.cc (cp_parser_class_head): Start a lambda scope when > > parsing base classes. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/lambda-7_a.C: > > * g++.dg/modules/lambda-7_b.C: > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > --- > > gcc/cp/module.cc | 8 +++++--- > > gcc/cp/parser.cc | 10 ++++++++-- > > gcc/testsuite/g++.dg/modules/lambda-7_a.C | 19 +++++++++++++++++++ > > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 23 +++++++++++++++++++++++ > > 4 files changed, 55 insertions(+), 5 deletions(-) > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > > index 9742bca922c..cceec79b26b 100644 > > --- a/gcc/cp/module.cc > > +++ b/gcc/cp/module.cc > > @@ -10784,7 +10784,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > > (TREE_TYPE (inner))); > > gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > > || TREE_CODE (scope) == FIELD_DECL > > - || TREE_CODE (scope) == PARM_DECL); > > + || TREE_CODE (scope) == PARM_DECL > > + || TREE_CODE (scope) == TYPE_DECL); > > auto *root = keyed_table->get (scope); > > unsigned ix = root->length (); > > /* If we don't find it, we'll write a really big number > > @@ -18980,10 +18981,11 @@ maybe_key_decl (tree ctx, tree decl) > > return; > > > > /* We only need to deal with lambdas attached to var, field, > > - or parm decls. */ > > + parm, or type decls. */ > > if (TREE_CODE (ctx) != VAR_DECL > > && TREE_CODE (ctx) != FIELD_DECL > > - && TREE_CODE (ctx) != PARM_DECL) > > + && TREE_CODE (ctx) != PARM_DECL > > + && TREE_CODE (ctx) != TYPE_DECL) > > return; > > > > if (!keyed_table) > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > index 09ecfa23b5d..151e724ed66 100644 > > --- a/gcc/cp/parser.cc > > +++ b/gcc/cp/parser.cc > > @@ -27663,10 +27663,16 @@ cp_parser_class_head (cp_parser* parser, > > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > > { > > if (type) > > - pushclass (type); > > + { > > + pushclass (type); > > + start_lambda_scope (TYPE_NAME (type)); > > + } > > bases = cp_parser_base_clause (parser); > > if (type) > > - popclass (); > > + { > > + finish_lambda_scope (); > > + popclass (); > > + } > > } > > else > > bases = NULL_TREE; > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > > index 289285cd926..9a23827a280 100644 > > --- a/gcc/testsuite/g++.dg/modules/lambda-7_a.C > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > > @@ -18,3 +18,22 @@ export struct S { > > export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > > return f(x); > > } > > + > > +// unevaluated lambdas > > +#if __cplusplus >= 202002L > > +export struct E : decltype([](int x) { return x * 6; }) { > > + decltype([](int x) { return x * 7; }) f; > > +}; > > + > > +export template <typename T> > > +struct G : decltype([](int x) { return x * 8; }) { > > + decltype([](int x) { return x * 9; }) h; > > +}; > > + > > +template <> > > +struct G<double> : decltype([](int x) { return x * 10; }) { > > + decltype([](int x) { return x * 11; }) i; > > +}; > > + > > +export decltype([](int x) { return x * 12; }) j; > > +#endif > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > > index a8762399ee1..59a82e05cbf 100644 > > --- a/gcc/testsuite/g++.dg/modules/lambda-7_b.C > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > > @@ -13,4 +13,27 @@ int main() { > > __builtin_abort(); > > if (d(10) != 50) > > __builtin_abort(); > > + > > +#if __cplusplus >= 202002L > > + E e; > > + if (e(10) != 60) > > + __builtin_abort(); > > + if (e.f(10) != 70) > > + __builtin_abort(); > > + > > + G<int> g1; > > + if (g1(10) != 80) > > + __builtin_abort(); > > + if (g1.h(10) != 90) > > + __builtin_abort(); > > + > > + G<double> g2; > > + if (g2(10) != 100) > > + __builtin_abort(); > > + if (g2.i(10) != 110) > > + __builtin_abort(); > > + > > + if (j(10) != 120) > > + __builtin_abort(); > > +#endif > > } > > -- > > 2.43.0 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead 2024-02-11 4:01 ` Andrew Pinski @ 2024-02-12 13:12 ` Marek Polacek 2024-02-14 2:44 ` Jason Merrill 2 siblings, 0 replies; 12+ messages in thread From: Marek Polacek @ 2024-02-12 13:12 UTC (permalink / raw) To: Nathaniel Shead; +Cc: gcc-patches, Jason Merrill, Nathan Sidwell On Sun, Feb 11, 2024 at 02:54:18PM +1100, Nathaniel Shead wrote: > Bootstrapped and regtested (so far just modules.exp and dg.exp) on > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? > > (Also I noticed I forgot to add the PR to the changelog in my last > patch, I've fixed that locally.) > > -- >8 -- > > After fixing PR111710, I noticed that we currently ICE when declaring a > type that derives from 'decltype([]{})'. As far as I can tell this > should be legal code, since by [basic.link] p15.2 a lambda defined in a > class-specifier should not be TU-local. > > This patch also adds a bunch of tests for unevaluated lambdas in other > contexts, which generally seem to work now. > > One interesting case is 'E::f' in the attached testcase: it appears to > get a merge kind of 'MK_field', rather than 'MK_keyed' as most other > lambdas do. I'm not entirely sure if this will cause issues in the > future, but I haven't been able to construct a testcase that causes > problems with this, and conversely wrapping the class body in > 'start_lambda_scope' causes issues with symbol duplication in COMDAT > groups, so I've left it as-is for now. Hi Nathaniel, > gcc/cp/ChangeLog: > > * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. > (maybe_key_decl): Likewise. > * parser.cc (cp_parser_class_head): Start a lambda scope when > parsing base classes. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7_a.C: > * g++.dg/modules/lambda-7_b.C: sorry to be pointing out nits, but this CL entry is missing a description. No need to repost the patch because of that, of couse. Marek ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead 2024-02-11 4:01 ` Andrew Pinski 2024-02-12 13:12 ` Marek Polacek @ 2024-02-14 2:44 ` Jason Merrill 2 siblings, 0 replies; 12+ messages in thread From: Jason Merrill @ 2024-02-14 2:44 UTC (permalink / raw) To: Nathaniel Shead, gcc-patches; +Cc: Nathan Sidwell On 2/10/24 22:54, Nathaniel Shead wrote: > Bootstrapped and regtested (so far just modules.exp and dg.exp) on > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds? > > (Also I noticed I forgot to add the PR to the changelog in my last > patch, I've fixed that locally.) > > -- >8 -- > > After fixing PR111710, I noticed that we currently ICE when declaring a > type that derives from 'decltype([]{})'. As far as I can tell this > should be legal code, since by [basic.link] p15.2 a lambda defined in a > class-specifier should not be TU-local. > > This patch also adds a bunch of tests for unevaluated lambdas in other > contexts, which generally seem to work now. > > One interesting case is 'E::f' in the attached testcase: it appears to > get a merge kind of 'MK_field', rather than 'MK_keyed' as most other > lambdas do. You mean the lambda in the type of E::f? That happens because when we see a class in the body of another class, we treat it as a member even if it's a closure. This ought to work fine. It also works with the suggested direction for https://github.com/itanium-cxx-abi/cxx-abi/issues/165 So, the patch is OK with the missing ChangeLog bits filled in. Do we also need to key lambdas to concepts? template <class T> concept L = requires { []{ T(); }; }; > I'm not entirely sure if this will cause issues in the > future, but I haven't been able to construct a testcase that causes > problems with this, and conversely wrapping the class body in > 'start_lambda_scope' causes issues with symbol duplication in COMDAT > groups, so I've left it as-is for now. > > gcc/cp/ChangeLog: > > * module.cc (trees_out::key_mergeable): Also support TYPE_DECLs. > (maybe_key_decl): Likewise. > * parser.cc (cp_parser_class_head): Start a lambda scope when > parsing base classes. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7_a.C: > * g++.dg/modules/lambda-7_b.C: > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/module.cc | 8 +++++--- > gcc/cp/parser.cc | 10 ++++++++-- > gcc/testsuite/g++.dg/modules/lambda-7_a.C | 19 +++++++++++++++++++ > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 23 +++++++++++++++++++++++ > 4 files changed, 55 insertions(+), 5 deletions(-) > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 9742bca922c..cceec79b26b 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -10784,7 +10784,8 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > (TREE_TYPE (inner))); > gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > || TREE_CODE (scope) == FIELD_DECL > - || TREE_CODE (scope) == PARM_DECL); > + || TREE_CODE (scope) == PARM_DECL > + || TREE_CODE (scope) == TYPE_DECL); > auto *root = keyed_table->get (scope); > unsigned ix = root->length (); > /* If we don't find it, we'll write a really big number > @@ -18980,10 +18981,11 @@ maybe_key_decl (tree ctx, tree decl) > return; > > /* We only need to deal with lambdas attached to var, field, > - or parm decls. */ > + parm, or type decls. */ > if (TREE_CODE (ctx) != VAR_DECL > && TREE_CODE (ctx) != FIELD_DECL > - && TREE_CODE (ctx) != PARM_DECL) > + && TREE_CODE (ctx) != PARM_DECL > + && TREE_CODE (ctx) != TYPE_DECL) > return; > > if (!keyed_table) > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 09ecfa23b5d..151e724ed66 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -27663,10 +27663,16 @@ cp_parser_class_head (cp_parser* parser, > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > { > if (type) > - pushclass (type); > + { > + pushclass (type); > + start_lambda_scope (TYPE_NAME (type)); > + } > bases = cp_parser_base_clause (parser); > if (type) > - popclass (); > + { > + finish_lambda_scope (); > + popclass (); > + } > } > else > bases = NULL_TREE; > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > index 289285cd926..9a23827a280 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-7_a.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > @@ -18,3 +18,22 @@ export struct S { > export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > return f(x); > } > + > +// unevaluated lambdas > +#if __cplusplus >= 202002L > +export struct E : decltype([](int x) { return x * 6; }) { > + decltype([](int x) { return x * 7; }) f; > +}; > + > +export template <typename T> > +struct G : decltype([](int x) { return x * 8; }) { > + decltype([](int x) { return x * 9; }) h; > +}; > + > +template <> > +struct G<double> : decltype([](int x) { return x * 10; }) { > + decltype([](int x) { return x * 11; }) i; > +}; > + > +export decltype([](int x) { return x * 12; }) j; > +#endif > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > index a8762399ee1..59a82e05cbf 100644 > --- a/gcc/testsuite/g++.dg/modules/lambda-7_b.C > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > @@ -13,4 +13,27 @@ int main() { > __builtin_abort(); > if (d(10) != 50) > __builtin_abort(); > + > +#if __cplusplus >= 202002L > + E e; > + if (e(10) != 60) > + __builtin_abort(); > + if (e.f(10) != 70) > + __builtin_abort(); > + > + G<int> g1; > + if (g1(10) != 80) > + __builtin_abort(); > + if (g1.h(10) != 90) > + __builtin_abort(); > + > + G<double> g2; > + if (g2(10) != 100) > + __builtin_abort(); > + if (g2.i(10) != 110) > + __builtin_abort(); > + > + if (j(10) != 120) > + __builtin_abort(); > +#endif > } ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] 2024-02-10 22:57 [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Nathaniel Shead 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead @ 2024-02-14 0:52 ` Jason Merrill 2024-02-16 12:21 ` [PATCH v2] c++/modules: Support lambdas attached to more places " Nathaniel Shead 1 sibling, 1 reply; 12+ messages in thread From: Jason Merrill @ 2024-02-14 0:52 UTC (permalink / raw) To: Nathaniel Shead, gcc-patches; +Cc: Nathan Sidwell On 2/10/24 17:57, Nathaniel Shead wrote: > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > Or should I re-introduce the tree checking and just add checks for the > new kinds of declarations that could be have keyed decls? This way is fine. > -- >8 -- > > The fix for PR107398 weakened the restrictions that lambdas must belong > to namespace scope. However this was not sufficient: we also need to > allow lambdas keyed to FIELD_DECLs or PARM_DECLs. I wonder about keying such lambdas to the class and function, respectively, rather than specifically to the field or parameter, but I suppose it doesn't matter. OK with the comment adjustment below. > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > fairly large number of different kinds of DECLs, and that in general > it's safe to just get 'false' as a result of a check on an unexpected > DECL type, this patch also removes the tree checking from the accessor. > > gcc/cp/ChangeLog: > > * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. > (struct lang_decl_base): Update comments and fix whitespace. > * module.cc (trees_out::lang_decl_bools): Always write > module_keyed_decls_p flag... > (trees_in::lang_decl_bools): ...and always read it. > (trees_out::decl_value): Also handle keyed decls on decls other > than VAR_DECL or FUNCTION_DECL. > (trees_in::decl_value): Likewise. > (trees_out::get_merge_kind): Likewise. > (maybe_key_decl): Also handle lambdas attached to FIELD_DECLs > and PARM_DECLS. > (trees_out::key_mergeable): Likewise. > (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL > container. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7_a.C: New test. > * g++.dg/modules/lambda-7_b.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/cp-tree.h | 23 ++++---- > gcc/cp/module.cc | 70 +++++++++++------------ > gcc/testsuite/g++.dg/modules/lambda-7_a.C | 20 +++++++ > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 16 ++++++ > 4 files changed, 81 insertions(+), 48 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 334c11396c2..6ab82dc2d0f 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) > (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) > > /* DECL that has attached decls for ODR-relatedness. */ > -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ > - ->u.base.module_keyed_decls_p) > +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) > > /* Whether this is an exported DECL. Held on any decl that can appear > at namespace scope (function, var, type, template, const or > @@ -2887,21 +2886,19 @@ struct GTY(()) lang_decl_base { > unsigned friend_or_tls : 1; /* var, fn, type or template */ > unsigned unknown_bound_p : 1; /* var */ > unsigned odr_used : 1; /* var or fn */ > - unsigned concept_p : 1; /* applies to vars and functions */ > + unsigned concept_p : 1; /* applies to vars and functions */ > unsigned var_declared_inline_p : 1; /* var */ > unsigned dependent_init_p : 1; /* var */ > > /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > decls. */ With your reformatting this comment now seems to apply to module_keyed_decls_p, which I don't think you mean. Maybe just adjust this comment to say "the following 4"? > - unsigned module_purview_p : 1; // in named-module purview > - unsigned module_attach_p : 1; // attached to named module > - unsigned module_import_p : 1; /* from an import */ > - unsigned module_entity_p : 1; /* is in the entitity ary & > - hash. */ > - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ > - unsigned module_keyed_decls_p : 1; > - > - /* 12 spare bits. */ > + unsigned module_purview_p : 1; /* in named-module purview */ > + unsigned module_attach_p : 1; /* attached to named module */ > + unsigned module_import_p : 1; /* from an import */ > + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ > + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ > + > + /* 11 spare bits. */ > }; > > /* True for DECL codes which have template info and access. */ > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 560d8f3b614..9742bca922c 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t) > want to mark them as in module purview. */ > WB (lang->u.base.module_purview_p && !header_module_p ()); > WB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - WB (lang->u.base.module_keyed_decls_p); > + WB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -5738,8 +5737,7 @@ trees_in::lang_decl_bools (tree t) > RB (lang->u.base.dependent_init_p); > RB (lang->u.base.module_purview_p); > RB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - RB (lang->u.base.module_keyed_decls_p); > + RB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -7869,8 +7867,7 @@ trees_out::decl_value (tree decl, depset *dep) > install_entity (decl, dep); > } > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner) > && !is_key_order ()) > { > @@ -8170,8 +8167,7 @@ trees_in::decl_value () > bool installed = install_entity (existing); > bool is_new = existing == decl; > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner)) > { > /* Read and maybe install the attached entities. */ > @@ -10482,8 +10478,7 @@ trees_out::get_merge_kind (tree decl, depset *dep) > if (tree scope > = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (decl)))) > - if (TREE_CODE (scope) == VAR_DECL > - && DECL_MODULE_KEYED_DECLS_P (scope)) > + if (DECL_MODULE_KEYED_DECLS_P (scope)) > { > mk = MK_keyed; > break; > @@ -10787,7 +10782,9 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); > tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (inner))); > - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); > + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > + || TREE_CODE (scope) == FIELD_DECL > + || TREE_CODE (scope) == PARM_DECL); > auto *root = keyed_table->get (scope); > unsigned ix = root->length (); > /* If we don't find it, we'll write a really big number > @@ -11065,6 +11062,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > } > } > } > + else if (mk == MK_keyed > + && DECL_LANG_SPECIFIC (name) > + && DECL_MODULE_KEYED_DECLS_P (name)) > + { > + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL > + || TREE_CODE (container) == TYPE_DECL); > + if (auto *set = keyed_table->get (name)) > + if (key.index < set->length ()) > + { > + existing = (*set)[key.index]; > + if (existing) > + { > + gcc_checking_assert > + (DECL_IMPLICIT_TYPEDEF_P (existing)); > + if (inner != decl) > + existing > + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > + } > + } > + } > else > switch (TREE_CODE (container)) > { > @@ -11072,27 +11089,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_unreachable (); > > case NAMESPACE_DECL: > - if (mk == MK_keyed) > - { > - if (DECL_LANG_SPECIFIC (name) > - && VAR_OR_FUNCTION_DECL_P (name) > - && DECL_MODULE_KEYED_DECLS_P (name)) > - if (auto *set = keyed_table->get (name)) > - if (key.index < set->length ()) > - { > - existing = (*set)[key.index]; > - if (existing) > - { > - gcc_checking_assert > - (DECL_IMPLICIT_TYPEDEF_P (existing)); > - if (inner != decl) > - existing > - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > - } > - } > - } > - else if (is_attached > - && !(state->is_module () || state->is_partition ())) > + if (is_attached > + && !(state->is_module () || state->is_partition ())) > kind = "unique"; > else > { > @@ -18981,9 +18979,11 @@ maybe_key_decl (tree ctx, tree decl) > if (!modules_p ()) > return; > > - // FIXME: For now just deal with lambdas attached to var decls. > - // This might be sufficient? > - if (TREE_CODE (ctx) != VAR_DECL) > + /* We only need to deal with lambdas attached to var, field, > + or parm decls. */ > + if (TREE_CODE (ctx) != VAR_DECL > + && TREE_CODE (ctx) != FIELD_DECL > + && TREE_CODE (ctx) != PARM_DECL) > return; > > if (!keyed_table) > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.C b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > new file mode 100644 > index 00000000000..289285cd926 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.C > @@ -0,0 +1,20 @@ > +// { dg-additional-options "-fmodules-ts" } > +// { dg-module-cmi foo } > + > +export module foo; > + > +export struct S { > + int (*a)(int) = [](int x) { return x * 2; }; > + > + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { > + return f(x); > + } > + > + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { > + return f(x); > + } > +}; > + > +export inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > + return f(x); > +} > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > new file mode 100644 > index 00000000000..a8762399ee1 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > @@ -0,0 +1,16 @@ > +// { dg-module-do run } > +// { dg-additional-options "-fmodules-ts" } > + > +import foo; > + > +int main() { > + S s; > + if (s.a(10) != 20) > + __builtin_abort(); > + if (s.b(10) != 30) > + __builtin_abort(); > + if (s.c(10) != 40) > + __builtin_abort(); > + if (d(10) != 50) > + __builtin_abort(); > +} ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v2] c++/modules: Support lambdas attached to more places in modules [PR111710] 2024-02-14 0:52 ` [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Jason Merrill @ 2024-02-16 12:21 ` Nathaniel Shead 2024-02-27 16:59 ` Patrick Palka 0 siblings, 1 reply; 12+ messages in thread From: Nathaniel Shead @ 2024-02-16 12:21 UTC (permalink / raw) To: Jason Merrill; +Cc: gcc-patches, Nathan Sidwell On Tue, Feb 13, 2024 at 07:52:01PM -0500, Jason Merrill wrote: > On 2/10/24 17:57, Nathaniel Shead wrote: > > The fix for PR107398 weakened the restrictions that lambdas must belong > > to namespace scope. However this was not sufficient: we also need to > > allow lambdas keyed to FIELD_DECLs or PARM_DECLs. > > I wonder about keying such lambdas to the class and function, respectively, > rather than specifically to the field or parameter, but I suppose it doesn't > matter. I did some more testing and realised my testcase didn't properly exercise whether I'd properly deduplicated or not, and an improved testcase proved that actually keying to the field rather than the class did cause issues. (Parameter vs. function doesn't seem to have mattered however.) Here's an updated patch that fixes this, and includes the changes for lambdas in base classes that I'd had as a separate patch earlier. I've also added some concepts testcases just in case. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- The fix for PR107398 weakened the restrictions that lambdas must belong to namespace scope. However this was not sufficient: we also need to allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. For field decls we key the lambda to its class rather than the field itself. This avoids some errors with deduplicating fields. Additionally, by [basic.link] p15.2 a lambda defined anywhere in a class-specifier should not be TU-local, which includes base-class declarations, so ensure that lambdas declared there are keyed appropriately as well. Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a fairly large number of different kinds of DECLs, and that in general it's safe to just get 'false' as a result of a check on an unexpected DECL type, this patch also removes the tree checking from the accessor. Finally, to handle deduplicating templated lambda fields, we need to ensure that we can determine that two lambdas from different field decls match. The modules code does not attempt to deduplicate expression nodes, which causes issues as the LAMBDA_EXPRs are then considered to be different. However, rather than checking the LAMBDA_EXPR directly we can instead check its type: the generated RECORD_TYPE for a LAMBDA_EXPR must also be unique, and /is/ deduplicated on import, so we can just check for that instead. PR c++/111710 gcc/cp/ChangeLog: * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. (struct lang_decl_base): Update comments and fix whitespace. * module.cc (trees_out::lang_decl_bools): Always write module_keyed_decls_p flag... (trees_in::lang_decl_bools): ...and always read it. (trees_out::decl_value): Handle all kinds of keyed decls. (trees_in::decl_value): Likewise. (maybe_key_decl): Also support lambdas attached to fields, parameters, and types. Key lambdas attached to fields to their class. (trees_out::get_merge_kind): Likewise. (trees_out::key_mergeable): Likewise. (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL container. * parser.cc (cp_parser_class_head): Start a lambda scope when parsing base classes. * tree.cc (cp_tree_equal): Check equality of the types of LAMBDA_EXPRs instead of the exprs themselves. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7.h: New test. * g++.dg/modules/lambda-7_a.H: New test. * g++.dg/modules/lambda-7_b.C: New test. * g++.dg/modules/lambda-7_c.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> --- gcc/cp/cp-tree.h | 26 +++---- gcc/cp/module.cc | 94 +++++++++++++---------- gcc/cp/parser.cc | 10 ++- gcc/cp/tree.cc | 4 +- gcc/testsuite/g++.dg/modules/lambda-7.h | 42 ++++++++++ gcc/testsuite/g++.dg/modules/lambda-7_a.H | 4 + gcc/testsuite/g++.dg/modules/lambda-7_b.C | 5 ++ gcc/testsuite/g++.dg/modules/lambda-7_c.C | 41 ++++++++++ 8 files changed, 169 insertions(+), 57 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7.h create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.H create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_c.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 334c11396c2..04c3aa6cd91 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) /* DECL that has attached decls for ODR-relatedness. */ -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ - ->u.base.module_keyed_decls_p) +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) /* Whether this is an exported DECL. Held on any decl that can appear at namespace scope (function, var, type, template, const or @@ -2887,21 +2886,20 @@ struct GTY(()) lang_decl_base { unsigned friend_or_tls : 1; /* var, fn, type or template */ unsigned unknown_bound_p : 1; /* var */ unsigned odr_used : 1; /* var or fn */ - unsigned concept_p : 1; /* applies to vars and functions */ + unsigned concept_p : 1; /* applies to vars and functions */ unsigned var_declared_inline_p : 1; /* var */ unsigned dependent_init_p : 1; /* var */ - /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE + /* The following four apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE decls. */ - unsigned module_purview_p : 1; // in named-module purview - unsigned module_attach_p : 1; // attached to named module - unsigned module_import_p : 1; /* from an import */ - unsigned module_entity_p : 1; /* is in the entitity ary & - hash. */ - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ - unsigned module_keyed_decls_p : 1; - - /* 12 spare bits. */ + unsigned module_purview_p : 1; /* in named-module purview */ + unsigned module_attach_p : 1; /* attached to named module */ + unsigned module_import_p : 1; /* from an import */ + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ + + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ + + /* 11 spare bits. */ }; /* True for DECL codes which have template info and access. */ diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 0291d456ff5..af99df2b79c 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5662,8 +5662,7 @@ trees_out::lang_decl_bools (tree t) want to mark them as in module purview. */ WB (lang->u.base.module_purview_p && !header_module_p ()); WB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - WB (lang->u.base.module_keyed_decls_p); + WB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -5736,8 +5735,7 @@ trees_in::lang_decl_bools (tree t) RB (lang->u.base.dependent_init_p); RB (lang->u.base.module_purview_p); RB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - RB (lang->u.base.module_keyed_decls_p); + RB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -7867,8 +7865,7 @@ trees_out::decl_value (tree decl, depset *dep) install_entity (decl, dep); } - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner) && !is_key_order ()) { @@ -8168,8 +8165,7 @@ trees_in::decl_value () bool installed = install_entity (existing); bool is_new = existing == decl; - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner)) { /* Read and maybe install the attached entities. */ @@ -10480,12 +10476,17 @@ trees_out::get_merge_kind (tree decl, depset *dep) if (tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (decl)))) - if (TREE_CODE (scope) == VAR_DECL - && DECL_MODULE_KEYED_DECLS_P (scope)) - { - mk = MK_keyed; - break; - } + { + /* Lambdas attached to fields are keyed to its class. */ + if (TREE_CODE (scope) == FIELD_DECL) + scope = TYPE_NAME (DECL_CONTEXT (scope)); + if (DECL_LANG_SPECIFIC (scope) + && DECL_MODULE_KEYED_DECLS_P (scope)) + { + mk = MK_keyed; + break; + } + } if (RECORD_OR_UNION_TYPE_P (ctx)) { @@ -10785,7 +10786,13 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (inner))); - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL + || TREE_CODE (scope) == FIELD_DECL + || TREE_CODE (scope) == PARM_DECL + || TREE_CODE (scope) == TYPE_DECL); + /* Lambdas attached to fields are keyed to the class. */ + if (TREE_CODE (scope) == FIELD_DECL) + scope = TYPE_NAME (DECL_CONTEXT (scope)); auto *root = keyed_table->get (scope); unsigned ix = root->length (); /* If we don't find it, we'll write a really big number @@ -11063,6 +11070,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, } } } + else if (mk == MK_keyed + && DECL_LANG_SPECIFIC (name) + && DECL_MODULE_KEYED_DECLS_P (name)) + { + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL + || TREE_CODE (container) == TYPE_DECL); + if (auto *set = keyed_table->get (name)) + if (key.index < set->length ()) + { + existing = (*set)[key.index]; + if (existing) + { + gcc_checking_assert + (DECL_IMPLICIT_TYPEDEF_P (existing)); + if (inner != decl) + existing + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); + } + } + } else switch (TREE_CODE (container)) { @@ -11070,27 +11097,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_unreachable (); case NAMESPACE_DECL: - if (mk == MK_keyed) - { - if (DECL_LANG_SPECIFIC (name) - && VAR_OR_FUNCTION_DECL_P (name) - && DECL_MODULE_KEYED_DECLS_P (name)) - if (auto *set = keyed_table->get (name)) - if (key.index < set->length ()) - { - existing = (*set)[key.index]; - if (existing) - { - gcc_checking_assert - (DECL_IMPLICIT_TYPEDEF_P (existing)); - if (inner != decl) - existing - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); - } - } - } - else if (is_attached - && !(state->is_module () || state->is_partition ())) + if (is_attached + && !(state->is_module () || state->is_partition ())) kind = "unique"; else { @@ -18980,11 +18988,19 @@ maybe_key_decl (tree ctx, tree decl) if (!modules_p ()) return; - // FIXME: For now just deal with lambdas attached to var decls. - // This might be sufficient? - if (TREE_CODE (ctx) != VAR_DECL) + /* We only need to deal with lambdas attached to var, field, + parm, or type decls. */ + if (TREE_CODE (ctx) != VAR_DECL + && TREE_CODE (ctx) != FIELD_DECL + && TREE_CODE (ctx) != PARM_DECL + && TREE_CODE (ctx) != TYPE_DECL) return; + /* For fields, key it to the containing type to handle deduplication + correctly. */ + if (TREE_CODE (ctx) == FIELD_DECL) + ctx = TYPE_NAME (DECL_CONTEXT (ctx)); + if (!keyed_table) keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 9d0914435fb..95d59973b6d 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27671,10 +27671,16 @@ cp_parser_class_head (cp_parser* parser, if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) { if (type) - pushclass (type); + { + pushclass (type); + start_lambda_scope (TYPE_NAME (type)); + } bases = cp_parser_base_clause (parser); if (type) - popclass (); + { + finish_lambda_scope (); + popclass (); + } } else bases = NULL_TREE; diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc index ad312710f68..ca598859b97 100644 --- a/gcc/cp/tree.cc +++ b/gcc/cp/tree.cc @@ -4239,8 +4239,8 @@ cp_tree_equal (tree t1, tree t2) DEFERRED_NOEXCEPT_ARGS (t2))); case LAMBDA_EXPR: - /* Two lambda-expressions are never considered equivalent. */ - return false; + /* Two lambda-expressions are only equivalent if their type is. */ + return TREE_TYPE (t1) == TREE_TYPE (t2); case TYPE_ARGUMENT_PACK: case NONTYPE_ARGUMENT_PACK: diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h new file mode 100644 index 00000000000..6f6080c1324 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h @@ -0,0 +1,42 @@ +struct S { + int (*a)(int) = [](int x) { return x * 2; }; + + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { + return f(x); + } + + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { + return f(x); + } +}; + +inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { + return f(x); +} + +// unevaluated lambdas +#if __cplusplus >= 202002L +struct E : decltype([](int x) { return x * 6; }) { + decltype([](int x) { return x * 7; }) f; +}; + +template <typename T> +struct G : decltype([](int x) { return x * 8; }) { + decltype([](int x) { return x * 9; }) h; +}; + +template <> +struct G<double> : decltype([](int x) { return x * 10; }) { + decltype([](int x) { return x * 11; }) i; +}; +#endif + +// concepts +#if __cpp_concepts >= 201907L +template <typename T> +concept J = requires { []{ T(); }; }; + +template <typename T> +concept K = []{ return sizeof(T) == 1; }(); +#endif + diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.H b/gcc/testsuite/g++.dg/modules/lambda-7_a.H new file mode 100644 index 00000000000..5197114f76c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.H @@ -0,0 +1,4 @@ +// { dg-additional-options "-fmodule-header -Wno-subobject-linkage" } +// { dg-module-cmi {} } + +#include "lambda-7.h" diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C new file mode 100644 index 00000000000..2d781e93067 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C @@ -0,0 +1,5 @@ +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } +// Test for ODR deduplication + +#include "lambda-7.h" +import "lambda-7_a.H"; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_c.C b/gcc/testsuite/g++.dg/modules/lambda-7_c.C new file mode 100644 index 00000000000..f283681fa96 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_c.C @@ -0,0 +1,41 @@ +// { dg-module-do run } +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } + +import "lambda-7_a.H"; + +int main() { + S s; + if (s.a(10) != 20) + __builtin_abort(); + if (s.b(10) != 30) + __builtin_abort(); + if (s.c(10) != 40) + __builtin_abort(); + if (d(10) != 50) + __builtin_abort(); + +#if __cplusplus >= 202002L + E e; + if (e(10) != 60) + __builtin_abort(); + if (e.f(10) != 70) + __builtin_abort(); + + G<int> g1; + if (g1(10) != 80) + __builtin_abort(); + if (g1.h(10) != 90) + __builtin_abort(); + + G<double> g2; + if (g2(10) != 100) + __builtin_abort(); + if (g2.i(10) != 110) + __builtin_abort(); +#endif + +#if __cpp_concepts >= 201907L + static_assert(J<char>); + static_assert(K<char>); +#endif +} -- 2.43.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v2] c++/modules: Support lambdas attached to more places in modules [PR111710] 2024-02-16 12:21 ` [PATCH v2] c++/modules: Support lambdas attached to more places " Nathaniel Shead @ 2024-02-27 16:59 ` Patrick Palka 2024-02-28 4:12 ` [PATCH v3] " Nathaniel Shead 0 siblings, 1 reply; 12+ messages in thread From: Patrick Palka @ 2024-02-27 16:59 UTC (permalink / raw) To: Nathaniel Shead; +Cc: Jason Merrill, gcc-patches, Nathan Sidwell On Fri, 16 Feb 2024, Nathaniel Shead wrote: > On Tue, Feb 13, 2024 at 07:52:01PM -0500, Jason Merrill wrote: > > On 2/10/24 17:57, Nathaniel Shead wrote: > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > > to namespace scope. However this was not sufficient: we also need to > > > allow lambdas keyed to FIELD_DECLs or PARM_DECLs. > > > > I wonder about keying such lambdas to the class and function, respectively, > > rather than specifically to the field or parameter, but I suppose it doesn't > > matter. > > I did some more testing and realised my testcase didn't properly > exercise whether I'd properly deduplicated or not, and an improved > testcase proved that actually keying to the field rather than the class > did cause issues. (Parameter vs. function doesn't seem to have mattered > however.) > > Here's an updated patch that fixes this, and includes the changes for > lambdas in base classes that I'd had as a separate patch earlier. I've > also added some concepts testcases just in case. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > -- >8 -- > > The fix for PR107398 weakened the restrictions that lambdas must belong > to namespace scope. However this was not sufficient: we also need to > allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. > > For field decls we key the lambda to its class rather than the field > itself. This avoids some errors with deduplicating fields. > > Additionally, by [basic.link] p15.2 a lambda defined anywhere in a > class-specifier should not be TU-local, which includes base-class > declarations, so ensure that lambdas declared there are keyed > appropriately as well. > > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > fairly large number of different kinds of DECLs, and that in general > it's safe to just get 'false' as a result of a check on an unexpected > DECL type, this patch also removes the tree checking from the accessor. > > Finally, to handle deduplicating templated lambda fields, we need to > ensure that we can determine that two lambdas from different field decls > match. The modules code does not attempt to deduplicate expression > nodes, which causes issues as the LAMBDA_EXPRs are then considered to be > different. However, rather than checking the LAMBDA_EXPR directly we can > instead check its type: the generated RECORD_TYPE for a LAMBDA_EXPR must > also be unique, and /is/ deduplicated on import, so we can just check > for that instead. We probably should be deduplicating LAMBDA_EXPR on stream-in, perhaps something like diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index e8eabb1f6f9..1b2ba2e0fa8 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -9183,6 +9183,13 @@ trees_in::tree_value () return NULL_TREE; } + if (TREE_CODE (t) == LAMBDA_EXPR + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) + { + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); + back_refs[~tag] = existing; + } + dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) would suffice? If not we probably need to take inspiration from the TREE_BINFO streaming, and handle LAMBDA_EXPR similarly.. > > PR c++/111710 > > gcc/cp/ChangeLog: > > * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. > (struct lang_decl_base): Update comments and fix whitespace. > * module.cc (trees_out::lang_decl_bools): Always write > module_keyed_decls_p flag... > (trees_in::lang_decl_bools): ...and always read it. > (trees_out::decl_value): Handle all kinds of keyed decls. > (trees_in::decl_value): Likewise. > (maybe_key_decl): Also support lambdas attached to fields, > parameters, and types. Key lambdas attached to fields to their > class. > (trees_out::get_merge_kind): Likewise. > (trees_out::key_mergeable): Likewise. > (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL > container. > * parser.cc (cp_parser_class_head): Start a lambda scope when > parsing base classes. > * tree.cc (cp_tree_equal): Check equality of the types of > LAMBDA_EXPRs instead of the exprs themselves. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7.h: New test. > * g++.dg/modules/lambda-7_a.H: New test. > * g++.dg/modules/lambda-7_b.C: New test. > * g++.dg/modules/lambda-7_c.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > --- > gcc/cp/cp-tree.h | 26 +++---- > gcc/cp/module.cc | 94 +++++++++++++---------- > gcc/cp/parser.cc | 10 ++- > gcc/cp/tree.cc | 4 +- > gcc/testsuite/g++.dg/modules/lambda-7.h | 42 ++++++++++ > gcc/testsuite/g++.dg/modules/lambda-7_a.H | 4 + > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 5 ++ > gcc/testsuite/g++.dg/modules/lambda-7_c.C | 41 ++++++++++ > 8 files changed, 169 insertions(+), 57 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7.h > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_c.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 334c11396c2..04c3aa6cd91 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) > (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) > > /* DECL that has attached decls for ODR-relatedness. */ > -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ > - ->u.base.module_keyed_decls_p) > +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) > > /* Whether this is an exported DECL. Held on any decl that can appear > at namespace scope (function, var, type, template, const or > @@ -2887,21 +2886,20 @@ struct GTY(()) lang_decl_base { > unsigned friend_or_tls : 1; /* var, fn, type or template */ > unsigned unknown_bound_p : 1; /* var */ > unsigned odr_used : 1; /* var or fn */ > - unsigned concept_p : 1; /* applies to vars and functions */ > + unsigned concept_p : 1; /* applies to vars and functions */ > unsigned var_declared_inline_p : 1; /* var */ > unsigned dependent_init_p : 1; /* var */ > > - /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > + /* The following four apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > decls. */ > - unsigned module_purview_p : 1; // in named-module purview > - unsigned module_attach_p : 1; // attached to named module > - unsigned module_import_p : 1; /* from an import */ > - unsigned module_entity_p : 1; /* is in the entitity ary & > - hash. */ > - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ > - unsigned module_keyed_decls_p : 1; > - > - /* 12 spare bits. */ > + unsigned module_purview_p : 1; /* in named-module purview */ > + unsigned module_attach_p : 1; /* attached to named module */ > + unsigned module_import_p : 1; /* from an import */ > + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ > + > + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ > + > + /* 11 spare bits. */ > }; > > /* True for DECL codes which have template info and access. */ > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 0291d456ff5..af99df2b79c 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5662,8 +5662,7 @@ trees_out::lang_decl_bools (tree t) > want to mark them as in module purview. */ > WB (lang->u.base.module_purview_p && !header_module_p ()); > WB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - WB (lang->u.base.module_keyed_decls_p); > + WB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -5736,8 +5735,7 @@ trees_in::lang_decl_bools (tree t) > RB (lang->u.base.dependent_init_p); > RB (lang->u.base.module_purview_p); > RB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - RB (lang->u.base.module_keyed_decls_p); > + RB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -7867,8 +7865,7 @@ trees_out::decl_value (tree decl, depset *dep) > install_entity (decl, dep); > } > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner) > && !is_key_order ()) > { > @@ -8168,8 +8165,7 @@ trees_in::decl_value () > bool installed = install_entity (existing); > bool is_new = existing == decl; > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner)) > { > /* Read and maybe install the attached entities. */ > @@ -10480,12 +10476,17 @@ trees_out::get_merge_kind (tree decl, depset *dep) > if (tree scope > = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (decl)))) > - if (TREE_CODE (scope) == VAR_DECL > - && DECL_MODULE_KEYED_DECLS_P (scope)) > - { > - mk = MK_keyed; > - break; > - } > + { > + /* Lambdas attached to fields are keyed to its class. */ > + if (TREE_CODE (scope) == FIELD_DECL) > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > + if (DECL_LANG_SPECIFIC (scope) > + && DECL_MODULE_KEYED_DECLS_P (scope)) > + { > + mk = MK_keyed; > + break; > + } > + } > > if (RECORD_OR_UNION_TYPE_P (ctx)) > { > @@ -10785,7 +10786,13 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); > tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (inner))); > - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); > + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > + || TREE_CODE (scope) == FIELD_DECL > + || TREE_CODE (scope) == PARM_DECL > + || TREE_CODE (scope) == TYPE_DECL); > + /* Lambdas attached to fields are keyed to the class. */ > + if (TREE_CODE (scope) == FIELD_DECL) > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > auto *root = keyed_table->get (scope); > unsigned ix = root->length (); > /* If we don't find it, we'll write a really big number > @@ -11063,6 +11070,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > } > } > } > + else if (mk == MK_keyed > + && DECL_LANG_SPECIFIC (name) > + && DECL_MODULE_KEYED_DECLS_P (name)) > + { > + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL > + || TREE_CODE (container) == TYPE_DECL); > + if (auto *set = keyed_table->get (name)) > + if (key.index < set->length ()) > + { > + existing = (*set)[key.index]; > + if (existing) > + { > + gcc_checking_assert > + (DECL_IMPLICIT_TYPEDEF_P (existing)); > + if (inner != decl) > + existing > + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > + } > + } > + } > else > switch (TREE_CODE (container)) > { > @@ -11070,27 +11097,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_unreachable (); > > case NAMESPACE_DECL: > - if (mk == MK_keyed) > - { > - if (DECL_LANG_SPECIFIC (name) > - && VAR_OR_FUNCTION_DECL_P (name) > - && DECL_MODULE_KEYED_DECLS_P (name)) > - if (auto *set = keyed_table->get (name)) > - if (key.index < set->length ()) > - { > - existing = (*set)[key.index]; > - if (existing) > - { > - gcc_checking_assert > - (DECL_IMPLICIT_TYPEDEF_P (existing)); > - if (inner != decl) > - existing > - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > - } > - } > - } > - else if (is_attached > - && !(state->is_module () || state->is_partition ())) > + if (is_attached > + && !(state->is_module () || state->is_partition ())) > kind = "unique"; > else > { > @@ -18980,11 +18988,19 @@ maybe_key_decl (tree ctx, tree decl) > if (!modules_p ()) > return; > > - // FIXME: For now just deal with lambdas attached to var decls. > - // This might be sufficient? > - if (TREE_CODE (ctx) != VAR_DECL) > + /* We only need to deal with lambdas attached to var, field, > + parm, or type decls. */ > + if (TREE_CODE (ctx) != VAR_DECL > + && TREE_CODE (ctx) != FIELD_DECL > + && TREE_CODE (ctx) != PARM_DECL > + && TREE_CODE (ctx) != TYPE_DECL) > return; > > + /* For fields, key it to the containing type to handle deduplication > + correctly. */ > + if (TREE_CODE (ctx) == FIELD_DECL) > + ctx = TYPE_NAME (DECL_CONTEXT (ctx)); > + > if (!keyed_table) > keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 9d0914435fb..95d59973b6d 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -27671,10 +27671,16 @@ cp_parser_class_head (cp_parser* parser, > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > { > if (type) > - pushclass (type); > + { > + pushclass (type); > + start_lambda_scope (TYPE_NAME (type)); > + } > bases = cp_parser_base_clause (parser); > if (type) > - popclass (); > + { > + finish_lambda_scope (); > + popclass (); > + } > } > else > bases = NULL_TREE; > diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc > index ad312710f68..ca598859b97 100644 > --- a/gcc/cp/tree.cc > +++ b/gcc/cp/tree.cc > @@ -4239,8 +4239,8 @@ cp_tree_equal (tree t1, tree t2) > DEFERRED_NOEXCEPT_ARGS (t2))); > > case LAMBDA_EXPR: > - /* Two lambda-expressions are never considered equivalent. */ > - return false; > + /* Two lambda-expressions are only equivalent if their type is. */ > + return TREE_TYPE (t1) == TREE_TYPE (t2); > > case TYPE_ARGUMENT_PACK: > case NONTYPE_ARGUMENT_PACK: > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h > new file mode 100644 > index 00000000000..6f6080c1324 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h > @@ -0,0 +1,42 @@ > +struct S { > + int (*a)(int) = [](int x) { return x * 2; }; > + > + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { > + return f(x); > + } > + > + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { > + return f(x); > + } > +}; > + > +inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > + return f(x); > +} > + > +// unevaluated lambdas > +#if __cplusplus >= 202002L > +struct E : decltype([](int x) { return x * 6; }) { > + decltype([](int x) { return x * 7; }) f; > +}; > + > +template <typename T> > +struct G : decltype([](int x) { return x * 8; }) { > + decltype([](int x) { return x * 9; }) h; > +}; > + > +template <> > +struct G<double> : decltype([](int x) { return x * 10; }) { > + decltype([](int x) { return x * 11; }) i; > +}; > +#endif > + > +// concepts > +#if __cpp_concepts >= 201907L > +template <typename T> > +concept J = requires { []{ T(); }; }; > + > +template <typename T> > +concept K = []{ return sizeof(T) == 1; }(); > +#endif > + > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.H b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > new file mode 100644 > index 00000000000..5197114f76c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > @@ -0,0 +1,4 @@ > +// { dg-additional-options "-fmodule-header -Wno-subobject-linkage" } > +// { dg-module-cmi {} } > + > +#include "lambda-7.h" > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > new file mode 100644 > index 00000000000..2d781e93067 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > @@ -0,0 +1,5 @@ > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > +// Test for ODR deduplication > + > +#include "lambda-7.h" > +import "lambda-7_a.H"; > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_c.C b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > new file mode 100644 > index 00000000000..f283681fa96 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > @@ -0,0 +1,41 @@ > +// { dg-module-do run } > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > + > +import "lambda-7_a.H"; > + > +int main() { > + S s; > + if (s.a(10) != 20) > + __builtin_abort(); > + if (s.b(10) != 30) > + __builtin_abort(); > + if (s.c(10) != 40) > + __builtin_abort(); > + if (d(10) != 50) > + __builtin_abort(); > + > +#if __cplusplus >= 202002L > + E e; > + if (e(10) != 60) > + __builtin_abort(); > + if (e.f(10) != 70) > + __builtin_abort(); > + > + G<int> g1; > + if (g1(10) != 80) > + __builtin_abort(); > + if (g1.h(10) != 90) > + __builtin_abort(); > + > + G<double> g2; > + if (g2(10) != 100) > + __builtin_abort(); > + if (g2.i(10) != 110) > + __builtin_abort(); > +#endif > + > +#if __cpp_concepts >= 201907L > + static_assert(J<char>); > + static_assert(K<char>); > +#endif > +} > -- > 2.43.0 > > ^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH v3] c++/modules: Support lambdas attached to more places in modules [PR111710] 2024-02-27 16:59 ` Patrick Palka @ 2024-02-28 4:12 ` Nathaniel Shead 2024-02-28 17:34 ` Jason Merrill 0 siblings, 1 reply; 12+ messages in thread From: Nathaniel Shead @ 2024-02-28 4:12 UTC (permalink / raw) To: Patrick Palka; +Cc: Jason Merrill, gcc-patches, Nathan Sidwell On Tue, Feb 27, 2024 at 11:59:46AM -0500, Patrick Palka wrote: > On Fri, 16 Feb 2024, Nathaniel Shead wrote: > > > On Tue, Feb 13, 2024 at 07:52:01PM -0500, Jason Merrill wrote: > > > On 2/10/24 17:57, Nathaniel Shead wrote: > > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > > > to namespace scope. However this was not sufficient: we also need to > > > > allow lambdas keyed to FIELD_DECLs or PARM_DECLs. > > > > > > I wonder about keying such lambdas to the class and function, respectively, > > > rather than specifically to the field or parameter, but I suppose it doesn't > > > matter. > > > > I did some more testing and realised my testcase didn't properly > > exercise whether I'd properly deduplicated or not, and an improved > > testcase proved that actually keying to the field rather than the class > > did cause issues. (Parameter vs. function doesn't seem to have mattered > > however.) > > > > Here's an updated patch that fixes this, and includes the changes for > > lambdas in base classes that I'd had as a separate patch earlier. I've > > also added some concepts testcases just in case. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > -- >8 -- > > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > to namespace scope. However this was not sufficient: we also need to > > allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. > > > > For field decls we key the lambda to its class rather than the field > > itself. This avoids some errors with deduplicating fields. > > > > Additionally, by [basic.link] p15.2 a lambda defined anywhere in a > > class-specifier should not be TU-local, which includes base-class > > declarations, so ensure that lambdas declared there are keyed > > appropriately as well. > > > > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > > fairly large number of different kinds of DECLs, and that in general > > it's safe to just get 'false' as a result of a check on an unexpected > > DECL type, this patch also removes the tree checking from the accessor. > > > > Finally, to handle deduplicating templated lambda fields, we need to > > ensure that we can determine that two lambdas from different field decls > > match. The modules code does not attempt to deduplicate expression > > nodes, which causes issues as the LAMBDA_EXPRs are then considered to be > > different. However, rather than checking the LAMBDA_EXPR directly we can > > instead check its type: the generated RECORD_TYPE for a LAMBDA_EXPR must > > also be unique, and /is/ deduplicated on import, so we can just check > > for that instead. > > We probably should be deduplicating LAMBDA_EXPR on stream-in, perhaps > something like > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index e8eabb1f6f9..1b2ba2e0fa8 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -9183,6 +9183,13 @@ trees_in::tree_value () > return NULL_TREE; > } > > + if (TREE_CODE (t) == LAMBDA_EXPR > + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) > + { > + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); > + back_refs[~tag] = existing; > + } > + > dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); > > if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) > > would suffice? If not we probably need to take inspiration from the > TREE_BINFO streaming, and handle LAMBDA_EXPR similarly.. > Ah yup, right, that makes more sense. Your suggestion seems to work, thanks! Here's an updated patch. Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? -- >8 -- The fix for PR107398 weakened the restrictions that lambdas must belong to namespace scope. However this was not sufficient: we also need to allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. For field decls we key the lambda to its class rather than the field itself. This avoids some errors with deduplicating fields. Additionally, by [basic.link] p15.2 a lambda defined anywhere in a class-specifier should not be TU-local, which includes base-class declarations, so ensure that lambdas declared there are keyed appropriately as well. Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a fairly large number of different kinds of DECLs, and that in general it's safe to just get 'false' as a result of a check on an unexpected DECL type, this patch also removes the tree checking from the accessor. Finally, to handle deduplicating templated lambda fields, we need to ensure that we can determine that two lambdas from different field decls match, so we ensure that we deduplicate LAMBDA_EXPRs on stream in. PR c++/111710 gcc/cp/ChangeLog: * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. (struct lang_decl_base): Update comments and fix whitespace. * module.cc (trees_out::lang_decl_bools): Always write module_keyed_decls_p flag... (trees_in::lang_decl_bools): ...and always read it. (trees_out::decl_value): Handle all kinds of keyed decls. (trees_in::decl_value): Likewise. (trees_in::tree_value): Deduplicate LAMBDA_EXPRs. (maybe_key_decl): Also support lambdas attached to fields, parameters, and types. Key lambdas attached to fields to their class. (trees_out::get_merge_kind): Likewise. (trees_out::key_mergeable): Likewise. (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL container. * parser.cc (cp_parser_class_head): Start a lambda scope when parsing base classes. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-7.h: New test. * g++.dg/modules/lambda-7_a.H: New test. * g++.dg/modules/lambda-7_b.C: New test. * g++.dg/modules/lambda-7_c.C: New test. Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> Reviewed-by: Patrick Palka <ppalka@redhat.com> --- gcc/cp/cp-tree.h | 26 +++--- gcc/cp/module.cc | 101 +++++++++++++--------- gcc/cp/parser.cc | 10 ++- gcc/testsuite/g++.dg/modules/lambda-7.h | 42 +++++++++ gcc/testsuite/g++.dg/modules/lambda-7_a.H | 4 + gcc/testsuite/g++.dg/modules/lambda-7_b.C | 5 ++ gcc/testsuite/g++.dg/modules/lambda-7_c.C | 41 +++++++++ 7 files changed, 174 insertions(+), 55 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7.h create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.H create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_c.C diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 334c11396c2..04c3aa6cd91 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) /* DECL that has attached decls for ODR-relatedness. */ -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ - ->u.base.module_keyed_decls_p) +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) /* Whether this is an exported DECL. Held on any decl that can appear at namespace scope (function, var, type, template, const or @@ -2887,21 +2886,20 @@ struct GTY(()) lang_decl_base { unsigned friend_or_tls : 1; /* var, fn, type or template */ unsigned unknown_bound_p : 1; /* var */ unsigned odr_used : 1; /* var or fn */ - unsigned concept_p : 1; /* applies to vars and functions */ + unsigned concept_p : 1; /* applies to vars and functions */ unsigned var_declared_inline_p : 1; /* var */ unsigned dependent_init_p : 1; /* var */ - /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE + /* The following four apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE decls. */ - unsigned module_purview_p : 1; // in named-module purview - unsigned module_attach_p : 1; // attached to named module - unsigned module_import_p : 1; /* from an import */ - unsigned module_entity_p : 1; /* is in the entitity ary & - hash. */ - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ - unsigned module_keyed_decls_p : 1; - - /* 12 spare bits. */ + unsigned module_purview_p : 1; /* in named-module purview */ + unsigned module_attach_p : 1; /* attached to named module */ + unsigned module_import_p : 1; /* from an import */ + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ + + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ + + /* 11 spare bits. */ }; /* True for DECL codes which have template info and access. */ diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc index 106af7bdb3e..1b2ba2e0fa8 100644 --- a/gcc/cp/module.cc +++ b/gcc/cp/module.cc @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t) want to mark them as in module purview. */ WB (lang->u.base.module_purview_p && !header_module_p ()); WB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - WB (lang->u.base.module_keyed_decls_p); + WB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -5738,8 +5737,7 @@ trees_in::lang_decl_bools (tree t) RB (lang->u.base.dependent_init_p); RB (lang->u.base.module_purview_p); RB (lang->u.base.module_attach_p); - if (VAR_OR_FUNCTION_DECL_P (t)) - RB (lang->u.base.module_keyed_decls_p); + RB (lang->u.base.module_keyed_decls_p); switch (lang->u.base.selector) { default: @@ -7871,8 +7869,7 @@ trees_out::decl_value (tree decl, depset *dep) install_entity (decl, dep); } - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner) && !is_key_order ()) { @@ -8172,8 +8169,7 @@ trees_in::decl_value () bool installed = install_entity (existing); bool is_new = existing == decl; - if (VAR_OR_FUNCTION_DECL_P (inner) - && DECL_LANG_SPECIFIC (inner) + if (DECL_LANG_SPECIFIC (inner) && DECL_MODULE_KEYED_DECLS_P (inner)) { /* Read and maybe install the attached entities. */ @@ -9187,6 +9183,13 @@ trees_in::tree_value () return NULL_TREE; } + if (TREE_CODE (t) == LAMBDA_EXPR + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) + { + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); + back_refs[~tag] = existing; + } + dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) @@ -10484,12 +10487,17 @@ trees_out::get_merge_kind (tree decl, depset *dep) if (tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (decl)))) - if (TREE_CODE (scope) == VAR_DECL - && DECL_MODULE_KEYED_DECLS_P (scope)) - { - mk = MK_keyed; - break; - } + { + /* Lambdas attached to fields are keyed to its class. */ + if (TREE_CODE (scope) == FIELD_DECL) + scope = TYPE_NAME (DECL_CONTEXT (scope)); + if (DECL_LANG_SPECIFIC (scope) + && DECL_MODULE_KEYED_DECLS_P (scope)) + { + mk = MK_keyed; + break; + } + } if (RECORD_OR_UNION_TYPE_P (ctx)) { @@ -10789,7 +10797,13 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (inner))); - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL + || TREE_CODE (scope) == FIELD_DECL + || TREE_CODE (scope) == PARM_DECL + || TREE_CODE (scope) == TYPE_DECL); + /* Lambdas attached to fields are keyed to the class. */ + if (TREE_CODE (scope) == FIELD_DECL) + scope = TYPE_NAME (DECL_CONTEXT (scope)); auto *root = keyed_table->get (scope); unsigned ix = root->length (); /* If we don't find it, we'll write a really big number @@ -11067,6 +11081,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, } } } + else if (mk == MK_keyed + && DECL_LANG_SPECIFIC (name) + && DECL_MODULE_KEYED_DECLS_P (name)) + { + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL + || TREE_CODE (container) == TYPE_DECL); + if (auto *set = keyed_table->get (name)) + if (key.index < set->length ()) + { + existing = (*set)[key.index]; + if (existing) + { + gcc_checking_assert + (DECL_IMPLICIT_TYPEDEF_P (existing)); + if (inner != decl) + existing + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); + } + } + } else switch (TREE_CODE (container)) { @@ -11074,27 +11108,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, gcc_unreachable (); case NAMESPACE_DECL: - if (mk == MK_keyed) - { - if (DECL_LANG_SPECIFIC (name) - && VAR_OR_FUNCTION_DECL_P (name) - && DECL_MODULE_KEYED_DECLS_P (name)) - if (auto *set = keyed_table->get (name)) - if (key.index < set->length ()) - { - existing = (*set)[key.index]; - if (existing) - { - gcc_checking_assert - (DECL_IMPLICIT_TYPEDEF_P (existing)); - if (inner != decl) - existing - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); - } - } - } - else if (is_attached - && !(state->is_module () || state->is_partition ())) + if (is_attached + && !(state->is_module () || state->is_partition ())) kind = "unique"; else { @@ -18984,11 +18999,19 @@ maybe_key_decl (tree ctx, tree decl) if (!modules_p ()) return; - // FIXME: For now just deal with lambdas attached to var decls. - // This might be sufficient? - if (TREE_CODE (ctx) != VAR_DECL) + /* We only need to deal with lambdas attached to var, field, + parm, or type decls. */ + if (TREE_CODE (ctx) != VAR_DECL + && TREE_CODE (ctx) != FIELD_DECL + && TREE_CODE (ctx) != PARM_DECL + && TREE_CODE (ctx) != TYPE_DECL) return; + /* For fields, key it to the containing type to handle deduplication + correctly. */ + if (TREE_CODE (ctx) == FIELD_DECL) + ctx = TYPE_NAME (DECL_CONTEXT (ctx)); + if (!keyed_table) keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index b2ed2baa3a5..3ee9d49fb8e 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -27678,10 +27678,16 @@ cp_parser_class_head (cp_parser* parser, if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) { if (type) - pushclass (type); + { + pushclass (type); + start_lambda_scope (TYPE_NAME (type)); + } bases = cp_parser_base_clause (parser); if (type) - popclass (); + { + finish_lambda_scope (); + popclass (); + } } else bases = NULL_TREE; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h new file mode 100644 index 00000000000..6f6080c1324 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h @@ -0,0 +1,42 @@ +struct S { + int (*a)(int) = [](int x) { return x * 2; }; + + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { + return f(x); + } + + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { + return f(x); + } +}; + +inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { + return f(x); +} + +// unevaluated lambdas +#if __cplusplus >= 202002L +struct E : decltype([](int x) { return x * 6; }) { + decltype([](int x) { return x * 7; }) f; +}; + +template <typename T> +struct G : decltype([](int x) { return x * 8; }) { + decltype([](int x) { return x * 9; }) h; +}; + +template <> +struct G<double> : decltype([](int x) { return x * 10; }) { + decltype([](int x) { return x * 11; }) i; +}; +#endif + +// concepts +#if __cpp_concepts >= 201907L +template <typename T> +concept J = requires { []{ T(); }; }; + +template <typename T> +concept K = []{ return sizeof(T) == 1; }(); +#endif + diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.H b/gcc/testsuite/g++.dg/modules/lambda-7_a.H new file mode 100644 index 00000000000..5197114f76c --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.H @@ -0,0 +1,4 @@ +// { dg-additional-options "-fmodule-header -Wno-subobject-linkage" } +// { dg-module-cmi {} } + +#include "lambda-7.h" diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C new file mode 100644 index 00000000000..2d781e93067 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C @@ -0,0 +1,5 @@ +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } +// Test for ODR deduplication + +#include "lambda-7.h" +import "lambda-7_a.H"; diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_c.C b/gcc/testsuite/g++.dg/modules/lambda-7_c.C new file mode 100644 index 00000000000..f283681fa96 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-7_c.C @@ -0,0 +1,41 @@ +// { dg-module-do run } +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } + +import "lambda-7_a.H"; + +int main() { + S s; + if (s.a(10) != 20) + __builtin_abort(); + if (s.b(10) != 30) + __builtin_abort(); + if (s.c(10) != 40) + __builtin_abort(); + if (d(10) != 50) + __builtin_abort(); + +#if __cplusplus >= 202002L + E e; + if (e(10) != 60) + __builtin_abort(); + if (e.f(10) != 70) + __builtin_abort(); + + G<int> g1; + if (g1(10) != 80) + __builtin_abort(); + if (g1.h(10) != 90) + __builtin_abort(); + + G<double> g2; + if (g2(10) != 100) + __builtin_abort(); + if (g2.i(10) != 110) + __builtin_abort(); +#endif + +#if __cpp_concepts >= 201907L + static_assert(J<char>); + static_assert(K<char>); +#endif +} -- 2.43.0 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] c++/modules: Support lambdas attached to more places in modules [PR111710] 2024-02-28 4:12 ` [PATCH v3] " Nathaniel Shead @ 2024-02-28 17:34 ` Jason Merrill 2024-02-29 4:57 ` Nathaniel Shead 0 siblings, 1 reply; 12+ messages in thread From: Jason Merrill @ 2024-02-28 17:34 UTC (permalink / raw) To: Nathaniel Shead, Patrick Palka; +Cc: gcc-patches, Nathan Sidwell On 2/27/24 23:12, Nathaniel Shead wrote: > On Tue, Feb 27, 2024 at 11:59:46AM -0500, Patrick Palka wrote: >> On Fri, 16 Feb 2024, Nathaniel Shead wrote: >> >>> On Tue, Feb 13, 2024 at 07:52:01PM -0500, Jason Merrill wrote: >>>> On 2/10/24 17:57, Nathaniel Shead wrote: >>>>> The fix for PR107398 weakened the restrictions that lambdas must belong >>>>> to namespace scope. However this was not sufficient: we also need to >>>>> allow lambdas keyed to FIELD_DECLs or PARM_DECLs. >>>> >>>> I wonder about keying such lambdas to the class and function, respectively, >>>> rather than specifically to the field or parameter, but I suppose it doesn't >>>> matter. >>> >>> I did some more testing and realised my testcase didn't properly >>> exercise whether I'd properly deduplicated or not, and an improved >>> testcase proved that actually keying to the field rather than the class >>> did cause issues. (Parameter vs. function doesn't seem to have mattered >>> however.) >>> >>> Here's an updated patch that fixes this, and includes the changes for >>> lambdas in base classes that I'd had as a separate patch earlier. I've >>> also added some concepts testcases just in case. >>> >>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? >>> >>> -- >8 -- >>> >>> The fix for PR107398 weakened the restrictions that lambdas must belong >>> to namespace scope. However this was not sufficient: we also need to >>> allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. >>> >>> For field decls we key the lambda to its class rather than the field >>> itself. This avoids some errors with deduplicating fields. >>> >>> Additionally, by [basic.link] p15.2 a lambda defined anywhere in a >>> class-specifier should not be TU-local, which includes base-class >>> declarations, so ensure that lambdas declared there are keyed >>> appropriately as well. >>> >>> Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a >>> fairly large number of different kinds of DECLs, and that in general >>> it's safe to just get 'false' as a result of a check on an unexpected >>> DECL type, this patch also removes the tree checking from the accessor. >>> >>> Finally, to handle deduplicating templated lambda fields, we need to >>> ensure that we can determine that two lambdas from different field decls >>> match. The modules code does not attempt to deduplicate expression >>> nodes, which causes issues as the LAMBDA_EXPRs are then considered to be >>> different. However, rather than checking the LAMBDA_EXPR directly we can >>> instead check its type: the generated RECORD_TYPE for a LAMBDA_EXPR must >>> also be unique, and /is/ deduplicated on import, so we can just check >>> for that instead. >> >> We probably should be deduplicating LAMBDA_EXPR on stream-in, perhaps >> something like >> >> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc >> index e8eabb1f6f9..1b2ba2e0fa8 100644 >> --- a/gcc/cp/module.cc >> +++ b/gcc/cp/module.cc >> @@ -9183,6 +9183,13 @@ trees_in::tree_value () >> return NULL_TREE; >> } >> >> + if (TREE_CODE (t) == LAMBDA_EXPR >> + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) >> + { >> + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); >> + back_refs[~tag] = existing; >> + } >> + >> dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); >> >> if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) >> >> would suffice? If not we probably need to take inspiration from the >> TREE_BINFO streaming, and handle LAMBDA_EXPR similarly.. >> > > Ah yup, right, that makes more sense. Your suggestion seems to work, > thanks! Here's an updated patch. > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? With that change, do you still need to key to the class instead of the field for dedup to work properly? OK either way. > -- >8 -- > > The fix for PR107398 weakened the restrictions that lambdas must belong > to namespace scope. However this was not sufficient: we also need to > allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. > > For field decls we key the lambda to its class rather than the field > itself. This avoids some errors with deduplicating fields. > > Additionally, by [basic.link] p15.2 a lambda defined anywhere in a > class-specifier should not be TU-local, which includes base-class > declarations, so ensure that lambdas declared there are keyed > appropriately as well. > > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > fairly large number of different kinds of DECLs, and that in general > it's safe to just get 'false' as a result of a check on an unexpected > DECL type, this patch also removes the tree checking from the accessor. > > Finally, to handle deduplicating templated lambda fields, we need to > ensure that we can determine that two lambdas from different field decls > match, so we ensure that we deduplicate LAMBDA_EXPRs on stream in. > > PR c++/111710 > > gcc/cp/ChangeLog: > > * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. > (struct lang_decl_base): Update comments and fix whitespace. > * module.cc (trees_out::lang_decl_bools): Always write > module_keyed_decls_p flag... > (trees_in::lang_decl_bools): ...and always read it. > (trees_out::decl_value): Handle all kinds of keyed decls. > (trees_in::decl_value): Likewise. > (trees_in::tree_value): Deduplicate LAMBDA_EXPRs. > (maybe_key_decl): Also support lambdas attached to fields, > parameters, and types. Key lambdas attached to fields to their > class. > (trees_out::get_merge_kind): Likewise. > (trees_out::key_mergeable): Likewise. > (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL > container. > * parser.cc (cp_parser_class_head): Start a lambda scope when > parsing base classes. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/lambda-7.h: New test. > * g++.dg/modules/lambda-7_a.H: New test. > * g++.dg/modules/lambda-7_b.C: New test. > * g++.dg/modules/lambda-7_c.C: New test. > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > Reviewed-by: Patrick Palka <ppalka@redhat.com> > --- > gcc/cp/cp-tree.h | 26 +++--- > gcc/cp/module.cc | 101 +++++++++++++--------- > gcc/cp/parser.cc | 10 ++- > gcc/testsuite/g++.dg/modules/lambda-7.h | 42 +++++++++ > gcc/testsuite/g++.dg/modules/lambda-7_a.H | 4 + > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 5 ++ > gcc/testsuite/g++.dg/modules/lambda-7_c.C | 41 +++++++++ > 7 files changed, 174 insertions(+), 55 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7.h > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.H > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_c.C > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > index 334c11396c2..04c3aa6cd91 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) > (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) > > /* DECL that has attached decls for ODR-relatedness. */ > -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ > - ->u.base.module_keyed_decls_p) > +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) > > /* Whether this is an exported DECL. Held on any decl that can appear > at namespace scope (function, var, type, template, const or > @@ -2887,21 +2886,20 @@ struct GTY(()) lang_decl_base { > unsigned friend_or_tls : 1; /* var, fn, type or template */ > unsigned unknown_bound_p : 1; /* var */ > unsigned odr_used : 1; /* var or fn */ > - unsigned concept_p : 1; /* applies to vars and functions */ > + unsigned concept_p : 1; /* applies to vars and functions */ > unsigned var_declared_inline_p : 1; /* var */ > unsigned dependent_init_p : 1; /* var */ > > - /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > + /* The following four apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > decls. */ > - unsigned module_purview_p : 1; // in named-module purview > - unsigned module_attach_p : 1; // attached to named module > - unsigned module_import_p : 1; /* from an import */ > - unsigned module_entity_p : 1; /* is in the entitity ary & > - hash. */ > - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ > - unsigned module_keyed_decls_p : 1; > - > - /* 12 spare bits. */ > + unsigned module_purview_p : 1; /* in named-module purview */ > + unsigned module_attach_p : 1; /* attached to named module */ > + unsigned module_import_p : 1; /* from an import */ > + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ > + > + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ > + > + /* 11 spare bits. */ > }; > > /* True for DECL codes which have template info and access. */ > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 106af7bdb3e..1b2ba2e0fa8 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t) > want to mark them as in module purview. */ > WB (lang->u.base.module_purview_p && !header_module_p ()); > WB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - WB (lang->u.base.module_keyed_decls_p); > + WB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -5738,8 +5737,7 @@ trees_in::lang_decl_bools (tree t) > RB (lang->u.base.dependent_init_p); > RB (lang->u.base.module_purview_p); > RB (lang->u.base.module_attach_p); > - if (VAR_OR_FUNCTION_DECL_P (t)) > - RB (lang->u.base.module_keyed_decls_p); > + RB (lang->u.base.module_keyed_decls_p); > switch (lang->u.base.selector) > { > default: > @@ -7871,8 +7869,7 @@ trees_out::decl_value (tree decl, depset *dep) > install_entity (decl, dep); > } > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner) > && !is_key_order ()) > { > @@ -8172,8 +8169,7 @@ trees_in::decl_value () > bool installed = install_entity (existing); > bool is_new = existing == decl; > > - if (VAR_OR_FUNCTION_DECL_P (inner) > - && DECL_LANG_SPECIFIC (inner) > + if (DECL_LANG_SPECIFIC (inner) > && DECL_MODULE_KEYED_DECLS_P (inner)) > { > /* Read and maybe install the attached entities. */ > @@ -9187,6 +9183,13 @@ trees_in::tree_value () > return NULL_TREE; > } > > + if (TREE_CODE (t) == LAMBDA_EXPR > + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) > + { > + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); > + back_refs[~tag] = existing; > + } > + > dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); > > if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) > @@ -10484,12 +10487,17 @@ trees_out::get_merge_kind (tree decl, depset *dep) > if (tree scope > = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (decl)))) > - if (TREE_CODE (scope) == VAR_DECL > - && DECL_MODULE_KEYED_DECLS_P (scope)) > - { > - mk = MK_keyed; > - break; > - } > + { > + /* Lambdas attached to fields are keyed to its class. */ > + if (TREE_CODE (scope) == FIELD_DECL) > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > + if (DECL_LANG_SPECIFIC (scope) > + && DECL_MODULE_KEYED_DECLS_P (scope)) > + { > + mk = MK_keyed; > + break; > + } > + } > > if (RECORD_OR_UNION_TYPE_P (ctx)) > { > @@ -10789,7 +10797,13 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); > tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > (TREE_TYPE (inner))); > - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); > + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > + || TREE_CODE (scope) == FIELD_DECL > + || TREE_CODE (scope) == PARM_DECL > + || TREE_CODE (scope) == TYPE_DECL); > + /* Lambdas attached to fields are keyed to the class. */ > + if (TREE_CODE (scope) == FIELD_DECL) > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > auto *root = keyed_table->get (scope); > unsigned ix = root->length (); > /* If we don't find it, we'll write a really big number > @@ -11067,6 +11081,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > } > } > } > + else if (mk == MK_keyed > + && DECL_LANG_SPECIFIC (name) > + && DECL_MODULE_KEYED_DECLS_P (name)) > + { > + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL > + || TREE_CODE (container) == TYPE_DECL); > + if (auto *set = keyed_table->get (name)) > + if (key.index < set->length ()) > + { > + existing = (*set)[key.index]; > + if (existing) > + { > + gcc_checking_assert > + (DECL_IMPLICIT_TYPEDEF_P (existing)); > + if (inner != decl) > + existing > + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > + } > + } > + } > else > switch (TREE_CODE (container)) > { > @@ -11074,27 +11108,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > gcc_unreachable (); > > case NAMESPACE_DECL: > - if (mk == MK_keyed) > - { > - if (DECL_LANG_SPECIFIC (name) > - && VAR_OR_FUNCTION_DECL_P (name) > - && DECL_MODULE_KEYED_DECLS_P (name)) > - if (auto *set = keyed_table->get (name)) > - if (key.index < set->length ()) > - { > - existing = (*set)[key.index]; > - if (existing) > - { > - gcc_checking_assert > - (DECL_IMPLICIT_TYPEDEF_P (existing)); > - if (inner != decl) > - existing > - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > - } > - } > - } > - else if (is_attached > - && !(state->is_module () || state->is_partition ())) > + if (is_attached > + && !(state->is_module () || state->is_partition ())) > kind = "unique"; > else > { > @@ -18984,11 +18999,19 @@ maybe_key_decl (tree ctx, tree decl) > if (!modules_p ()) > return; > > - // FIXME: For now just deal with lambdas attached to var decls. > - // This might be sufficient? > - if (TREE_CODE (ctx) != VAR_DECL) > + /* We only need to deal with lambdas attached to var, field, > + parm, or type decls. */ > + if (TREE_CODE (ctx) != VAR_DECL > + && TREE_CODE (ctx) != FIELD_DECL > + && TREE_CODE (ctx) != PARM_DECL > + && TREE_CODE (ctx) != TYPE_DECL) > return; > > + /* For fields, key it to the containing type to handle deduplication > + correctly. */ > + if (TREE_CODE (ctx) == FIELD_DECL) > + ctx = TYPE_NAME (DECL_CONTEXT (ctx)); > + > if (!keyed_table) > keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index b2ed2baa3a5..3ee9d49fb8e 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -27678,10 +27678,16 @@ cp_parser_class_head (cp_parser* parser, > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > { > if (type) > - pushclass (type); > + { > + pushclass (type); > + start_lambda_scope (TYPE_NAME (type)); > + } > bases = cp_parser_base_clause (parser); > if (type) > - popclass (); > + { > + finish_lambda_scope (); > + popclass (); > + } > } > else > bases = NULL_TREE; > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h > new file mode 100644 > index 00000000000..6f6080c1324 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h > @@ -0,0 +1,42 @@ > +struct S { > + int (*a)(int) = [](int x) { return x * 2; }; > + > + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { > + return f(x); > + } > + > + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { > + return f(x); > + } > +}; > + > +inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > + return f(x); > +} > + > +// unevaluated lambdas > +#if __cplusplus >= 202002L > +struct E : decltype([](int x) { return x * 6; }) { > + decltype([](int x) { return x * 7; }) f; > +}; > + > +template <typename T> > +struct G : decltype([](int x) { return x * 8; }) { > + decltype([](int x) { return x * 9; }) h; > +}; > + > +template <> > +struct G<double> : decltype([](int x) { return x * 10; }) { > + decltype([](int x) { return x * 11; }) i; > +}; > +#endif > + > +// concepts > +#if __cpp_concepts >= 201907L > +template <typename T> > +concept J = requires { []{ T(); }; }; > + > +template <typename T> > +concept K = []{ return sizeof(T) == 1; }(); > +#endif > + > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.H b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > new file mode 100644 > index 00000000000..5197114f76c > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > @@ -0,0 +1,4 @@ > +// { dg-additional-options "-fmodule-header -Wno-subobject-linkage" } > +// { dg-module-cmi {} } > + > +#include "lambda-7.h" > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > new file mode 100644 > index 00000000000..2d781e93067 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > @@ -0,0 +1,5 @@ > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > +// Test for ODR deduplication > + > +#include "lambda-7.h" > +import "lambda-7_a.H"; > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_c.C b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > new file mode 100644 > index 00000000000..f283681fa96 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > @@ -0,0 +1,41 @@ > +// { dg-module-do run } > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > + > +import "lambda-7_a.H"; > + > +int main() { > + S s; > + if (s.a(10) != 20) > + __builtin_abort(); > + if (s.b(10) != 30) > + __builtin_abort(); > + if (s.c(10) != 40) > + __builtin_abort(); > + if (d(10) != 50) > + __builtin_abort(); > + > +#if __cplusplus >= 202002L > + E e; > + if (e(10) != 60) > + __builtin_abort(); > + if (e.f(10) != 70) > + __builtin_abort(); > + > + G<int> g1; > + if (g1(10) != 80) > + __builtin_abort(); > + if (g1.h(10) != 90) > + __builtin_abort(); > + > + G<double> g2; > + if (g2(10) != 100) > + __builtin_abort(); > + if (g2.i(10) != 110) > + __builtin_abort(); > +#endif > + > +#if __cpp_concepts >= 201907L > + static_assert(J<char>); > + static_assert(K<char>); > +#endif > +} ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH v3] c++/modules: Support lambdas attached to more places in modules [PR111710] 2024-02-28 17:34 ` Jason Merrill @ 2024-02-29 4:57 ` Nathaniel Shead 0 siblings, 0 replies; 12+ messages in thread From: Nathaniel Shead @ 2024-02-29 4:57 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches, Nathan Sidwell On Wed, Feb 28, 2024 at 12:34:51PM -0500, Jason Merrill wrote: > On 2/27/24 23:12, Nathaniel Shead wrote: > > On Tue, Feb 27, 2024 at 11:59:46AM -0500, Patrick Palka wrote: > > > On Fri, 16 Feb 2024, Nathaniel Shead wrote: > > > > > > > On Tue, Feb 13, 2024 at 07:52:01PM -0500, Jason Merrill wrote: > > > > > On 2/10/24 17:57, Nathaniel Shead wrote: > > > > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > > > > > to namespace scope. However this was not sufficient: we also need to > > > > > > allow lambdas keyed to FIELD_DECLs or PARM_DECLs. > > > > > > > > > > I wonder about keying such lambdas to the class and function, respectively, > > > > > rather than specifically to the field or parameter, but I suppose it doesn't > > > > > matter. > > > > > > > > I did some more testing and realised my testcase didn't properly > > > > exercise whether I'd properly deduplicated or not, and an improved > > > > testcase proved that actually keying to the field rather than the class > > > > did cause issues. (Parameter vs. function doesn't seem to have mattered > > > > however.) > > > > > > > > Here's an updated patch that fixes this, and includes the changes for > > > > lambdas in base classes that I'd had as a separate patch earlier. I've > > > > also added some concepts testcases just in case. > > > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > > > > > > > -- >8 -- > > > > > > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > > > to namespace scope. However this was not sufficient: we also need to > > > > allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. > > > > > > > > For field decls we key the lambda to its class rather than the field > > > > itself. This avoids some errors with deduplicating fields. > > > > > > > > Additionally, by [basic.link] p15.2 a lambda defined anywhere in a > > > > class-specifier should not be TU-local, which includes base-class > > > > declarations, so ensure that lambdas declared there are keyed > > > > appropriately as well. > > > > > > > > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > > > > fairly large number of different kinds of DECLs, and that in general > > > > it's safe to just get 'false' as a result of a check on an unexpected > > > > DECL type, this patch also removes the tree checking from the accessor. > > > > > > > > Finally, to handle deduplicating templated lambda fields, we need to > > > > ensure that we can determine that two lambdas from different field decls > > > > match. The modules code does not attempt to deduplicate expression > > > > nodes, which causes issues as the LAMBDA_EXPRs are then considered to be > > > > different. However, rather than checking the LAMBDA_EXPR directly we can > > > > instead check its type: the generated RECORD_TYPE for a LAMBDA_EXPR must > > > > also be unique, and /is/ deduplicated on import, so we can just check > > > > for that instead. > > > > > > We probably should be deduplicating LAMBDA_EXPR on stream-in, perhaps > > > something like > > > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > > > index e8eabb1f6f9..1b2ba2e0fa8 100644 > > > --- a/gcc/cp/module.cc > > > +++ b/gcc/cp/module.cc > > > @@ -9183,6 +9183,13 @@ trees_in::tree_value () > > > return NULL_TREE; > > > } > > > + if (TREE_CODE (t) == LAMBDA_EXPR > > > + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) > > > + { > > > + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); > > > + back_refs[~tag] = existing; > > > + } > > > + > > > dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); > > > if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) > > > > > > would suffice? If not we probably need to take inspiration from the > > > TREE_BINFO streaming, and handle LAMBDA_EXPR similarly.. > > > > > > > Ah yup, right, that makes more sense. Your suggestion seems to work, > > thanks! Here's an updated patch. > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk? > > With that change, do you still need to key to the class instead of the field > for dedup to work properly? > > OK either way. > Thanks. And yes, we still need to key to the class rather than the field; it looks like otherwise what happens is that when reading the lambda's TYPE_DECL for a lambda used as an NSDMI, 'key_mergeable' ends up loading the FIELD_DECL it's attached to (and attempting to install its keyed declarations) when reading its 'name' before it gets a chance to decide that the TYPE_DECL has an existing definition and deduplicate, which causes 'set_overrun' to be called due to apparent mismatch. By keying to the class instead of the field this isn't an issue because the keyed declarations of the containing type are handled after the nested TYPE_DECLs are deduplicated. I'll added a comment to this effect to the commit message. > > -- >8 -- > > > > The fix for PR107398 weakened the restrictions that lambdas must belong > > to namespace scope. However this was not sufficient: we also need to > > allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs. > > > > For field decls we key the lambda to its class rather than the field > > itself. This avoids some errors with deduplicating fields. > > > > Additionally, by [basic.link] p15.2 a lambda defined anywhere in a > > class-specifier should not be TU-local, which includes base-class > > declarations, so ensure that lambdas declared there are keyed > > appropriately as well. > > > > Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a > > fairly large number of different kinds of DECLs, and that in general > > it's safe to just get 'false' as a result of a check on an unexpected > > DECL type, this patch also removes the tree checking from the accessor. > > > > Finally, to handle deduplicating templated lambda fields, we need to > > ensure that we can determine that two lambdas from different field decls > > match, so we ensure that we deduplicate LAMBDA_EXPRs on stream in. > > > > PR c++/111710 > > > > gcc/cp/ChangeLog: > > > > * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking. > > (struct lang_decl_base): Update comments and fix whitespace. > > * module.cc (trees_out::lang_decl_bools): Always write > > module_keyed_decls_p flag... > > (trees_in::lang_decl_bools): ...and always read it. > > (trees_out::decl_value): Handle all kinds of keyed decls. > > (trees_in::decl_value): Likewise. > > (trees_in::tree_value): Deduplicate LAMBDA_EXPRs. > > (maybe_key_decl): Also support lambdas attached to fields, > > parameters, and types. Key lambdas attached to fields to their > > class. > > (trees_out::get_merge_kind): Likewise. > > (trees_out::key_mergeable): Likewise. > > (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL > > container. > > * parser.cc (cp_parser_class_head): Start a lambda scope when > > parsing base classes. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/modules/lambda-7.h: New test. > > * g++.dg/modules/lambda-7_a.H: New test. > > * g++.dg/modules/lambda-7_b.C: New test. > > * g++.dg/modules/lambda-7_c.C: New test. > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com> > > Reviewed-by: Patrick Palka <ppalka@redhat.com> > > --- > > gcc/cp/cp-tree.h | 26 +++--- > > gcc/cp/module.cc | 101 +++++++++++++--------- > > gcc/cp/parser.cc | 10 ++- > > gcc/testsuite/g++.dg/modules/lambda-7.h | 42 +++++++++ > > gcc/testsuite/g++.dg/modules/lambda-7_a.H | 4 + > > gcc/testsuite/g++.dg/modules/lambda-7_b.C | 5 ++ > > gcc/testsuite/g++.dg/modules/lambda-7_c.C | 41 +++++++++ > > 7 files changed, 174 insertions(+), 55 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7.h > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_a.H > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_b.C > > create mode 100644 gcc/testsuite/g++.dg/modules/lambda-7_c.C > > > > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h > > index 334c11396c2..04c3aa6cd91 100644 > > --- a/gcc/cp/cp-tree.h > > +++ b/gcc/cp/cp-tree.h > > @@ -1773,9 +1773,8 @@ check_constraint_info (tree t) > > (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) > > /* DECL that has attached decls for ODR-relatedness. */ > > -#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > > - (DECL_LANG_SPECIFIC (TREE_CHECK2(NODE,FUNCTION_DECL,VAR_DECL))\ > > - ->u.base.module_keyed_decls_p) > > +#define DECL_MODULE_KEYED_DECLS_P(NODE) \ > > + (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_keyed_decls_p) > > /* Whether this is an exported DECL. Held on any decl that can appear > > at namespace scope (function, var, type, template, const or > > @@ -2887,21 +2886,20 @@ struct GTY(()) lang_decl_base { > > unsigned friend_or_tls : 1; /* var, fn, type or template */ > > unsigned unknown_bound_p : 1; /* var */ > > unsigned odr_used : 1; /* var or fn */ > > - unsigned concept_p : 1; /* applies to vars and functions */ > > + unsigned concept_p : 1; /* applies to vars and functions */ > > unsigned var_declared_inline_p : 1; /* var */ > > unsigned dependent_init_p : 1; /* var */ > > - /* The following apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > > + /* The following four apply to VAR, FUNCTION, TYPE, CONCEPT, & NAMESPACE > > decls. */ > > - unsigned module_purview_p : 1; // in named-module purview > > - unsigned module_attach_p : 1; // attached to named module > > - unsigned module_import_p : 1; /* from an import */ > > - unsigned module_entity_p : 1; /* is in the entitity ary & > > - hash. */ > > - /* VAR_DECL or FUNCTION_DECL has keyed decls. */ > > - unsigned module_keyed_decls_p : 1; > > - > > - /* 12 spare bits. */ > > + unsigned module_purview_p : 1; /* in named-module purview */ > > + unsigned module_attach_p : 1; /* attached to named module */ > > + unsigned module_import_p : 1; /* from an import */ > > + unsigned module_entity_p : 1; /* is in the entitity ary & hash */ > > + > > + unsigned module_keyed_decls_p : 1; /* has keys, applies to all decls */ > > + > > + /* 11 spare bits. */ > > }; > > /* True for DECL codes which have template info and access. */ > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > > index 106af7bdb3e..1b2ba2e0fa8 100644 > > --- a/gcc/cp/module.cc > > +++ b/gcc/cp/module.cc > > @@ -5664,8 +5664,7 @@ trees_out::lang_decl_bools (tree t) > > want to mark them as in module purview. */ > > WB (lang->u.base.module_purview_p && !header_module_p ()); > > WB (lang->u.base.module_attach_p); > > - if (VAR_OR_FUNCTION_DECL_P (t)) > > - WB (lang->u.base.module_keyed_decls_p); > > + WB (lang->u.base.module_keyed_decls_p); > > switch (lang->u.base.selector) > > { > > default: > > @@ -5738,8 +5737,7 @@ trees_in::lang_decl_bools (tree t) > > RB (lang->u.base.dependent_init_p); > > RB (lang->u.base.module_purview_p); > > RB (lang->u.base.module_attach_p); > > - if (VAR_OR_FUNCTION_DECL_P (t)) > > - RB (lang->u.base.module_keyed_decls_p); > > + RB (lang->u.base.module_keyed_decls_p); > > switch (lang->u.base.selector) > > { > > default: > > @@ -7871,8 +7869,7 @@ trees_out::decl_value (tree decl, depset *dep) > > install_entity (decl, dep); > > } > > - if (VAR_OR_FUNCTION_DECL_P (inner) > > - && DECL_LANG_SPECIFIC (inner) > > + if (DECL_LANG_SPECIFIC (inner) > > && DECL_MODULE_KEYED_DECLS_P (inner) > > && !is_key_order ()) > > { > > @@ -8172,8 +8169,7 @@ trees_in::decl_value () > > bool installed = install_entity (existing); > > bool is_new = existing == decl; > > - if (VAR_OR_FUNCTION_DECL_P (inner) > > - && DECL_LANG_SPECIFIC (inner) > > + if (DECL_LANG_SPECIFIC (inner) > > && DECL_MODULE_KEYED_DECLS_P (inner)) > > { > > /* Read and maybe install the attached entities. */ > > @@ -9187,6 +9183,13 @@ trees_in::tree_value () > > return NULL_TREE; > > } > > + if (TREE_CODE (t) == LAMBDA_EXPR > > + && CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t))) > > + { > > + existing = CLASSTYPE_LAMBDA_EXPR (TREE_TYPE (t)); > > + back_refs[~tag] = existing; > > + } > > + > > dump (dumper::TREE) && dump ("Read tree:%d %C:%N", tag, TREE_CODE (t), t); > > if (TREE_CODE (existing) == INTEGER_CST && !TREE_OVERFLOW (existing)) > > @@ -10484,12 +10487,17 @@ trees_out::get_merge_kind (tree decl, depset *dep) > > if (tree scope > > = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > > (TREE_TYPE (decl)))) > > - if (TREE_CODE (scope) == VAR_DECL > > - && DECL_MODULE_KEYED_DECLS_P (scope)) > > - { > > - mk = MK_keyed; > > - break; > > - } > > + { > > + /* Lambdas attached to fields are keyed to its class. */ > > + if (TREE_CODE (scope) == FIELD_DECL) > > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > > + if (DECL_LANG_SPECIFIC (scope) > > + && DECL_MODULE_KEYED_DECLS_P (scope)) > > + { > > + mk = MK_keyed; > > + break; > > + } > > + } > > if (RECORD_OR_UNION_TYPE_P (ctx)) > > { > > @@ -10789,7 +10797,13 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > > gcc_checking_assert (LAMBDA_TYPE_P (TREE_TYPE (inner))); > > tree scope = LAMBDA_EXPR_EXTRA_SCOPE (CLASSTYPE_LAMBDA_EXPR > > (TREE_TYPE (inner))); > > - gcc_checking_assert (TREE_CODE (scope) == VAR_DECL); > > + gcc_checking_assert (TREE_CODE (scope) == VAR_DECL > > + || TREE_CODE (scope) == FIELD_DECL > > + || TREE_CODE (scope) == PARM_DECL > > + || TREE_CODE (scope) == TYPE_DECL); > > + /* Lambdas attached to fields are keyed to the class. */ > > + if (TREE_CODE (scope) == FIELD_DECL) > > + scope = TYPE_NAME (DECL_CONTEXT (scope)); > > auto *root = keyed_table->get (scope); > > unsigned ix = root->length (); > > /* If we don't find it, we'll write a really big number > > @@ -11067,6 +11081,26 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > > } > > } > > } > > + else if (mk == MK_keyed > > + && DECL_LANG_SPECIFIC (name) > > + && DECL_MODULE_KEYED_DECLS_P (name)) > > + { > > + gcc_checking_assert (TREE_CODE (container) == NAMESPACE_DECL > > + || TREE_CODE (container) == TYPE_DECL); > > + if (auto *set = keyed_table->get (name)) > > + if (key.index < set->length ()) > > + { > > + existing = (*set)[key.index]; > > + if (existing) > > + { > > + gcc_checking_assert > > + (DECL_IMPLICIT_TYPEDEF_P (existing)); > > + if (inner != decl) > > + existing > > + = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > > + } > > + } > > + } > > else > > switch (TREE_CODE (container)) > > { > > @@ -11074,27 +11108,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, > > gcc_unreachable (); > > case NAMESPACE_DECL: > > - if (mk == MK_keyed) > > - { > > - if (DECL_LANG_SPECIFIC (name) > > - && VAR_OR_FUNCTION_DECL_P (name) > > - && DECL_MODULE_KEYED_DECLS_P (name)) > > - if (auto *set = keyed_table->get (name)) > > - if (key.index < set->length ()) > > - { > > - existing = (*set)[key.index]; > > - if (existing) > > - { > > - gcc_checking_assert > > - (DECL_IMPLICIT_TYPEDEF_P (existing)); > > - if (inner != decl) > > - existing > > - = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (existing)); > > - } > > - } > > - } > > - else if (is_attached > > - && !(state->is_module () || state->is_partition ())) > > + if (is_attached > > + && !(state->is_module () || state->is_partition ())) > > kind = "unique"; > > else > > { > > @@ -18984,11 +18999,19 @@ maybe_key_decl (tree ctx, tree decl) > > if (!modules_p ()) > > return; > > - // FIXME: For now just deal with lambdas attached to var decls. > > - // This might be sufficient? > > - if (TREE_CODE (ctx) != VAR_DECL) > > + /* We only need to deal with lambdas attached to var, field, > > + parm, or type decls. */ > > + if (TREE_CODE (ctx) != VAR_DECL > > + && TREE_CODE (ctx) != FIELD_DECL > > + && TREE_CODE (ctx) != PARM_DECL > > + && TREE_CODE (ctx) != TYPE_DECL) > > return; > > + /* For fields, key it to the containing type to handle deduplication > > + correctly. */ > > + if (TREE_CODE (ctx) == FIELD_DECL) > > + ctx = TYPE_NAME (DECL_CONTEXT (ctx)); > > + > > if (!keyed_table) > > keyed_table = new keyed_map_t (EXPERIMENT (1, 400)); > > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > > index b2ed2baa3a5..3ee9d49fb8e 100644 > > --- a/gcc/cp/parser.cc > > +++ b/gcc/cp/parser.cc > > @@ -27678,10 +27678,16 @@ cp_parser_class_head (cp_parser* parser, > > if (cp_lexer_next_token_is (parser->lexer, CPP_COLON)) > > { > > if (type) > > - pushclass (type); > > + { > > + pushclass (type); > > + start_lambda_scope (TYPE_NAME (type)); > > + } > > bases = cp_parser_base_clause (parser); > > if (type) > > - popclass (); > > + { > > + finish_lambda_scope (); > > + popclass (); > > + } > > } > > else > > bases = NULL_TREE; > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7.h b/gcc/testsuite/g++.dg/modules/lambda-7.h > > new file mode 100644 > > index 00000000000..6f6080c1324 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7.h > > @@ -0,0 +1,42 @@ > > +struct S { > > + int (*a)(int) = [](int x) { return x * 2; }; > > + > > + int b(int x, int (*f)(int) = [](int x) { return x * 3; }) { > > + return f(x); > > + } > > + > > + static int c(int x, int (*f)(int) = [](int x) { return x * 4; }) { > > + return f(x); > > + } > > +}; > > + > > +inline int d(int x, int (*f)(int) = [](int x) { return x * 5; }) { > > + return f(x); > > +} > > + > > +// unevaluated lambdas > > +#if __cplusplus >= 202002L > > +struct E : decltype([](int x) { return x * 6; }) { > > + decltype([](int x) { return x * 7; }) f; > > +}; > > + > > +template <typename T> > > +struct G : decltype([](int x) { return x * 8; }) { > > + decltype([](int x) { return x * 9; }) h; > > +}; > > + > > +template <> > > +struct G<double> : decltype([](int x) { return x * 10; }) { > > + decltype([](int x) { return x * 11; }) i; > > +}; > > +#endif > > + > > +// concepts > > +#if __cpp_concepts >= 201907L > > +template <typename T> > > +concept J = requires { []{ T(); }; }; > > + > > +template <typename T> > > +concept K = []{ return sizeof(T) == 1; }(); > > +#endif > > + > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_a.H b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > > new file mode 100644 > > index 00000000000..5197114f76c > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_a.H > > @@ -0,0 +1,4 @@ > > +// { dg-additional-options "-fmodule-header -Wno-subobject-linkage" } > > +// { dg-module-cmi {} } > > + > > +#include "lambda-7.h" > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_b.C b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > > new file mode 100644 > > index 00000000000..2d781e93067 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_b.C > > @@ -0,0 +1,5 @@ > > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > > +// Test for ODR deduplication > > + > > +#include "lambda-7.h" > > +import "lambda-7_a.H"; > > diff --git a/gcc/testsuite/g++.dg/modules/lambda-7_c.C b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > > new file mode 100644 > > index 00000000000..f283681fa96 > > --- /dev/null > > +++ b/gcc/testsuite/g++.dg/modules/lambda-7_c.C > > @@ -0,0 +1,41 @@ > > +// { dg-module-do run } > > +// { dg-additional-options "-fmodules-ts -fno-module-lazy -Wno-subobject-linkage" } > > + > > +import "lambda-7_a.H"; > > + > > +int main() { > > + S s; > > + if (s.a(10) != 20) > > + __builtin_abort(); > > + if (s.b(10) != 30) > > + __builtin_abort(); > > + if (s.c(10) != 40) > > + __builtin_abort(); > > + if (d(10) != 50) > > + __builtin_abort(); > > + > > +#if __cplusplus >= 202002L > > + E e; > > + if (e(10) != 60) > > + __builtin_abort(); > > + if (e.f(10) != 70) > > + __builtin_abort(); > > + > > + G<int> g1; > > + if (g1(10) != 80) > > + __builtin_abort(); > > + if (g1.h(10) != 90) > > + __builtin_abort(); > > + > > + G<double> g2; > > + if (g2(10) != 100) > > + __builtin_abort(); > > + if (g2.i(10) != 110) > > + __builtin_abort(); > > +#endif > > + > > +#if __cpp_concepts >= 201907L > > + static_assert(J<char>); > > + static_assert(K<char>); > > +#endif > > +} > ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2024-02-29 4:57 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-02-10 22:57 [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Nathaniel Shead 2024-02-11 3:54 ` [PATCH 2/1] c++: Also support lambdas attached to TYPE_DECLs in modules Nathaniel Shead 2024-02-11 4:01 ` Andrew Pinski 2024-02-11 4:24 ` Nathaniel Shead 2024-02-12 13:12 ` Marek Polacek 2024-02-14 2:44 ` Jason Merrill 2024-02-14 0:52 ` [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] Jason Merrill 2024-02-16 12:21 ` [PATCH v2] c++/modules: Support lambdas attached to more places " Nathaniel Shead 2024-02-27 16:59 ` Patrick Palka 2024-02-28 4:12 ` [PATCH v3] " Nathaniel Shead 2024-02-28 17:34 ` Jason Merrill 2024-02-29 4:57 ` Nathaniel Shead
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).