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>();
+}

Reply via email to