From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id A68DE3857BA3 for ; Wed, 14 Feb 2024 00:52:05 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org A68DE3857BA3 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org A68DE3857BA3 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707871928; cv=none; b=ncHhA4ll4sRxAYcAzv6EgF3PucRWRVYa3orT0GsAeXsICt0va2msBZtrIx6/Nz2QXyHpq+/Rj2wUXrkg+QYCbZoocCM9R0eam+UYoxVnm+NZvJ2CHy7vp8WUD6MJsO8ocyKXW5//f6WXBxuGlbjXzQV+LMrx0HRncaAmROTIsPo= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1707871928; c=relaxed/simple; bh=2pkfPRVlEXUxkHjF0mklwAbsq2RS6/oFy7/QSbM3caE=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=nOyQlo8Y3iL5g7r0yCtdU1CzEA5ovQ/2N+2zm+zfh9Q8tRu67dQ9x68oLMXd2WqKunGpl+Kg+Ewpy3Ja698Yqq5UQkZUdsIX9B8mwazu4wf/6Ne51MhjvbQO2rufxh7IVglDXCf2BvtP8G4E2K5YCTjlJfgVO91P2/KKrMLZW+U= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1707871925; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=xMvvFSLErAv95T9Fk+IcZhcaic4Dk1mocClIgB2x4IY=; b=b31ZiTLXABc9v0UgFfnxPBjTO2hIQQM8ewsZYxnxg2nfmFzdfl5PPqxqG4pFQm3LUi3I06 Ilsf3qEmDUM7iZGTaowKEdq9KtSotdL6HT6nNnZ6LWEZ/WLx9nSUwUxBMQm1ZE8w1LIHH4 NLRBpqeTugs/eCIgpQzejBWAAFWlXAw= Received: from mail-qt1-f197.google.com (mail-qt1-f197.google.com [209.85.160.197]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-426-ZAMg90QdNrumZjugimfSkg-1; Tue, 13 Feb 2024 19:52:03 -0500 X-MC-Unique: ZAMg90QdNrumZjugimfSkg-1 Received: by mail-qt1-f197.google.com with SMTP id d75a77b69052e-42c6b58b448so6362941cf.0 for ; Tue, 13 Feb 2024 16:52:03 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1707871923; x=1708476723; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=xMvvFSLErAv95T9Fk+IcZhcaic4Dk1mocClIgB2x4IY=; b=WSQbAyZ9rgkiKL0ZoELTleS+qgRLNjEp56twhh4BbYG8lFleGf/Vzw2azBtCkOQOIV y4EduDZjF12aCcsepY7r/be2eByj0+T1u0muTNGx0/xKkJmEy1lYkgDMC0MV2mqz0r2/ rG6F9D2Q9fQo7P33YFbFCE7SfsaaVEN97Fck7WGauc8exvOTgw7VbL5j9SBbpuPou/+m K09o34HuTdcihHZqHGX1oSxDT9bFw99ow3VmXvVykubEF1HFBYCbmZ4l1ffTt4vlkFig iI3Q9TI93ICayyGNVT8sD8fr6d8DxVMMgHPTDh7Iy4QBRqlrPWjUZsVdy5xQuqTd7Qig K0JQ== X-Forwarded-Encrypted: i=1; AJvYcCXf939Kw0x6cWVxVyR69rOMDjq6le2aq2ijILOg3VHUS95nM6S96+Wpqk5l3fwlyDE4DqP4tth0uJjKAr9xCCoa78kO/aztdQ== X-Gm-Message-State: AOJu0Yyy156Ug98qgwrJJJdwOpLAvjGPAi9vmsXYrwTb7H/NTgi6++jR aDss8M1QPRvvmPEJ/4V47TtdjZVCfPy8yIwRPQm+zr63XlKHe/U92u/11V+U+QDkBUyRlMdCXmG rKygVsVjCgwM5weCvA6kxOZSRkhTDqdtXMlYrfOqYavr1Lt8w5hEpzzXeg+3SZKg= X-Received: by 2002:a05:622a:11d2:b0:42c:66b7:de02 with SMTP id n18-20020a05622a11d200b0042c66b7de02mr815363qtk.24.1707871922972; Tue, 13 Feb 2024 16:52:02 -0800 (PST) X-Google-Smtp-Source: AGHT+IH8stCVi6WceAYmfDl1n+P4LeMxUqcUlF1Ocw0z8nOa866YNufuoktQVAMdiDpR2VmWiyibYA== X-Received: by 2002:a05:622a:11d2:b0:42c:66b7:de02 with SMTP id n18-20020a05622a11d200b0042c66b7de02mr815355qtk.24.1707871922570; Tue, 13 Feb 2024 16:52:02 -0800 (PST) X-Forwarded-Encrypted: i=1; AJvYcCVnUnk/58+aD2CcpiNmRyu6d9v7OFcjReouCUE5wR+cA0mXfofZzbelnO+V2A8g78Ae2Ylk4INlxGex8tnLVEsXzM1Fh+Lu7A== Received: from [192.168.1.130] (130-44-146-16.s12558.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.146.16]) by smtp.gmail.com with ESMTPSA id x4-20020ac84d44000000b0042c4f46a994sm1576659qtv.75.2024.02.13.16.52.01 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 13 Feb 2024 16:52:01 -0800 (PST) Message-ID: Date: Tue, 13 Feb 2024 19:52:01 -0500 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] c++: Support lambdas attached to FIELD_DECLs in modules [PR111710] To: Nathaniel Shead , gcc-patches@gcc.gnu.org Cc: Nathan Sidwell References: <65c7ff51.170a0220.8dcc0.a404@mx.google.com> From: Jason Merrill In-Reply-To: <65c7ff51.170a0220.8dcc0.a404@mx.google.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,TXREP,T_SCC_BODY_TEXT_LINE,URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: 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 > --- > 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(); > +}