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.129.124]) by sourceware.org (Postfix) with ESMTPS id AAD6B38560A7 for ; Mon, 6 Jun 2022 18:27:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org AAD6B38560A7 Received: from mail-qk1-f199.google.com (mail-qk1-f199.google.com [209.85.222.199]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-483-DfzdfK5lNz-N6X9NR2A-4g-1; Mon, 06 Jun 2022 14:27:40 -0400 X-MC-Unique: DfzdfK5lNz-N6X9NR2A-4g-1 Received: by mail-qk1-f199.google.com with SMTP id j12-20020ae9c20c000000b0069e8ac6b244so12257964qkg.1 for ; Mon, 06 Jun 2022 11:27:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:date:to:cc:subject:message-id:mime-version; bh=W0ZZW81JcES71+puZBH54XvZMcxb1cBd82Glj/wPo9Q=; b=VVYzey/aF1iNqiTcQguZ87SS0o60pfJxDXLQUKCCtJrA8hZS/ZriZpmh/9FGglc+1b evdggdilS1S4vkwHvs4MAxKZivmo+fwpUktbYxYZk6RxAiK8EyLAm0Aw6XLsJlrzhfld KGR1/mULULMpkZ1MuC+L/joDVGRAyRcbTXKxf3KBOvPi7Vl+0w5Gn0H21YVoO96AGRyK 28IUa9yH/1vgTKicNA36mkNMySYFXJmZOkpjmtibQ6eS26dBCHDu3/zP67SFx8PkzU83 a2lmsvZ+/0GhCy2Lu+yCu6WGiFlysRzWnMayd1FKerCjU71t/WXUgjqp4I0D+OfO0dyk nM8w== X-Gm-Message-State: AOAM532xL6W3D29lppgycqdyBKtMZBVDCIQFT30zfqITkiXFNUi4Lyge TVe9hOh2ZINMACXdYxTWYpNkhb2wHls5SsWE7XtjCDRklXmRjABhLjDfCzsdjnrCru5owgrfy1N yQLuCHDqvC1l0d3MOXw== X-Received: by 2002:a05:620a:2907:b0:6a5:723a:e291 with SMTP id m7-20020a05620a290700b006a5723ae291mr16591248qkp.36.1654540059061; Mon, 06 Jun 2022 11:27:39 -0700 (PDT) X-Google-Smtp-Source: ABdhPJzCoA7z+OxvCrUaTTo8EkBkMtVOy6znen11E8QruJxSarhFyqcrN5AgvfA1j8qH7rbvwEG0rw== X-Received: by 2002:a05:620a:2907:b0:6a5:723a:e291 with SMTP id m7-20020a05620a290700b006a5723ae291mr16591228qkp.36.1654540058671; Mon, 06 Jun 2022 11:27:38 -0700 (PDT) Received: from [192.168.1.130] (ool-457670bb.dyn.optonline.net. [69.118.112.187]) by smtp.gmail.com with ESMTPSA id 66-20020aed3148000000b003026a08257fsm11093650qtg.21.2022.06.06.11.27.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 06 Jun 2022 11:27:38 -0700 (PDT) From: Patrick Palka X-Google-Original-From: Patrick Palka Date: Mon, 6 Jun 2022 14:27:37 -0400 (EDT) To: Jason Merrill cc: Patrick Palka , gcc-patches@gcc.gnu.org Subject: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] Message-ID: <95c2097c-5466-fcae-fdcd-fafdf1ab9431@idea> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-14.7 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, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 06 Jun 2022 18:27:44 -0000 On Thu, 7 Oct 2021, Jason Merrill wrote: > On 10/7/21 11:17, Patrick Palka wrote: > > On Wed, 6 Oct 2021, Jason Merrill wrote: > > > > > On 10/6/21 15:52, Patrick Palka wrote: > > > > On Wed, 6 Oct 2021, Patrick Palka wrote: > > > > > > > > > On Tue, 5 Oct 2021, Jason Merrill wrote: > > > > > > > > > > > On 10/5/21 15:17, Patrick Palka wrote: > > > > > > > On Mon, 4 Oct 2021, Patrick Palka wrote: > > > > > > > > > > > > > > > When passing a function template as the argument to a function > > > > > > > > NTTP > > > > > > > > inside a template, we resolve it to the right specialization > > > > > > > > ahead > > > > > > > > of > > > > > > > > time via resolve_address_of_overloaded_function, though the call > > > > > > > > to > > > > > > > > mark_used within defers odr-using it until instantiation time > > > > > > > > (as > > > > > > > > usual). > > > > > > > > But at instantiation time we end up never calling mark_used on > > > > > > > > the > > > > > > > > specialization. > > > > > > > > > > > > > > > > This patch fixes this by adding a call to mark_used in > > > > > > > > convert_nontype_argument_function. > > > > > > > > > > > > > > > > PR c++/53164 > > > > > > > > > > > > > > > > gcc/cp/ChangeLog: > > > > > > > > > > > > > > > > * pt.c (convert_nontype_argument_function): Call mark_used. > > > > > > > > > > > > > > > > gcc/testsuite/ChangeLog: > > > > > > > > > > > > > > > > * g++.dg/template/non-dependent16.C: New test. > > > > > > > > --- > > > > > > > > gcc/cp/pt.c | 3 +++ > > > > > > > > gcc/testsuite/g++.dg/template/non-dependent16.C | 16 > > > > > > > > ++++++++++++++++ > > > > > > > > 2 files changed, 19 insertions(+) > > > > > > > > create mode 100644 > > > > > > > > gcc/testsuite/g++.dg/template/non-dependent16.C > > > > > > > > > > > > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > > > > > > > index f950f4a21b7..5e819c9598c 100644 > > > > > > > > --- a/gcc/cp/pt.c > > > > > > > > +++ b/gcc/cp/pt.c > > > > > > > > @@ -6668,6 +6668,9 @@ convert_nontype_argument_function (tree > > > > > > > > type, > > > > > > > > tree > > > > > > > > expr, > > > > > > > > return NULL_TREE; > > > > > > > > } > > > > > > > > + if (!mark_used (fn_no_ptr, complain) && !(complain & > > > > > > > > tf_error)) > > > > > > > > + return NULL_TREE; > > > > > > > > + > > > > > > > > linkage = decl_linkage (fn_no_ptr); > > > > > > > > if (cxx_dialect >= cxx11 ? linkage == lk_none : linkage != > > > > > > > > lk_external) > > > > > > > > { > > > > > > > > diff --git a/gcc/testsuite/g++.dg/template/non-dependent16.C > > > > > > > > b/gcc/testsuite/g++.dg/template/non-dependent16.C > > > > > > > > new file mode 100644 > > > > > > > > index 00000000000..b7dca8f6752 > > > > > > > > --- /dev/null > > > > > > > > +++ b/gcc/testsuite/g++.dg/template/non-dependent16.C > > > > > > > > @@ -0,0 +1,16 @@ > > > > > > > > +// PR c++/53164 > > > > > > > > + > > > > > > > > +template > > > > > > > > +void f(T) { > > > > > > > > + T::fail; // { dg-error "not a member" } > > > > > > > > +} > > > > > > > > + > > > > > > > > +template > > > > > > > > +struct A { }; > > > > > > > > + > > > > > > > > +template > > > > > > > > +void g() { > > > > > > > > + A a; > > > > > > > > +} > > > > > > > > > > > > > > I should mention that the original testcase in the PR was slightly > > > > > > > different than this one in that it also performed a call to the > > > > > > > NTTP, > > > > > > > e.g. > > > > > > > > > > > > > > template > > > > > > > struct A { > > > > > > > static void h() { > > > > > > > p(0); > > > > > > > } > > > > > > > }; > > > > > > > > > > > > > > template > > > > > > > void g() { > > > > > > > A::h(); > > > > > > > } > > > > > > > > > > > > > > templated void g<0>(); > > > > > > > > > > > > > > and not even the call was enough to odr-use f, apparently because > > > > > > > the > > > > > > > CALL_EXPR case of tsubst_expr calls mark_used on the callee only > > > > > > > when > > > > > > > it's a FUNCTION_DECL, but in this case after substitution it's an > > > > > > > ADDR_EXPR of a FUNCTION_DECL. Fixing this by looking through the > > > > > > > ADDR_EXPR > > > > > > > worked, but IIUC the call isn't necessary for f to be odr-used, > > > > > > > simply > > > > > > > using f as a template argument should be sufficient, so it seems > > > > > > > the > > > > > > > above is better fix. > > > > > > > > > > > > I agree that pedantically the use happens when substituting into the > > > > > > use > > > > > > of > > > > > > A, but convert_nontype_argument_function seems like a weird place > > > > > > to > > > > > > implement that; it's only called again during instantiation of A, > > > > > > when we > > > > > > instantiate the injected-class-name. If A isn't instantiated, > > > > > > e.g. > > > > > > if 'a' > > > > > > is a pointer to A, we again don't instantiate f. > > > > > > > > > > I see, makes sense.. I'm not sure where else we can mark the use, > > > > > then. > > > > > Since we resolve the OVERLOAD f to the FUNCTION_DECL f ahead of > > > > > time (during which mark_used doesn't actually instantiate f > > > > > because > > > > > we're inside a template), at instantiation time the type A is > > > > > already > > > > > non-dependent so tsubst_aggr_type avoids doing the work that would end > > > > > up calling convert_nontype_argument_function. > > > > > > > > > > > > > > > > > I see that clang doesn't reject your testcase, either, but MSVC and > > > > > > icc > > > > > > do > > > > > > (even with 'a' a pointer): https://godbolt.org/z/MGE6TcMch > > > > > > > > > > FWIW although Clang doesn't reject 'A a;', it does reject > > > > > 'using type = A;' weirdly enough: https://godbolt.org/z/T9qEn6bWW > > > > > > > > > > > > > > > Shall we just go with the other more specific approach, that makes > > > > > sure > > > > > the CALL_EXPR case of tsubst_expr calls mark_used when the callee is > > > > > an > > > > > ADDR_EXPR? Something like (bootstrapped and regtested): > > > > > > > > Err, this approach is wrong because by stripping the ADDR_EXPR here we > > > > end up checking access of the unwrapped FUNCTION_DECL again after > > > > substituting into the call. So we incorrectly reject e.g. > > > > > > > > template > > > > void g() { > > > > P(); // error: ‘static void A::h()’ is private within this context > > > > } > > > > > > > > struct A { > > > > void f() { > > > > g(); > > > > } > > > > private: > > > > static void h(); > > > > }; > > > > > > > > since A::h isn't accessible from g. > > > > > > I guess you could call mark_used directly instead of stripping the > > > ADDR_EXPR. > > > > That seems to work nicely, how does the below look? Bootstrapped and > > regtested on x86_64-pc-linux-gnu. > > > > > > > > Or for the general problem, perhaps we could mark the TEMPLATE_INFO or > > > TI_ARGS > > > to indicate that we still need to mark_used the arguments when we > > > encounter > > > A again during instantiation? > > > > That sounds plausible, though I suppose it might not be worth it only to > > handle such a corner case.. > > Indeed. A lower-overhead possibility would be to remember, for a template, > decls that we wanted to mark_used but didn't because we were in a template. > But I wouldn't worry about it for now. > > > -- >8 -- > > > > Subject: [PATCH] c++: function NTTP argument considered unused [PR53164] > > > > Here at parse time the template argument f (an OVERLOAD) in A gets > > resolved ahead of time to the FUNCTION_DECL f, and we defer marking > > f as used until instantiation (of g) as usual. > > > > Later when instantiating g the type A (where f has already been resolved) > > is non-dependent, so tsubst_aggr_type avoids re-processing its template > > arguments, and we end up never actually marking f as used (which means > > we never instantiate it) even though A::h() calls it. > > > > This patch works around this problem by making us look through ADDR_EXPR > > when calling mark_used on the callee of a substituted CALL_EXPR. > > > > PR c++/53164 > > > > gcc/cp/ChangeLog: > > > > * pt.c (tsubst_copy_and_build) : Look through an > > ADDR_EXPR callee when calling mark_used. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/template/fn-ptr3.C: New test. > > --- > > gcc/cp/pt.c | 12 ++++++++---- > > gcc/testsuite/g++.dg/template/fn-ptr3.C | 20 ++++++++++++++++++++ > > 2 files changed, 28 insertions(+), 4 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3.C > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index 1e52aa757e1..cd10340ce12 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -20508,10 +20508,14 @@ tsubst_copy_and_build (tree t, > > } > > /* Remember that there was a reference to this entity. */ > > - if (function != NULL_TREE > > - && DECL_P (function) > > - && !mark_used (function, complain) && !(complain & tf_error)) > > - RETURN (error_mark_node); > > + if (function) > > + { > > + tree sub = function; > > + if (TREE_CODE (sub) == ADDR_EXPR) > > + sub = TREE_OPERAND (sub, 0); > > Let's add a comment about why this is needed. OK with that change. Thanks. I ended up neglecting to commit this patch for GCC 12, and turns out it also fixes the recently reported regression PR105848 (which is essentially another manifestation of PR53164 exposed by the non-dependent overload set pruning patch r12-6075), so I'm going to commit this now. I suppose it's also OK to backport to 12? -- >8 -- Subject: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] Here at parse time the template argument f (an OVERLOAD) in A gets resolved ahead of time to the FUNCTION_DECL f, and we defer marking f as used until instantiation (of g) as usual. Later when instantiating g the type A (where f has already been resolved) is non-dependent, so tsubst_aggr_type avoids re-processing its template arguments, and we end up never actually marking f as used (which means we never instantiate it) even though A::h() later calls it, leading to a link error. This patch works around this issue by looking through ADDR_EXPR when calling mark_used on the substituted callee of a CALL_EXPR. PR c++/53164 PR c++/105848 gcc/cp/ChangeLog: * pt.c (tsubst_copy_and_build) : Look through an ADDR_EXPR callee when calling mark_used. gcc/testsuite/ChangeLog: * g++.dg/template/fn-ptr3.C: New test. --- gcc/cp/pt.cc | 20 ++++++++++++++---- gcc/testsuite/g++.dg/template/fn-ptr3.C | 28 +++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index 59b94317e88..dcacba56a1c 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -20884,10 +20884,22 @@ tsubst_copy_and_build (tree t, } /* Remember that there was a reference to this entity. */ - if (function != NULL_TREE - && DECL_P (function) - && !mark_used (function, complain) && !(complain & tf_error)) - RETURN (error_mark_node); + if (function != NULL_TREE) + { + tree inner = function; + if (TREE_CODE (inner) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (inner, 0)) == FUNCTION_DECL) + /* We should already have called mark_used when taking the + address of this function, but do so again anyway to make + sure it's odr-used: at worst this is a no-op, but if we + obtained this FUNCTION_DECL as part of ahead-of-time overload + resolution then that call to mark_used wouldn't have marked it + odr-used yet (53164). */ + inner = TREE_OPERAND (inner, 0); + if (DECL_P (inner) + && !mark_used (inner, complain) && !(complain & tf_error)) + RETURN (error_mark_node); + } if (!maybe_fold_fn_template_args (function, complain)) return error_mark_node; diff --git a/gcc/testsuite/g++.dg/template/fn-ptr3.C b/gcc/testsuite/g++.dg/template/fn-ptr3.C new file mode 100644 index 00000000000..4c651f124f6 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/fn-ptr3.C @@ -0,0 +1,28 @@ +// PR c++/53164 +// PR c++/105848 +// { dg-do link } + +template +void f(T) { } + +template +struct A { + static void wrap() { + P(0); + } +}; + +template +void wrap() { + P(0); +} + +template +void g() { + A::wrap(); + wrap(); +} + +int main() { + g<0>(); +} -- 2.36.1.299.gab336e8f1c