c++: improve "NTTP argument considered unused" fix [PR53164, PR105848]

r13-995-g733a792a2b2e16 worked around the problem of (pointer to)
function NTTP arguments not always getting marked as odr-used, by
redundantly calling mark_used on the substituted ADDR_EXPR callee of a
CALL_EXPR.  That is just a narrow workaround however, since it assumes
the function is later called, but the use as a template argument alone
should constitute an odr-use of the function (since template arguments
are an evaluated context, and we're really passing its address); we
shouldn't need to subsequently call or otherwise use the function NTTP
argument.

This patch fixes this in a more general way by walking the template
arguments of each specialization that's about to be instantiated and
redundantly calling mark_used on all entities used within.  As before,
the call to mark_used as it worst a no-op, but it compensates for the
situation where the specialization was first formed in a template context
in which mark_used is inhibited.

Another approach would be to call mark_used whenever we substitute a
TEMPLATE_PARM_INDEX, but that would result in many more redundant calls
to mark_used compared to this approach.  And as the second testcase
below illustrates, we also need to walk C++20 class NTTP arguments which
can be large and thus expensive to walk repeatedly.  The change to
invalid_tparm_referent_p is needed to avoid incorrectly rejecting class
NTTP arguments containing function pointers as in the testcase.

(The third testcase is unrelated to this fix, but it helped rule out an
earlier approach I was considering and it seems we don't have existing
test coverage for this situation.)

	PR c++/53164
	PR c++/105848

gcc/cp/ChangeLog:

	* pt.cc (invalid_tparm_referent_p): Accept ADDR_EXPR of
	FUNCTION_DECL.
	(instantiate_class_template): Call mark_template_arguments_used.
	(tsubst_copy_and_build) <case CALL_EXPR>: Revert r13-995 change.
	(mark_template_arguments_used): Define.
	(instantiate_body): Call mark_template_arguments_used.

gcc/testsuite/ChangeLog:

	* g++.dg/template/fn-ptr3a.C: New test.
	* g++.dg/template/fn-ptr3b.C: New test.
	* g++.dg/template/fn-ptr4.C: New test.

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index dd7f0db96580988bdb8aa3c6f683237f0658717c..022f295b36f8bc26fa8d78bae6e4232bd50453a0 100644 (file)
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -220,6 +220,7 @@ static tree make_argument_pack (tree);
 static tree enclosing_instantiation_of (tree tctx);
 static void instantiate_body (tree pattern, tree args, tree d, bool nested);
 static tree maybe_dependent_member_ref (tree, tree, tsubst_flags_t, tree);
+static void mark_template_arguments_used (tree, tree);
 
 /* Make the current scope suitable for access checking when we are
    processing T.  T can be FUNCTION_DECL for instantiated function
@@ -7142,12 +7143,13 @@ invalid_tparm_referent_p (tree type, tree expr, tsubst_flags_t complain)
              decl = TREE_OPERAND (decl, 0);
            }
 
-       if (!VAR_P (decl))
+       if (!VAR_OR_FUNCTION_DECL_P (decl))
          {
            if (complain & tf_error)
              error_at (cp_expr_loc_or_input_loc (expr),
                        "%qE is not a valid template argument of type %qT "
-                       "because %qE is not a variable", expr, type, decl);
+                       "because %qE is not a variable or function",
+                       expr, type, decl);
            return true;
          }
        else if (cxx_dialect < cxx11 && !DECL_EXTERNAL_LINKAGE_P (decl))
@@ -12152,6 +12154,9 @@ instantiate_class_template (tree type)
       cp_unevaluated_operand = 0;
       c_inhibit_evaluation_warnings = 0;
     }
+
+  mark_template_arguments_used (templ, CLASSTYPE_TI_ARGS (type));
+
   /* Use #pragma pack from the template context.  */
   saved_maximum_field_alignment = maximum_field_alignment;
   maximum_field_alignment = TYPE_PRECISION (pattern);
@@ -21192,22 +21197,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;
@@ -21902,6 +21895,61 @@ check_instantiated_args (tree tmpl, tree args, tsubst_flags_t complain)
   return result;
 }
 
+/* Call mark_used on each entity within the non-type template arguments in
+   ARGS for an instantiation of TMPL, to ensure that each such entity is
+   considered odr-used (and therefore marked for instantiation) regardless of
+   whether the specialization was first formed in a template context (which
+   inhibits mark_used).
+
+   This function assumes push_to_top_level has been called beforehand.  */
+
+static void
+mark_template_arguments_used (tree tmpl, tree args)
+{
+  /* It suffices to do this only when instantiating a primary template.  */
+  if (TREE_CODE (tmpl) != TEMPLATE_DECL || !PRIMARY_TEMPLATE_P (tmpl))
+    return;
+
+  /* We already marked outer arguments when specializing the context.  */
+  args = INNERMOST_TEMPLATE_ARGS (args);
+
+  for (tree arg : tree_vec_range (args))
+    {
+      /* A (pointer/reference to) function or variable NTTP argument.  */
+      if (TREE_CODE (arg) == ADDR_EXPR
+         || TREE_CODE (arg) == INDIRECT_REF)
+       {
+         while (TREE_CODE (arg) == ADDR_EXPR
+                || REFERENCE_REF_P (arg)
+                || CONVERT_EXPR_P (arg))
+           arg = TREE_OPERAND (arg, 0);
+         if (VAR_OR_FUNCTION_DECL_P (arg))
+           {
+             /* Pass tf_none to avoid duplicate diagnostics: if this call
+                fails then an earlier call to mark_used for this argument
+                must have also failed and emitted a diagnostic.  */
+             bool ok = mark_used (arg, tf_none);
+             gcc_checking_assert (ok || seen_error ());
+           }
+       }
+      /* A class NTTP argument.  */
+      else if (VAR_P (arg)
+              && DECL_NTTP_OBJECT_P (arg))
+       {
+         auto mark_used_r = [](tree *tp, int *, void *) {
+           if (VAR_OR_FUNCTION_DECL_P (*tp))
+             {
+               bool ok = mark_used (*tp, tf_none);
+               gcc_checking_assert (ok || seen_error ());
+             }
+           return NULL_TREE;
+         };
+         cp_walk_tree_without_duplicates (&DECL_INITIAL (arg),
+                                          mark_used_r, nullptr);
+       }
+    }
+}
+
 /* We're out of SFINAE context now, so generate diagnostics for the access
    errors we saw earlier when instantiating D from TMPL and ARGS.  */
 
@@ -26775,6 +26823,8 @@ instantiate_body (tree pattern, tree args, tree d, bool nested_p)
       c_inhibit_evaluation_warnings = 0;
     }
 
+  mark_template_arguments_used (pattern, args);
+
   if (VAR_P (d))
     {
       /* The variable might be a lambda's extra scope, and that

diff --git a/gcc/testsuite/g++.dg/template/fn-ptr3a.C b/gcc/testsuite/g++.dg/template/fn-ptr3a.C
new file mode 100644 (file)
index 0000000..345c621
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fn-ptr3a.C
@@ -0,0 +1,27 @@
+// 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 {
+  // P not called
+};
+
+template<void (&P)(char)>
+void wrap() {
+  // P not called
+}
+
+template<int>
+void g() {
+  A<f> a; // { dg-message "required from" }
+  wrap<f>(); // { dg-message "required from" }
+}
+
+int main() {
+  g<0>();
+}

diff --git a/gcc/testsuite/g++.dg/template/fn-ptr3b.C b/gcc/testsuite/g++.dg/template/fn-ptr3b.C
new file mode 100644 (file)
index 0000000..899c355
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fn-ptr3b.C
@@ -0,0 +1,30 @@
+// PR c++/53164
+// PR c++/105848
+// A C++20 version of fn-ptr3a.C using class NTTPs.
+// { dg-do compile { target c++20 } }
+
+template<class T>
+void f(T) { T::fail; } // { dg-error "fail" }
+
+struct B_int { void (*P)(int); };
+struct B_char { void (&P)(char); };
+
+template<B_int B>
+struct A {
+  // B.P not called
+};
+
+template<B_char B>
+void wrap() {
+  // B.P not called
+}
+
+template<int>
+void g() {
+  A<B_int{f}> a; // { dg-message "required from" }
+  wrap<B_char{f}>(); // { dg-message "required from" }
+}
+
+int main() {
+  g<0>();
+}

diff --git a/gcc/testsuite/g++.dg/template/fn-ptr4.C b/gcc/testsuite/g++.dg/template/fn-ptr4.C
new file mode 100644 (file)
index 0000000..36551ec
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fn-ptr4.C
@@ -0,0 +1,14 @@
+// { dg-do compile }
+
+template<void (*P)()>
+void wrap() {
+  P(); // OK, despite A::g not being accessible from here.
+}
+
+struct A {
+  static void f() {
+    wrap<A::g>();
+  }
+private:
+  static void g();
+};