From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-x836.google.com (mail-qt1-x836.google.com [IPv6:2607:f8b0:4864:20::836]) by sourceware.org (Postfix) with ESMTPS id 67A563858C00 for ; Tue, 11 Oct 2022 17:12:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 67A563858C00 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-x836.google.com with SMTP id a24so1108400qto.10 for ; Tue, 11 Oct 2022 10:12:41 -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:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :sender:from:to:cc:subject:date:message-id:reply-to; bh=qvVQU5lB0iCl1J04iCHPgVpl5e03fcoaTVx/WOX6utc=; b=JFssPazmLH6eeOWvv62hcQp2re6W4mzo9b8PrYvUT/7h5ACwOmQj8JBluTkjQ973ge k0n27AZaeLSHQGGaxMp3JOG4QXOAbE76cneGqSsr0whC2tLz7fiblbNqq16J73z7Ls5C kR5+jZ71OLAmYfmiqnm5wIGPDTv3ULrfsd5kL8Yq5lXzKAR7t5JlhAjyf8WFhMTBPeR6 R49IHtINfuY8BhB5lmev80MAeNNrZ5doZpNo7ICWXpfRodNb7WeBvMsKb/pLXpqmIQSe KR8S67FLaMFLCSV7lXfIyOieNjo8TzwFEglV5JWt/Efg+Fc34evleHFesszIUHbvk8aL I0Tg== 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:content-language :references:cc:to:subject:user-agent:mime-version:date:message-id :sender:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=qvVQU5lB0iCl1J04iCHPgVpl5e03fcoaTVx/WOX6utc=; b=sLhjrcmO31bw7pRxx1GMnrAgbF4MobePK67GBjsIAId2t80uWw4cIPpizWTsy3CNxH V8Ou2Hx4VrZsyvESuuLKYn8trarJ0nbzPavxcyxPhvOvaP5JgtVIGIuFwPLgdAdBrDQm PfJJ2KWIblohwN3ScobCUc9X0qE29KrRmyWEGcVdp7As4dTcfDY8Tkxr8Xfd3arRGspg IYM9efstL4Ny3xEeixjab8YaO4KUrAqEpXcDDhz/1Bv65n8uXw/ceQ7eaOWowhSSzAUE lp7sRkPgM+2m9G/NIWGyDzbe7bz0Hv4G1oV208+TrqBpozxydfXRzwBTyQuivJ7w+qXM LUqQ== X-Gm-Message-State: ACrzQf3QWxqAVGeWQMfcnB1vnELTIrToQPCkGSVdxApW8QruxJg1277W a013wW69XFVSTU9xMInbves= X-Google-Smtp-Source: AMsMyM5fjR/a1mltsdVNlq9u2ViAsOoLLV12txyLmGnBpMxotgsO5WgzuppgdP6WKUNLNDNKyyLTEQ== X-Received: by 2002:a05:622a:307:b0:394:dddf:d28c with SMTP id q7-20020a05622a030700b00394dddfd28cmr19701322qtw.121.1665508360563; Tue, 11 Oct 2022 10:12:40 -0700 (PDT) Received: from ?IPV6:2620:10d:c0a8:11c9::1442? ([2620:10d:c091:480::bdde]) by smtp.googlemail.com with ESMTPSA id bv12-20020a05622a0a0c00b00393c2067ca6sm11004653qtb.16.2022.10.11.10.12.39 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 11 Oct 2022 10:12:39 -0700 (PDT) Sender: Nathan Sidwell Message-ID: <13ac0eb2-6527-057f-4b4a-8514542a9350@acm.org> Date: Tue, 11 Oct 2022 13:12:38 -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: lazy loading from within template [PR99377] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org, jason@redhat.com References: <20221004173631.2958133-1-ppalka@redhat.com> Content-Language: en-US From: Nathan Sidwell In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3040.6 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/11/22 10:58, Patrick Palka wrote: > On Mon, 10 Oct 2022, Nathan Sidwell wrote: > >> On 10/4/22 13:36, Patrick Palka wrote: >>> Here when lazily loading the binding for f at parse time from the >>> template g, processing_template_decl is set and thus the call to >>> note_vague_linkage_fn from module_state::read_cluster has no effect, >>> and we never push f onto deferred_fns and end up never emitting its >>> definition. >>> >>> ISTM the behavior of the lazy loading machinery shouldn't be sensitive >>> to whether we're inside a template, and therefore we should probably be >>> clearing processing_template_decl somewhere e.g in lazy_load_binding. >>> This is sufficient to fix the testcase. >> >> yeah, I remember hitting issues with this, but thought I'd got rid of the need >> to override processing_template_decl. Do you also need to override it in >> lazy_load_pendings though? that's a lazy loader and my suspicion is it might >> be susceptible to the same issues. > > Hmm yeah, looks like we need to override it in lazy_load_pendings too: > I ran the testsuite with gcc_assert (!processing_template_decl) added to > module_state::read_cluster and if we don't also override it in > lazy_load_binding then the assert triggers for pr99425-2_b.X. > >> >>> >>> But it also seems the processing_template_decl test in >>> note_vague_linkage_fn, added by r8-7539-g977bc3ee11383e for PR84973, is >>> perhaps too strong: if the intent is to avoid deferring output for >>> uninstantiated templates, we should make sure that DECL in question is >>> an uninstantiated template by checking e.g. value_dependent_expression_p. >>> This too is sufficient to fix the testcase (since f isn't a template) >>> and survives bootstrap and regtest. >> >> I think this is an orthogonal issue -- can we remove it from this patch? > > Done. > > -- >8 -- > > Subject: [PATCH] c++ modules: lazy loading from within template [PR99377] > > Here when lazily loading the binding for f at parse time from the > template g, processing_template_decl is set and thus the call to > note_vague_linkage_fn from module_state::read_cluster has no effect, > and we never push f onto deferred_fns and end up never emitting its > definition. > > ISTM the behavior of the lazy loading machinery shouldn't be sensitive > to whether we're inside a template, and therefore we should be clearing > processing_template_decl in the entrypoints lazy_load_binding and > lazy_load_pendings. > > PR c++/99377 > > gcc/cp/ChangeLog: > > * module.cc (lazy_load_binding): Clear processing_template_decl. > (lazy_load_pendings): Likewise. ok, thanks > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/pr99377-2_a.C: New test. > * g++.dg/modules/pr99377-2_b.C: New test. > --- > gcc/cp/module.cc | 8 ++++++++ > gcc/testsuite/g++.dg/modules/pr99377-2_a.C | 5 +++++ > gcc/testsuite/g++.dg/modules/pr99377-2_b.C | 6 ++++++ > 3 files changed, 19 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_a.C > create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_b.C > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 94fbee85225..7c48602136c 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -19081,6 +19081,10 @@ lazy_load_binding (unsigned mod, tree ns, tree id, binding_slot *mslot) > > timevar_start (TV_MODULE_IMPORT); > > + /* Make sure lazy loading from a template context behaves as if > + from a non-template context. */ > + processing_template_decl_sentinel ptds; > + > /* Stop GC happening, even in outermost loads (because our caller > could well be building up a lookup set). */ > function_depth++; > @@ -19129,6 +19133,10 @@ lazy_load_binding (unsigned mod, tree ns, tree id, binding_slot *mslot) > void > lazy_load_pendings (tree decl) > { > + /* Make sure lazy loading from a template context behaves as if > + from a non-template context. */ > + processing_template_decl_sentinel ptds; > + > tree key_decl; > pending_key key; > key.ns = find_pending_key (decl, &key_decl); > diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_a.C b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C > new file mode 100644 > index 00000000000..26e2bccbbbe > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C > @@ -0,0 +1,5 @@ > +// PR c++/99377 > +// { dg-additional-options -fmodules-ts } > +// { dg-module-cmi pr99377 } > +export module pr99377; > +export inline void f() { } > diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_b.C b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C > new file mode 100644 > index 00000000000..69571952c8a > --- /dev/null > +++ b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C > @@ -0,0 +1,6 @@ > +// PR c++/99377 > +// { dg-additional-options -fmodules-ts } > +// { dg-do link } > +import pr99377; > +template void g() { f(); } > +int main() { g(); } -- Nathan Sidwell