On Wed, 29 Mar 2023, Jason Merrill wrote:
On 3/27/23 09:30, Patrick Palka wrote:
On Thu, 23 Mar 2023, Patrick Palka wrote:
r13-995-g733a792a2b2e16 worked around the problem of FUNCTION_DECL
template arguments not always getting marked as odr-used by redundantly
calling mark_used on the substituted ADDR_EXPR callee of a CALL_EXPR.
This is just a narrow workaround however, since using a FUNCTION_DECL as
a template argument alone should constitutes an odr-use; we shouldn't
need to subsequently e.g. call the function or take its address.
Agreed. But why didn't we already wrap it in an ADDR_EXPR? Even for
reference tparms convert_nontype_argument should do that.
Indeed we do, the commit message was just rather sloppy/inaccurate...
I'll try to correct it.
This patch fixes this in a more general way at template specialization
time by walking the template arguments of the specialization and 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 we end up
forming a specialization from 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.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
PR c++/53164
PR c++/105848
gcc/cp/ChangeLog:
* pt.cc (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_template): Call mark_template_arguments_used.
gcc/testsuite/ChangeLog:
* g++.dg/template/fn-ptr3a.C: New test.
* g++.dg/template/fn-ptr4.C: New test.
---
gcc/cp/pt.cc | 51 ++++++++++++++++--------
gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 ++++++++++++
gcc/testsuite/g++.dg/template/fn-ptr4.C | 14 +++++++
3 files changed, 74 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr4.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 7e4a8de0c8b..9b3cc33331c 100644
--- 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);
/* Make the current scope suitable for access checking when we are
processing T. T can be FUNCTION_DECL for instantiated function
@@ -12142,6 +12143,9 @@ instantiate_class_template (tree type)
cp_unevaluated_operand = 0;
c_inhibit_evaluation_warnings = 0;
}
+
+ mark_template_arguments_used (INNERMOST_TEMPLATE_ARGS (args));
+
/* Use #pragma pack from the template context. */
saved_maximum_field_alignment = maximum_field_alignment;
maximum_field_alignment = TYPE_PRECISION (pattern);
@@ -21173,22 +21177,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;
@@ -21883,6 +21875,31 @@ check_instantiated_args (tree tmpl, tree args,
tsubst_flags_t complain)
return result;
}
+/* Call mark_used on each entity within the template arguments ARGS of
some
+ template specialization, to ensure that each such entity is considered
+ odr-used regardless of whether the specialization was first formed in
a
+ template context.
+
+ This function assumes push_to_top_level has been called beforehand,
and
+ that processing_template_decl has been set iff the template arguments
+ are dependent. */
+
+static void
+mark_template_arguments_used (tree args)
+{
+ gcc_checking_assert (TMPL_ARGS_DEPTH (args) == 1);
+
+ if (processing_template_decl)
+ return;
+
+ auto mark_used_r = [](tree *tp, int *, void *) {
+ if (DECL_P (*tp))
+ mark_used (*tp, tf_none);
+ return NULL_TREE;
+ };
+ cp_walk_tree_without_duplicates (&args, mark_used_r, nullptr);
Here's v2 which optimizes this function by not walking if
!PRIMARY_TEMPLATE_P (since in that case the innermost arguments have
already been walked when instantiating the context), and walking only
non-type arguments:
-- >8
Subject: [PATCH] c++: improve "NTTP argument considered unused" fix
[PR53164,
PR105848]
PR c++/53164
PR c++/105848
gcc/cp/ChangeLog:
* pt.cc (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_template): Call mark_template_arguments_used.
gcc/testsuite/ChangeLog:
* g++.dg/template/fn-ptr3a.C: New test.
* g++.dg/template/fn-ptr4.C: New test.
---
gcc/cp/pt.cc | 57 +++++++++++++++++-------
gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 +++++++++++
gcc/testsuite/g++.dg/template/fn-ptr4.C | 14 ++++++
3 files changed, 80 insertions(+), 16 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr4.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 3bb98ebeac1..a87366bb616 100644
--- 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
@@ -12152,6 +12153,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 +21196,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 +21894,37 @@ 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 a specialization of TMPL, to ensure that each such entity is
+ considered odr-used regardless of whether the specialization was first
+ formed in a template context.
+
+ This function assumes push_to_top_level has been called beforehand, and
+ that processing_template_decl is set iff the template arguments are
+ dependent. */
+
+static void
+mark_template_arguments_used (tree tmpl, tree args)
+{
+ /* It suffices to do this only when fully specializing a primary
template. */
+ if (processing_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))
+ if (!TYPE_P (arg))
+ {
+ auto mark_used_r = [](tree *tp, int *, void *) {
+ if (DECL_P (*tp))
+ mark_used (*tp, tf_none);
+ return NULL_TREE;
+ };
+ cp_walk_tree_without_duplicates (&args, mark_used_r, nullptr);
Do we really need to walk into the args?
Hmm, I think we might need to some sort for walking for class NTTP
arguments since they could be nested CONSTRUCTORs. But for non-class
arguments we know that an entity will only appear as a pointer- or
reference-to-function/variable, so we could pattern match for such a
structure directly. And doing so should be preferable because
cp_walk_tree is relatively expensive and this is a relatively hot code
path. So maybe the following would be preferable?
-- >8 --
Subject: [PATCH] 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 need its address); we shouldn't need to
subsequently call or otherwise use the NTTP argument.
This patch fixes this in a more general way at template specialization
time by walking the template arguments of the specialization and 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 we end
up forming a specialization 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 in theory be large and thus expensive to walk repeatedly. The
change to invalid_tparm_referent_p is needed to avoid bogusly rejecting
the testcase's class NTTP arguments containing function pointers.
(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_template): 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.
---
gcc/cp/pt.cc | 78 ++++++++++++++++++------
gcc/testsuite/g++.dg/template/fn-ptr3a.C | 25 ++++++++
gcc/testsuite/g++.dg/template/fn-ptr3b.C | 28 +++++++++
gcc/testsuite/g++.dg/template/fn-ptr4.C | 14 +++++
4 files changed, 127 insertions(+), 18 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3a.C
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr3b.C
create mode 100644 gcc/testsuite/g++.dg/template/fn-ptr4.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index e514a277872..01ab220320c 100644
--- 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,53 @@ 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 a specialization of TMPL, to ensure that each such entity is
+ considered odr-used regardless of whether the specialization was first
+ formed in a template context.
+
+ This function assumes push_to_top_level has been called beforehand, and
+ that processing_template_decl is set iff the template arguments are
+ dependent. */
+
+static void
+mark_template_arguments_used (tree tmpl, tree args)
+{
+ /* It suffices to do this only when fully specializing a primary template.
*/
+ if (processing_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 (DECL_P (arg))
+ mark_used (arg, tf_none);
+ {
+ auto mark_used_r = [](tree *tp, int *, void *) {
+ if (DECL_P (*tp))
+ mark_used (*tp, tf_none);
+ 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. */
@@ -22031,6 +22071,8 @@ instantiate_template (tree tmpl, tree orig_args, tsubst_flags_t complain)
push_nested_class (ctx);
}
+ mark_template_arguments_used (gen_tmpl, targ_ptr);
+
tree pattern = DECL_TEMPLATE_RESULT (gen_tmpl);
fndecl = NULL_TREE;
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>();
+}
diff --git a/gcc/testsuite/g++.dg/template/fn-ptr3b.C
b/gcc/testsuite/g++.dg/template/fn-ptr3b.C
new file mode 100644
index 00000000000..90d988ce896
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/fn-ptr3b.C
@@ -0,0 +1,28 @@
+// 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 {
+ static void wrap();
+};
+
+template<B_char B>
+void wrap();
+
+template<int>
+void g() {
+ A<B_int{f}>::wrap(); // { dg-message "required from here" }
+ wrap<B_char{f}>(); // { dg-message "required from here" }
+}
+
+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
index 00000000000..36551ec5d7f
--- /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();
+};
