From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x735.google.com (mail-qk1-x735.google.com [IPv6:2607:f8b0:4864:20::735]) by sourceware.org (Postfix) with ESMTPS id D84CF3858280 for ; Mon, 10 Oct 2022 21:01:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org D84CF3858280 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-qk1-x735.google.com with SMTP id m6so2296165qkm.4 for ; Mon, 10 Oct 2022 14:01:19 -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=w2PLsovUADgoyYK0bYTgjEmD8syj/keDuNBzOLnAH4Y=; b=DREYDEvdiUGf/IM5y85V5GJynxBS0nGEQpTAO/dNGK0pAORgMyuiRo/+KKl5yNWNNX /JqtXCpa+ZvP8UBLwBa5ZXvXz3ibYPpGZycKC3+Dva6INgBhf9LuR63Jhop592h13euW rRuaKpomUcUoNS6s42co2od4bf32qC/mndV+cw+caEoktBDODGcKjVepKU55gG1f7af9 srw5jI6THkEdXQXnnyX2jFdDKYXGDUkrBf/XumBqa2LFVoPuNVcCrunlpEYosiNm5LMv K1Q4ICwiPjmYV32ChISp5yZzEdDwDz6uOJuy+6x08v9ArcUmKlHT8QjnSkqtja736iYl JpSw== 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=w2PLsovUADgoyYK0bYTgjEmD8syj/keDuNBzOLnAH4Y=; b=rAHDUWFR7dCc0TH8QXjGiOTjPMv8NFuUwW2a6+rZjqfBt6m5T6lvI/7qhtNpQIUWOP 7kQO5dBoJ2ONLVAm8GsxyBK0JgqhvBxOjONzWeINeLHAczJVDwBvAmSfZRQdpG9a9VEp y+p/MaOUyaYRvSU4r1fvdjxSplVx317q4b1BDObgImGyv5aW0HU72j+FqKGXePf/3/H6 4UqYOtQIt2lld+SI7d92yXfodM2PBQplT4/vnASmPptCuC1DZDE5o4WpIgwijLhRjTGL OKl+LJGq3Nk78kR7y798wdr/ikD5bg6RaVq3443TKl3JR+F0tpighEPxVjrZph7nnKO9 21tg== X-Gm-Message-State: ACrzQf34j6sERcUSStY7R37dcwCs0jn5nBpCRW5vxqT7cF4Nms7Z9ATQ OVbYPrSzx0Yp0PyYKs+XMw8= X-Google-Smtp-Source: AMsMyM7i0W0+/UWRtkO2WztPYBGmdXE2CztBZmCCQA5mMRRc94pvNJSKCnVURA+/C7xI1NOl7yFqMg== X-Received: by 2002:a05:620a:1448:b0:6ec:4576:d0b9 with SMTP id i8-20020a05620a144800b006ec4576d0b9mr7981693qkl.265.1665435677752; Mon, 10 Oct 2022 14:01:17 -0700 (PDT) Received: from ?IPV6:2601:19c:527f:bfd0::5? ([2601:19c:527f:bfd0::5]) by smtp.googlemail.com with ESMTPSA id l20-20020a05620a28d400b006ee79bb1f8asm347964qkp.68.2022.10.10.14.01.16 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 Oct 2022 14:01:17 -0700 (PDT) Sender: Nathan Sidwell Message-ID: Date: Mon, 10 Oct 2022 17:01:16 -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 , gcc-patches@gcc.gnu.org Cc: jason@redhat.com References: <20221004173631.2958133-1-ppalka@redhat.com> Content-Language: en-US From: Nathan Sidwell In-Reply-To: <20221004173631.2958133-1-ppalka@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3040.0 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/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. > > 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? > > Does one or the other approach look like the correct fix for this PR? > > PR c++/99377 > > gcc/cp/ChangeLog: > > * decl2.cc (note_vague_linkage_fn): Relax processing_template_decl > test to value_dependent_expression_p. > * module.cc (lazy_load_binding): Clear processing_template_decl. > > gcc/testsuite/ChangeLog: > > * g++.dg/modules/pr99377-2_a.C: New test. > * g++.dg/modules/pr99377-2_b.C: New test. > --- > gcc/cp/decl2.cc | 2 +- > gcc/cp/module.cc | 2 ++ > gcc/testsuite/g++.dg/modules/pr99377-2_a.C | 5 +++++ > gcc/testsuite/g++.dg/modules/pr99377-2_b.C | 6 ++++++ > 4 files changed, 14 insertions(+), 1 deletion(-) > 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/decl2.cc b/gcc/cp/decl2.cc > index 9f18466192f..5af4d17ee3b 100644 > --- a/gcc/cp/decl2.cc > +++ b/gcc/cp/decl2.cc > @@ -876,7 +876,7 @@ check_classfn (tree ctype, tree function, tree template_parms) > void > note_vague_linkage_fn (tree decl) > { > - if (processing_template_decl) > + if (value_dependent_expression_p (decl)) > return; > > DECL_DEFER_OUTPUT (decl) = 1; > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc > index 500ac06563a..79cbb346ffa 100644 > --- a/gcc/cp/module.cc > +++ b/gcc/cp/module.cc > @@ -19074,6 +19074,8 @@ lazy_load_binding (unsigned mod, tree ns, tree id, binding_slot *mslot) > > timevar_start (TV_MODULE_IMPORT); > > + 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++; > 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