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 5C7473857BBC for ; Mon, 6 Jun 2022 19:26:56 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 5C7473857BBC 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-593-ik6YtdkbMT2GmoofAiuX1g-1; Mon, 06 Jun 2022 15:26:52 -0400 X-MC-Unique: ik6YtdkbMT2GmoofAiuX1g-1 Received: by mail-qk1-f199.google.com with SMTP id c8-20020a05620a268800b0069c0f1b3206so12335722qkp.18 for ; Mon, 06 Jun 2022 12:26:52 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=amTSca84T53WBq0NATGpNS5ZczA5VrCLo7yXtStckO0=; b=gxN7focx3O/RaWPQVZkyD13s37w/JXMxYmdNUTcfRQMiilg5dabOsvTb+9J//7b3iM OyUrOKGV7TYks5Gn9JzpX7Gs+OGqwe1nZ2cKFOet9v8UB67tabj3pO/TuYhRkoSzJuiz KT/UDnNp+IZK0vR7L6PHQUWWqxgSI5plWylI0OHd4mkwc7ASIhfk0t5jVHHVa5bLUlZv RUIPYOzUDDwkO0wS2QLmhJ8skINu9n62/DaxycT2hdXauQYkAbeOqaH5Vvbt/qiN1aYG iInY/vVsBoiwJDYykkaLETShCTDSDre+Q5P2APjpeAJBAf0ccq7Dn1ED62fiMKis394Z 9cOA== X-Gm-Message-State: AOAM5303FiE+YHyC2bK46Isc6nkT2u1ju7gGRLeHlh1Xfd3W91MKdSpe K6d4f70uQSwoqkdGcR+JGHMuLS4SzHhI96dpivGFhyyDd3wocbzrA2sqJExIcO77x9nxw4cD2Gq HfgjsjMcOd7crbPUNXw== X-Received: by 2002:a05:620a:2592:b0:6a6:3cd4:c2ee with SMTP id x18-20020a05620a259200b006a63cd4c2eemr16520886qko.291.1654543611132; Mon, 06 Jun 2022 12:26:51 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxQ5LsOvxvT888mK52iSz2pYNJCqiacJeMF0p9qxRW2DWHvVgYRvW+JCZX8ciKRILqRq/EDlQ== X-Received: by 2002:a05:620a:2592:b0:6a6:3cd4:c2ee with SMTP id x18-20020a05620a259200b006a63cd4c2eemr16520851qko.291.1654543610605; Mon, 06 Jun 2022 12:26:50 -0700 (PDT) Received: from [192.168.1.100] (130-44-159-43.s15913.c3-0.arl-cbr1.sbo-arl.ma.cable.rcncustomer.com. [130.44.159.43]) by smtp.gmail.com with ESMTPSA id w13-20020a05620a424d00b006a69ee117b6sm8653326qko.97.2022.06.06.12.26.49 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Jun 2022 12:26:50 -0700 (PDT) Message-ID: <41cec505-46f4-5622-ab08-e693fa8b7d90@redhat.com> Date: Mon, 6 Jun 2022 15:26:49 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.10.0 Subject: Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] To: Patrick Palka Cc: gcc-patches@gcc.gnu.org References: <95c2097c-5466-fcae-fdcd-fafdf1ab9431@idea> From: Jason Merrill In-Reply-To: <95c2097c-5466-fcae-fdcd-fafdf1ab9431@idea> 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: 8bit X-Spam-Status: No, score=-14.4 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, NICE_REPLY_A, 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 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 19:26:58 -0000 On 6/6/22 14:27, Patrick Palka wrote: > 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? What if the use is not as a call, but say assigning the address to a global pointer? I wonder about calling mark_used when substituting the TEMPLATE_PARM_INDEX, probably with tf_none or tf_error to avoid redundant deprecated warnings. > -- >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>(); > +}