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