* [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] @ 2022-06-06 18:27 Patrick Palka 2022-06-06 19:26 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2022-06-06 18:27 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches 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<class T> > > > > > > > > +void f(T) { > > > > > > > > + T::fail; // { dg-error "not a member" } > > > > > > > > +} > > > > > > > > + > > > > > > > > +template<void(int)> > > > > > > > > +struct A { }; > > > > > > > > + > > > > > > > > +template<int> > > > > > > > > +void g() { > > > > > > > > + A<f> 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<void p(int)> > > > > > > > struct A { > > > > > > > static void h() { > > > > > > > p(0); > > > > > > > } > > > > > > > }; > > > > > > > > > > > > > > template<int> > > > > > > > void g() { > > > > > > > A<f>::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<f>, but convert_nontype_argument_function seems like a weird place > > > > > > to > > > > > > implement that; it's only called again during instantiation of A<f>, > > > > > > when we > > > > > > instantiate the injected-class-name. If A<f> isn't instantiated, > > > > > > e.g. > > > > > > if 'a' > > > > > > is a pointer to A<f>, we again don't instantiate f<int>. > > > > > > > > > > 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<int> ahead of > > > > > time (during which mark_used doesn't actually instantiate f<int> > > > > > because > > > > > we're inside a template), at instantiation time the type A<f> 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<f> a;', it does reject > > > > > 'using type = A<f>;' 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 P()> > > > > void g() { > > > > P(); // error: ‘static void A::h()’ is private within this context > > > > } > > > > > > > > struct A { > > > > void f() { > > > > g<h>(); > > > > } > > > > 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<f> 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<f> gets > > resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking > > f<int> as used until instantiation (of g) as usual. > > > > Later when instantiating g the type A<f> (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<int> as used (which means > > we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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<f> gets resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking f<int> as used until instantiation (of g) as usual. Later when instantiating g the type A<f> (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<int> as used (which means we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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<class T> +void f(T) { } + +template<void (*P)(int)> +struct A { + static void wrap() { + P(0); + } +}; + +template<void (*P)(char)> +void wrap() { + P(0); +} + +template<int> +void g() { + A<f>::wrap(); + wrap<f>(); +} + +int main() { + g<0>(); +} -- 2.36.1.299.gab336e8f1c ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] 2022-06-06 18:27 [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] Patrick Palka @ 2022-06-06 19:26 ` Jason Merrill 2022-06-07 13:24 ` Patrick Palka 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2022-06-06 19:26 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches 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<class T> >>>>>>>>> +void f(T) { >>>>>>>>> + T::fail; // { dg-error "not a member" } >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> +template<void(int)> >>>>>>>>> +struct A { }; >>>>>>>>> + >>>>>>>>> +template<int> >>>>>>>>> +void g() { >>>>>>>>> + A<f> 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<void p(int)> >>>>>>>> struct A { >>>>>>>> static void h() { >>>>>>>> p(0); >>>>>>>> } >>>>>>>> }; >>>>>>>> >>>>>>>> template<int> >>>>>>>> void g() { >>>>>>>> A<f>::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<f>, but convert_nontype_argument_function seems like a weird place >>>>>>> to >>>>>>> implement that; it's only called again during instantiation of A<f>, >>>>>>> when we >>>>>>> instantiate the injected-class-name. If A<f> isn't instantiated, >>>>>>> e.g. >>>>>>> if 'a' >>>>>>> is a pointer to A<f>, we again don't instantiate f<int>. >>>>>> >>>>>> 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<int> ahead of >>>>>> time (during which mark_used doesn't actually instantiate f<int> >>>>>> because >>>>>> we're inside a template), at instantiation time the type A<f> 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<f> a;', it does reject >>>>>> 'using type = A<f>;' 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 P()> >>>>> void g() { >>>>> P(); // error: ‘static void A::h()’ is private within this context >>>>> } >>>>> >>>>> struct A { >>>>> void f() { >>>>> g<h>(); >>>>> } >>>>> 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<f> 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<f> gets >>> resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking >>> f<int> as used until instantiation (of g) as usual. >>> >>> Later when instantiating g the type A<f> (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<int> as used (which means >>> we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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<f> gets > resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking > f<int> as used until instantiation (of g) as usual. > > Later when instantiating g the type A<f> (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<int> as used > (which means we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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<class T> > +void f(T) { } > + > +template<void (*P)(int)> > +struct A { > + static void wrap() { > + P(0); > + } > +}; > + > +template<void (*P)(char)> > +void wrap() { > + P(0); > +} > + > +template<int> > +void g() { > + A<f>::wrap(); > + wrap<f>(); > +} > + > +int main() { > + g<0>(); > +} ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] 2022-06-06 19:26 ` Jason Merrill @ 2022-06-07 13:24 ` Patrick Palka 2022-06-07 18:05 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2022-06-07 13:24 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches On Mon, 6 Jun 2022, Jason Merrill wrote: > 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<class T> > > > > > > > > > > +void f(T) { > > > > > > > > > > + T::fail; // { dg-error "not a member" } > > > > > > > > > > +} > > > > > > > > > > + > > > > > > > > > > +template<void(int)> > > > > > > > > > > +struct A { }; > > > > > > > > > > + > > > > > > > > > > +template<int> > > > > > > > > > > +void g() { > > > > > > > > > > + A<f> 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<void p(int)> > > > > > > > > > struct A { > > > > > > > > > static void h() { > > > > > > > > > p(0); > > > > > > > > > } > > > > > > > > > }; > > > > > > > > > > > > > > > > > > template<int> > > > > > > > > > void g() { > > > > > > > > > A<f>::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<f>, but convert_nontype_argument_function seems like a weird > > > > > > > > place > > > > > > > > to > > > > > > > > implement that; it's only called again during instantiation of > > > > > > > > A<f>, > > > > > > > > when we > > > > > > > > instantiate the injected-class-name. If A<f> isn't > > > > > > > > instantiated, > > > > > > > > e.g. > > > > > > > > if 'a' > > > > > > > > is a pointer to A<f>, we again don't instantiate f<int>. > > > > > > > > > > > > > > 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<int> ahead > > > > > > > of > > > > > > > time (during which mark_used doesn't actually instantiate f<int> > > > > > > > because > > > > > > > we're inside a template), at instantiation time the type A<f> 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<f> a;', it does reject > > > > > > > 'using type = A<f>;' 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 P()> > > > > > > void g() { > > > > > > P(); // error: ‘static void A::h()’ is private within this > > > > > > context > > > > > > } > > > > > > > > > > > > struct A { > > > > > > void f() { > > > > > > g<h>(); > > > > > > } > > > > > > 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<f> 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<f> gets > > > > resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking > > > > f<int> as used until instantiation (of g) as usual. > > > > > > > > Later when instantiating g the type A<f> (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<int> as used (which > > > > means > > > > we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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. Ah, that seems to work very nicely -- it even handles the case where there the only use is as a template argument and there is no "use" inside the body of the function, because we always have to substitute the TEMPLATE_PARM_INDEX in the generic template arguments. Like so? Bootstrapped and tested on x86_64-pc-linux-gnu. I'm not sure if it's worth also checking !processing_template_decl && !DECL_ODR_USED before calling mark_used here, since otherwise the call ought to be certainly redundant. -- >8 -- Subject: [PATCH] c++: improve "NTTP argument considered unused" fix [PR53164] PR c++/53164 PR c++/105848 gcc/cp/ChangeLog: * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on an ADDR_EXPR argument. (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. gcc/testsuite/ChangeLog: * g++.dg/template/fn-ptr3a.C: New test. Co-authored-by: Jason Merrill <jason@redhat.com> --- gcc/cp/pt.cc | 40 +++++++++++++----------- gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 +++++++++++++++ 2 files changed, 46 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index dcacba56a1c..5863d83b4fd 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -15858,9 +15858,23 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) } else if (code == TEMPLATE_TEMPLATE_PARM) return arg; - else - /* TEMPLATE_PARM_INDEX. */ - return convert_from_reference (unshare_expr (arg)); + else /* if (code == TEMPLATE_PARM_INDEX) */ + { + if (TREE_CODE (arg) == ADDR_EXPR + && DECL_P (TREE_OPERAND (arg, 0))) + /* Remember that there was a reference to this entity. + We should already have called mark_used when taking + the address of this entity, but do so again to make + sure: at worst it's redundant, but if we so far only + called mark_used in a template context then we might + not have yet marked it odr-used (53164). */ + /* Mask out tf_warning_or_error to avoid issuing redundant + diagnostics -- we just care about making sure this + entity is odr-used. */ + mark_used (TREE_OPERAND (arg, 0), + complain & ~tf_warning_or_error); + return convert_from_reference (unshare_expr (arg)); + } } if (level == 1) @@ -20884,22 +20898,10 @@ tsubst_copy_and_build (tree t, } /* Remember that there was a reference to this entity. */ - 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 (function != NULL_TREE + && DECL_P (function) + && !mark_used (function, 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-ptr3a.C b/gcc/testsuite/g++.dg/template/fn-ptr3a.C new file mode 100644 index 00000000000..c76b5cc32b3 --- /dev/null +++ b/gcc/testsuite/g++.dg/template/fn-ptr3a.C @@ -0,0 +1,25 @@ +// PR c++/53164 +// PR c++/105848 +// A stricter version of fn-ptr3.C that verifies using f as a template +// argument alone constitutes an odr-use. + +template<class T> +void f(T) { T::fail; } // { dg-error "fail" } + +template<void (*P)(int)> +struct A { + static void wrap(); +}; + +template<void (*P)(char)> +void wrap(); + +template<int> +void g() { + A<f>::wrap(); // { dg-message "required from here" } + wrap<f>(); // { dg-message "required from here" } +} + +int main() { + g<0>(); +} -- 2.36.1.299.gab336e8f1c ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] 2022-06-07 13:24 ` Patrick Palka @ 2022-06-07 18:05 ` Jason Merrill 2022-06-07 19:18 ` Patrick Palka 0 siblings, 1 reply; 6+ messages in thread From: Jason Merrill @ 2022-06-07 18:05 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches On 6/7/22 09:24, Patrick Palka wrote: > On Mon, 6 Jun 2022, Jason Merrill wrote: > >> 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<class T> >>>>>>>>>>> +void f(T) { >>>>>>>>>>> + T::fail; // { dg-error "not a member" } >>>>>>>>>>> +} >>>>>>>>>>> + >>>>>>>>>>> +template<void(int)> >>>>>>>>>>> +struct A { }; >>>>>>>>>>> + >>>>>>>>>>> +template<int> >>>>>>>>>>> +void g() { >>>>>>>>>>> + A<f> 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<void p(int)> >>>>>>>>>> struct A { >>>>>>>>>> static void h() { >>>>>>>>>> p(0); >>>>>>>>>> } >>>>>>>>>> }; >>>>>>>>>> >>>>>>>>>> template<int> >>>>>>>>>> void g() { >>>>>>>>>> A<f>::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<f>, but convert_nontype_argument_function seems like a weird >>>>>>>>> place >>>>>>>>> to >>>>>>>>> implement that; it's only called again during instantiation of >>>>>>>>> A<f>, >>>>>>>>> when we >>>>>>>>> instantiate the injected-class-name. If A<f> isn't >>>>>>>>> instantiated, >>>>>>>>> e.g. >>>>>>>>> if 'a' >>>>>>>>> is a pointer to A<f>, we again don't instantiate f<int>. >>>>>>>> >>>>>>>> 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<int> ahead >>>>>>>> of >>>>>>>> time (during which mark_used doesn't actually instantiate f<int> >>>>>>>> because >>>>>>>> we're inside a template), at instantiation time the type A<f> 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<f> a;', it does reject >>>>>>>> 'using type = A<f>;' 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 P()> >>>>>>> void g() { >>>>>>> P(); // error: ‘static void A::h()’ is private within this >>>>>>> context >>>>>>> } >>>>>>> >>>>>>> struct A { >>>>>>> void f() { >>>>>>> g<h>(); >>>>>>> } >>>>>>> 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<f> 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<f> gets >>>>> resolved ahead of time to the FUNCTION_DECL f<int>, and we defer marking >>>>> f<int> as used until instantiation (of g) as usual. >>>>> >>>>> Later when instantiating g the type A<f> (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<int> as used (which >>>>> means >>>>> we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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. > > Ah, that seems to work very nicely -- it even handles the case where there the > only use is as a template argument and there is no "use" inside the body > of the function, because we always have to substitute the TEMPLATE_PARM_INDEX > in the generic template arguments. > > Like so? Bootstrapped and tested on x86_64-pc-linux-gnu. I'm not sure > if it's worth also checking !processing_template_decl && !DECL_ODR_USED > before calling mark_used here, since otherwise the call ought to be > certainly redundant. Wouldn't hurt to benchmark that, but I wouldn't expect it to make a measurable difference. > -- >8 -- > > Subject: [PATCH] c++: improve "NTTP argument considered unused" fix [PR53164] > > PR c++/53164 > PR c++/105848 > > gcc/cp/ChangeLog: > > * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on > an ADDR_EXPR argument. > (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/fn-ptr3a.C: New test. > > Co-authored-by: Jason Merrill <jason@redhat.com> > --- > gcc/cp/pt.cc | 40 +++++++++++++----------- > gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 +++++++++++++++ > 2 files changed, 46 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index dcacba56a1c..5863d83b4fd 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -15858,9 +15858,23 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > } > else if (code == TEMPLATE_TEMPLATE_PARM) > return arg; > - else > - /* TEMPLATE_PARM_INDEX. */ > - return convert_from_reference (unshare_expr (arg)); > + else /* if (code == TEMPLATE_PARM_INDEX) */ > + { > + if (TREE_CODE (arg) == ADDR_EXPR > + && DECL_P (TREE_OPERAND (arg, 0))) If the parameter is a reference to function, rather than a pointer, do we need to look through a NOP_EXPR? > + /* Remember that there was a reference to this entity. > + We should already have called mark_used when taking > + the address of this entity, but do so again to make > + sure: at worst it's redundant, but if we so far only > + called mark_used in a template context then we might > + not have yet marked it odr-used (53164). */ > + /* Mask out tf_warning_or_error to avoid issuing redundant > + diagnostics -- we just care about making sure this > + entity is odr-used. */ > + mark_used (TREE_OPERAND (arg, 0), > + complain & ~tf_warning_or_error); > + return convert_from_reference (unshare_expr (arg)); > + } > } > > if (level == 1) > @@ -20884,22 +20898,10 @@ tsubst_copy_and_build (tree t, > } > > /* Remember that there was a reference to this entity. */ > - 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 (function != NULL_TREE > + && DECL_P (function) > + && !mark_used (function, 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-ptr3a.C b/gcc/testsuite/g++.dg/template/fn-ptr3a.C > new file mode 100644 > index 00000000000..c76b5cc32b3 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/fn-ptr3a.C > @@ -0,0 +1,25 @@ > +// PR c++/53164 > +// PR c++/105848 > +// A stricter version of fn-ptr3.C that verifies using f as a template > +// argument alone constitutes an odr-use. > + > +template<class T> > +void f(T) { T::fail; } // { dg-error "fail" } > + > +template<void (*P)(int)> > +struct A { > + static void wrap(); > +}; > + > +template<void (*P)(char)> > +void wrap(); > + > +template<int> > +void g() { > + A<f>::wrap(); // { dg-message "required from here" } > + wrap<f>(); // { dg-message "required from here" } > +} > + > +int main() { > + g<0>(); > +} ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] 2022-06-07 18:05 ` Jason Merrill @ 2022-06-07 19:18 ` Patrick Palka 2022-06-07 20:09 ` Jason Merrill 0 siblings, 1 reply; 6+ messages in thread From: Patrick Palka @ 2022-06-07 19:18 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches On Tue, 7 Jun 2022, Jason Merrill wrote: > On 6/7/22 09:24, Patrick Palka wrote: > > On Mon, 6 Jun 2022, Jason Merrill wrote: > > > > > 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<class T> > > > > > > > > > > > > +void f(T) { > > > > > > > > > > > > + T::fail; // { dg-error "not a member" } > > > > > > > > > > > > +} > > > > > > > > > > > > + > > > > > > > > > > > > +template<void(int)> > > > > > > > > > > > > +struct A { }; > > > > > > > > > > > > + > > > > > > > > > > > > +template<int> > > > > > > > > > > > > +void g() { > > > > > > > > > > > > + A<f> 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<void p(int)> > > > > > > > > > > > struct A { > > > > > > > > > > > static void h() { > > > > > > > > > > > p(0); > > > > > > > > > > > } > > > > > > > > > > > }; > > > > > > > > > > > > > > > > > > > > > > template<int> > > > > > > > > > > > void g() { > > > > > > > > > > > A<f>::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<f>, but convert_nontype_argument_function seems like a > > > > > > > > > > weird > > > > > > > > > > place > > > > > > > > > > to > > > > > > > > > > implement that; it's only called again during instantiation > > > > > > > > > > of > > > > > > > > > > A<f>, > > > > > > > > > > when we > > > > > > > > > > instantiate the injected-class-name. If A<f> isn't > > > > > > > > > > instantiated, > > > > > > > > > > e.g. > > > > > > > > > > if 'a' > > > > > > > > > > is a pointer to A<f>, we again don't instantiate f<int>. > > > > > > > > > > > > > > > > > > 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<int> > > > > > > > > > ahead > > > > > > > > > of > > > > > > > > > time (during which mark_used doesn't actually instantiate > > > > > > > > > f<int> > > > > > > > > > because > > > > > > > > > we're inside a template), at instantiation time the type A<f> > > > > > > > > > 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<f> a;', it does reject > > > > > > > > > 'using type = A<f>;' 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 P()> > > > > > > > > void g() { > > > > > > > > P(); // error: ‘static void A::h()’ is private within > > > > > > > > this > > > > > > > > context > > > > > > > > } > > > > > > > > > > > > > > > > struct A { > > > > > > > > void f() { > > > > > > > > g<h>(); > > > > > > > > } > > > > > > > > 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<f> 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<f> > > > > > > gets > > > > > > resolved ahead of time to the FUNCTION_DECL f<int>, and we defer > > > > > > marking > > > > > > f<int> as used until instantiation (of g) as usual. > > > > > > > > > > > > Later when instantiating g the type A<f> (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<int> as used > > > > > > (which > > > > > > means > > > > > > we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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. > > > > Ah, that seems to work very nicely -- it even handles the case where there > > the > > only use is as a template argument and there is no "use" inside the body > > of the function, because we always have to substitute the > > TEMPLATE_PARM_INDEX > > in the generic template arguments. > > > > Like so? Bootstrapped and tested on x86_64-pc-linux-gnu. I'm not sure > > if it's worth also checking !processing_template_decl && !DECL_ODR_USED > > before calling mark_used here, since otherwise the call ought to be > > certainly redundant. > > Wouldn't hurt to benchmark that, but I wouldn't expect it to make a measurable > difference. I suspect the same, but will benchmark to make sure. > > > -- >8 -- > > > > Subject: [PATCH] c++: improve "NTTP argument considered unused" fix > > [PR53164] > > > > PR c++/53164 > > PR c++/105848 > > > > gcc/cp/ChangeLog: > > > > * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on > > an ADDR_EXPR argument. > > (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. > > > > gcc/testsuite/ChangeLog: > > > > * g++.dg/template/fn-ptr3a.C: New test. > > > > Co-authored-by: Jason Merrill <jason@redhat.com> > > --- > > gcc/cp/pt.cc | 40 +++++++++++++----------- > > gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 +++++++++++++++ > > 2 files changed, 46 insertions(+), 19 deletions(-) > > create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > > index dcacba56a1c..5863d83b4fd 100644 > > --- a/gcc/cp/pt.cc > > +++ b/gcc/cp/pt.cc > > @@ -15858,9 +15858,23 @@ tsubst (tree t, tree args, tsubst_flags_t complain, > > tree in_decl) > > } > > else if (code == TEMPLATE_TEMPLATE_PARM) > > return arg; > > - else > > - /* TEMPLATE_PARM_INDEX. */ > > - return convert_from_reference (unshare_expr (arg)); > > + else /* if (code == TEMPLATE_PARM_INDEX) */ > > + { > > + if (TREE_CODE (arg) == ADDR_EXPR > > + && DECL_P (TREE_OPERAND (arg, 0))) > > If the parameter is a reference to function, rather than a pointer, do we need > to look through a NOP_EXPR? Ah yeah, NOP_EXPR as well as REFERENCE_REF_P it seems. (And I guess in light of C++20 class NTTPs we might need to walk the entire argument in search of decls to call mark_used on, but that seems overkill, Clang doesn't seem to handle that either.) I'm testing the following, which additionally handles/tests the reference-to-function case: -- >8 -- Subject: [PATCH] c++: improve "NTTP argument considered unused" fix [PR53164, PR105848] PR c++/53164 PR c++/105848 gcc/cp/ChangeLog: * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on an ADDR_EXPR argument. (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. gcc/testsuite/ChangeLog: * g++.dg/template/fn-ptr3a.C: New test. Co-authored-by: Jason Merrill <jason@redhat.com> --- gcc/cp/pt.cc | 43 +++++++++++++----------- gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 ++++++++++++++ 2 files changed, 49 insertions(+), 19 deletions(-) create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc index dcacba56a1c..ac9b5a5bb04 100644 --- a/gcc/cp/pt.cc +++ b/gcc/cp/pt.cc @@ -15858,9 +15858,26 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) } else if (code == TEMPLATE_TEMPLATE_PARM) return arg; - else - /* TEMPLATE_PARM_INDEX. */ - return convert_from_reference (unshare_expr (arg)); + else /* if (code == TEMPLATE_PARM_INDEX) */ + { + tree sub = arg; + while (CONVERT_EXPR_P (sub) || REFERENCE_REF_P (sub)) + sub = TREE_OPERAND (sub, 0); + if (TREE_CODE (sub) == ADDR_EXPR + && DECL_P (TREE_OPERAND (sub, 0))) + /* Remember that there was a reference to this entity. + We should already have called mark_used when taking + the address of this entity, but do so again to make + sure: at worst it's redundant, but if we so far only + called mark_used in a template context then we might + not have yet marked it odr-used (53164). */ + /* Mask out tf_warning_or_error so that we don't issue + redundant diagnostics -- we just want to make sure + this entity is odr-used. */ + mark_used (TREE_OPERAND (sub, 0), + complain & ~tf_warning_or_error); + return convert_from_reference (unshare_expr (arg)); + } } if (level == 1) @@ -20884,22 +20901,10 @@ tsubst_copy_and_build (tree t, } /* Remember that there was a reference to this entity. */ - 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 (function != NULL_TREE + && DECL_P (function) + && !mark_used (function, 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-ptr3a.C b/gcc/testsuite/g++.dg/template/fn-ptr3a.C new file mode 100644 index 00000000000..7456be5d51f --- /dev/null +++ b/gcc/testsuite/g++.dg/template/fn-ptr3a.C @@ -0,0 +1,25 @@ +// PR c++/53164 +// PR c++/105848 +// A stricter version of fn-ptr3.C that verifies using f as a template +// argument alone constitutes an odr-use. + +template<class T> +void f(T) { T::fail; } // { dg-error "fail" } + +template<void (*P)(int)> +struct A { + static void wrap(); +}; + +template<void (&P)(char)> +void wrap(); + +template<int> +void g() { + A<f>::wrap(); // { dg-message "required from here" } + wrap<f>(); // { dg-message "required from here" } +} + +int main() { + g<0>(); +} -- 2.36.1.299.gab336e8f1c I'm testing the folowing: ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] 2022-06-07 19:18 ` Patrick Palka @ 2022-06-07 20:09 ` Jason Merrill 0 siblings, 0 replies; 6+ messages in thread From: Jason Merrill @ 2022-06-07 20:09 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches On 6/7/22 15:18, Patrick Palka wrote: > On Tue, 7 Jun 2022, Jason Merrill wrote: > >> On 6/7/22 09:24, Patrick Palka wrote: >>> On Mon, 6 Jun 2022, Jason Merrill wrote: >>> >>>> 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<class T> >>>>>>>>>>>>> +void f(T) { >>>>>>>>>>>>> + T::fail; // { dg-error "not a member" } >>>>>>>>>>>>> +} >>>>>>>>>>>>> + >>>>>>>>>>>>> +template<void(int)> >>>>>>>>>>>>> +struct A { }; >>>>>>>>>>>>> + >>>>>>>>>>>>> +template<int> >>>>>>>>>>>>> +void g() { >>>>>>>>>>>>> + A<f> 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<void p(int)> >>>>>>>>>>>> struct A { >>>>>>>>>>>> static void h() { >>>>>>>>>>>> p(0); >>>>>>>>>>>> } >>>>>>>>>>>> }; >>>>>>>>>>>> >>>>>>>>>>>> template<int> >>>>>>>>>>>> void g() { >>>>>>>>>>>> A<f>::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<f>, but convert_nontype_argument_function seems like a >>>>>>>>>>> weird >>>>>>>>>>> place >>>>>>>>>>> to >>>>>>>>>>> implement that; it's only called again during instantiation >>>>>>>>>>> of >>>>>>>>>>> A<f>, >>>>>>>>>>> when we >>>>>>>>>>> instantiate the injected-class-name. If A<f> isn't >>>>>>>>>>> instantiated, >>>>>>>>>>> e.g. >>>>>>>>>>> if 'a' >>>>>>>>>>> is a pointer to A<f>, we again don't instantiate f<int>. >>>>>>>>>> >>>>>>>>>> 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<int> >>>>>>>>>> ahead >>>>>>>>>> of >>>>>>>>>> time (during which mark_used doesn't actually instantiate >>>>>>>>>> f<int> >>>>>>>>>> because >>>>>>>>>> we're inside a template), at instantiation time the type A<f> >>>>>>>>>> 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<f> a;', it does reject >>>>>>>>>> 'using type = A<f>;' 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 P()> >>>>>>>>> void g() { >>>>>>>>> P(); // error: ‘static void A::h()’ is private within >>>>>>>>> this >>>>>>>>> context >>>>>>>>> } >>>>>>>>> >>>>>>>>> struct A { >>>>>>>>> void f() { >>>>>>>>> g<h>(); >>>>>>>>> } >>>>>>>>> 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<f> 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<f> >>>>>>> gets >>>>>>> resolved ahead of time to the FUNCTION_DECL f<int>, and we defer >>>>>>> marking >>>>>>> f<int> as used until instantiation (of g) as usual. >>>>>>> >>>>>>> Later when instantiating g the type A<f> (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<int> as used >>>>>>> (which >>>>>>> means >>>>>>> we never instantiate it) even though A<f>::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) <case CALL_EXPR>: 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. >>> >>> Ah, that seems to work very nicely -- it even handles the case where there >>> the >>> only use is as a template argument and there is no "use" inside the body >>> of the function, because we always have to substitute the >>> TEMPLATE_PARM_INDEX >>> in the generic template arguments. >>> >>> Like so? Bootstrapped and tested on x86_64-pc-linux-gnu. I'm not sure >>> if it's worth also checking !processing_template_decl && !DECL_ODR_USED >>> before calling mark_used here, since otherwise the call ought to be >>> certainly redundant. >> >> Wouldn't hurt to benchmark that, but I wouldn't expect it to make a measurable >> difference. > > I suspect the same, but will benchmark to make sure. > >> >>> -- >8 -- >>> >>> Subject: [PATCH] c++: improve "NTTP argument considered unused" fix >>> [PR53164] >>> >>> PR c++/53164 >>> PR c++/105848 >>> >>> gcc/cp/ChangeLog: >>> >>> * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on >>> an ADDR_EXPR argument. >>> (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. >>> >>> gcc/testsuite/ChangeLog: >>> >>> * g++.dg/template/fn-ptr3a.C: New test. >>> >>> Co-authored-by: Jason Merrill <jason@redhat.com> >>> --- >>> gcc/cp/pt.cc | 40 +++++++++++++----------- >>> gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 +++++++++++++++ >>> 2 files changed, 46 insertions(+), 19 deletions(-) >>> create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C >>> >>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc >>> index dcacba56a1c..5863d83b4fd 100644 >>> --- a/gcc/cp/pt.cc >>> +++ b/gcc/cp/pt.cc >>> @@ -15858,9 +15858,23 @@ tsubst (tree t, tree args, tsubst_flags_t complain, >>> tree in_decl) >>> } >>> else if (code == TEMPLATE_TEMPLATE_PARM) >>> return arg; >>> - else >>> - /* TEMPLATE_PARM_INDEX. */ >>> - return convert_from_reference (unshare_expr (arg)); >>> + else /* if (code == TEMPLATE_PARM_INDEX) */ >>> + { >>> + if (TREE_CODE (arg) == ADDR_EXPR >>> + && DECL_P (TREE_OPERAND (arg, 0))) >> >> If the parameter is a reference to function, rather than a pointer, do we need >> to look through a NOP_EXPR? > > Ah yeah, NOP_EXPR as well as REFERENCE_REF_P it seems. (And I guess in > light of C++20 class NTTPs we might need to walk the entire argument in > search of decls to call mark_used on, but that seems overkill, Clang > doesn't seem to handle that either.) > > I'm testing the following, which additionally handles/tests the > reference-to-function case: OK. > -- >8 -- > > Subject: [PATCH] c++: improve "NTTP argument considered unused" fix [PR53164, > PR105848] > > PR c++/53164 > PR c++/105848 > > gcc/cp/ChangeLog: > > * pt.cc (tsubst) <case TEMPLATE_PARM_INDEX>: Call mark_used on > an ADDR_EXPR argument. > (tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change. > > gcc/testsuite/ChangeLog: > > * g++.dg/template/fn-ptr3a.C: New test. > > Co-authored-by: Jason Merrill <jason@redhat.com> > --- > gcc/cp/pt.cc | 43 +++++++++++++----------- > gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 ++++++++++++++ > 2 files changed, 49 insertions(+), 19 deletions(-) > create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc > index dcacba56a1c..ac9b5a5bb04 100644 > --- a/gcc/cp/pt.cc > +++ b/gcc/cp/pt.cc > @@ -15858,9 +15858,26 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl) > } > else if (code == TEMPLATE_TEMPLATE_PARM) > return arg; > - else > - /* TEMPLATE_PARM_INDEX. */ > - return convert_from_reference (unshare_expr (arg)); > + else /* if (code == TEMPLATE_PARM_INDEX) */ > + { > + tree sub = arg; > + while (CONVERT_EXPR_P (sub) || REFERENCE_REF_P (sub)) > + sub = TREE_OPERAND (sub, 0); > + if (TREE_CODE (sub) == ADDR_EXPR > + && DECL_P (TREE_OPERAND (sub, 0))) > + /* Remember that there was a reference to this entity. > + We should already have called mark_used when taking > + the address of this entity, but do so again to make > + sure: at worst it's redundant, but if we so far only > + called mark_used in a template context then we might > + not have yet marked it odr-used (53164). */ > + /* Mask out tf_warning_or_error so that we don't issue > + redundant diagnostics -- we just want to make sure > + this entity is odr-used. */ > + mark_used (TREE_OPERAND (sub, 0), > + complain & ~tf_warning_or_error); > + return convert_from_reference (unshare_expr (arg)); > + } > } > > if (level == 1) > @@ -20884,22 +20901,10 @@ tsubst_copy_and_build (tree t, > } > > /* Remember that there was a reference to this entity. */ > - 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 (function != NULL_TREE > + && DECL_P (function) > + && !mark_used (function, 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-ptr3a.C b/gcc/testsuite/g++.dg/template/fn-ptr3a.C > new file mode 100644 > index 00000000000..7456be5d51f > --- /dev/null > +++ b/gcc/testsuite/g++.dg/template/fn-ptr3a.C > @@ -0,0 +1,25 @@ > +// PR c++/53164 > +// PR c++/105848 > +// A stricter version of fn-ptr3.C that verifies using f as a template > +// argument alone constitutes an odr-use. > + > +template<class T> > +void f(T) { T::fail; } // { dg-error "fail" } > + > +template<void (*P)(int)> > +struct A { > + static void wrap(); > +}; > + > +template<void (&P)(char)> > +void wrap(); > + > +template<int> > +void g() { > + A<f>::wrap(); // { dg-message "required from here" } > + wrap<f>(); // { dg-message "required from here" } > +} > + > +int main() { > + g<0>(); > +} ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-06-07 20:09 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-06-06 18:27 [PATCH] c++: function NTTP argument considered unused [PR53164, PR105848] Patrick Palka 2022-06-06 19:26 ` Jason Merrill 2022-06-07 13:24 ` Patrick Palka 2022-06-07 18:05 ` Jason Merrill 2022-06-07 19:18 ` Patrick Palka 2022-06-07 20:09 ` Jason Merrill
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).