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 <nathanielosh...@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();
+}
