From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x82e.google.com (mail-qt1-x82e.google.com [IPv6:2607:f8b0:4864:20::82e]) by sourceware.org (Postfix) with ESMTPS id 0DA9B3858C2C for ; Sun, 16 Oct 2022 00:14:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 0DA9B3858C2C Authentication-Results: sourceware.org; dmarc=fail (p=none dis=none) header.from=acm.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-qt1-x82e.google.com with SMTP id h24so5537959qta.7 for ; Sat, 15 Oct 2022 17:14:20 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date:message-id:reply-to; bh=IEDzyKjtbwDLqWclEoyAOwgyAUlsn8TGGAZRVcBdddQ=; b=ZwM1oSIh/oYIKfRrW9fp+gcqpEF3fl6ENGJAFNbpoEK1cgnK9zQj5O2MbtTOno4ulA huPyO9GZj8OmAqrVJcwzXEQJHnVgJj38FbLb0370atAwXYARRQs6x2ARbRmapRoese7M ZeTcXLWk9yTb444s0VUsNDpHaVFL9DQK5G22+duPO9Tx64t4B/nC41dJ+PuhRqQJZmgd oTva9NnVrcvIuzvWmcsQ400ZZnnwfC9XEL2ErR40/9CGQztOlC1XLEy4yfUi8rXbhod3 sWv3GaLnlxUP+hLZZkCTcTXXJ0OsckHCRDkZCe53FKuMgrdjvZDM9IH1jP4j4JTmnx0U +TOQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IEDzyKjtbwDLqWclEoyAOwgyAUlsn8TGGAZRVcBdddQ=; b=T7r3IA+mju2T7t+/sWvy2WMbhWACm6/TAXtpYitiuy7ySPrzMrbdsJHsZYNPDmRzof RDwy8nalNCuHYNzYcfPEMfKPSruIyQzkArQQ+HLjrCzzJ1yJJsQ5CMug0YqVyeB3Ul5J 1uyGQ0ONIYkdVdAqW2chqt/HGRtkWh3w2jWsVviNq0OEBfNRJhNOpBT1KkJo0xgvkCE6 lEaUl6lt92zlpAOEXsrRDntpICJpE/rHaCEVqul9Ofb9IRMIXRe0XlfJQ/h32PbOhSlu F8JwTsyaz+rsYegH/TFAFGjFayBm/JU/q6CUr3QY2VIeTLAEC82sRGSNQRKcQX8AQYSP 637A== X-Gm-Message-State: ACrzQf1emEwu0+39VahJo+R/Iu06A0Ofs4+77CW9ZXTaqo7xTmXARrK3 r5Yi3Z8vy2rkZYcPjxXdxik= X-Google-Smtp-Source: AMsMyM6LK05xdGZhzb2r++dEeRgEIoIYN3Ai4hJRSPx4dT4IOyRtGwybWfn7eEWnQfAfLusrvdkdlw== X-Received: by 2002:a05:622a:312:b0:39c:dc09:49e3 with SMTP id q18-20020a05622a031200b0039cdc0949e3mr3657559qtw.4.1665879258825; Sat, 15 Oct 2022 17:14:18 -0700 (PDT) Received: from ?IPV6:2601:19c:527f:bfd0::5? ([2601:19c:527f:bfd0::5]) by smtp.googlemail.com with ESMTPSA id fw11-20020a05622a4a8b00b0039c72bb51f3sm4818257qtb.86.2022.10.15.17.14.18 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 15 Oct 2022 17:14:18 -0700 (PDT) Sender: Nathan Sidwell Message-ID: Date: Sat, 15 Oct 2022 20:14:17 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH] c++ modules: streaming constexpr_fundef [PR101449] Content-Language: en-US To: Patrick Palka , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20221014170018.892575-1-ppalka@redhat.com> From: Nathan Sidwell In-Reply-To: <20221014170018.892575-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3040.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_EF,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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 10/14/22 13:00, Patrick Palka wrote: > IIUC we currently avoid streaming the RESULT_DECL and PARM_DECLs > of a constexpr_fundef entry under the assumption that they're just > copies of the DECL_RESULT and DECL_ARGUMENTS of the FUNCTION_DECL. > Thus we can just make new copies of DECL_RESULT and DECL_ARGUMENTs > on stream in rather than separately streaming them. > > Unfortunately this assumption isn't true generally: the FUNCTION_DECL > contains genericized trees, whereas the constexpr_fundef entry contains > pre-GENERIC trees. So in particular DECL_RESULT and DECL_ARGUMENTs > may have been turned into invisiref parms which we don't handle during > during constexpr evaluation and so we ICE in the below testcase. > > This patch fixes this by faithfully streaming the RESULT_DECL and > PARM_DECLs of a constexpr_fundef entry. Hm, the reason for the complexity was that I wanted to recreate the tree graph where the fndecl came from one TU and the defn came from another one -- we need the definition to refer to argument decls from the already-read decl. However, it seems that for constexpr fns here, that is not needed, resulting in a significant simplification. ok. nathan > > PR c++/101449 > > gcc/cp/ChangeLog: > > * module.cc (trees_out::write_function_def): Stream the > parms and result of the constexpr_fundef entry. > (trees_in::read_function_def): Likewise. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/cexpr-3_a.C: New test. > * g++.dg/modules/cexpr-3_b.C: New test. > --- > gcc/cp/module.cc | 59 ++++-------------------- > gcc/testsuite/g++.dg/modules/cexpr-3_a.C | 14 ++++++ > gcc/testsuite/g++.dg/modules/cexpr-3_b.C | 7 +++ > 3 files changed, 29 insertions(+), 51 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-3_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-3_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 7ffeefa7c1f..999ff3faafc 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -11553,34 +11553,13 @@ trees_out::write_function_def (tree decl) > tree_node (DECL_FRIEND_CONTEXT (decl)); > > constexpr_fundef *cexpr = retrieve_constexpr_fundef (decl); > - int tag = 0; > - if (cexpr) > - { > - if (cexpr->result == error_mark_node) > - /* We'll stream the RESULT_DECL naturally during the > - serialization. We never need to fish it back again, so > - that's ok. */ > - tag = 0; > - else > - tag = insert (cexpr->result); > - } > + > if (streaming_p ()) > + u (cexpr != nullptr); > + if (cexpr) > { > - i (tag); > - if (tag) > - dump (dumper::TREE) > - && dump ("Constexpr:%d result %N", tag, cexpr->result); > - } > - if (tag) > - { > - unsigned ix = 0; > - for (tree parm = cexpr->parms; parm; parm = DECL_CHAIN (parm), ix++) > - { > - tag = insert (parm); > - if (streaming_p ()) > - dump (dumper::TREE) > - && dump ("Constexpr:%d parm:%u %N", tag, ix, parm); > - } > + chained_decls (cexpr->parms); > + tree_node (cexpr->result); > tree_node (cexpr->body); > } > > @@ -11613,32 +11592,10 @@ trees_in::read_function_def (tree decl, tree maybe_template) > tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl)); > bool installing = maybe_dup && !DECL_SAVED_TREE (decl); > > - if (int wtag = i ()) > + if (u ()) > { > - int tag = 1; > - cexpr.result = error_mark_node; > - > - cexpr.result = copy_decl (result); > - tag = insert (cexpr.result); > - > - if (wtag != tag) > - set_overrun (); > - dump (dumper::TREE) > - && dump ("Constexpr:%d result %N", tag, cexpr.result); > - > - cexpr.parms = NULL_TREE; > - tree *chain = &cexpr.parms; > - unsigned ix = 0; > - for (tree parm = DECL_ARGUMENTS (maybe_dup ? maybe_dup : decl); > - parm; parm = DECL_CHAIN (parm), ix++) > - { > - tree p = copy_decl (parm); > - tag = insert (p); > - dump (dumper::TREE) > - && dump ("Constexpr:%d parm:%u %N", tag, ix, p); > - *chain = p; > - chain = &DECL_CHAIN (p); > - } > + cexpr.parms = chained_decls (); > + cexpr.result = tree_node (); > cexpr.body = tree_node (); > cexpr.decl = decl; > } > diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_a.C b/gcc/testsuite/g++.dg/modules/cexpr-3_a.C > new file mode 100644 > index 00000000000..be24bb43a7b > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/cexpr-3_a.C > @@ -0,0 +1,14 @@ > +// PR c++/101449 > +// { dg-additional-options -fmodules-ts } > +// { dg-module-cmi pr101449 } > + > +export module pr101449; > + > +struct X { > + bool b = true; > + constexpr X() { } > + constexpr X(const X&) { } > +}; > + > +export constexpr X f() { return {}; } > +export constexpr bool g(X x) { return x.b; } > diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_b.C b/gcc/testsuite/g++.dg/modules/cexpr-3_b.C > new file mode 100644 > index 00000000000..cbf3be4fcab > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/cexpr-3_b.C > @@ -0,0 +1,7 @@ > +// PR c++/101449 > +// { dg-additional-options -fmodules-ts } > + > +import pr101449; > + > +static_assert(f().b); > +static_assert(g(f())); -- Nathan Sidwell