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..

-- >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);
+           if (!mark_used (sub, 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..fd7b31bf775
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fn-ptr3.C
@@ -0,0 +1,20 @@
+// PR c++/53164
+
+template<class T>
+void f(T) {
+  T::fail; // { dg-error "'fail' is not a member of 'int'" }
+}
+
+template<void P(int)>
+struct A {
+  static void h() {
+    P(0);
+  }
+};
+
+template<int>
+void g() {
+  A<f>::h();
+}
+
+template void g<0>();
-- 
2.33.0.721.g106298f7f9

Reply via email to