On Fri, Mar 05, 2021 at 05:15:49PM -0500, Jason Merrill via Gcc-patches wrote:
> On 3/3/21 7:55 PM, Marek Polacek wrote:
> > In this test we are building a call in a template, but since neither
> > the function nor any of its arguments are dependent, we go down the
> > normal path in finish_call_expr. convert_arguments sees that we're
> > binding a reference to int to double and therein convert_to_integer
> > creates a FIX_TRUNC_EXPR. Later, we call check_function_arguments
> > which folds the arguments, and, in a template, fold_for_warn calls
> > fold_non_dependent_expr. But tsubst_copy_and_build should not see
> > a FIX_TRUNC_EXPR (see the patch discussed in
> > <https://gcc.gnu.org/pipermail/gcc-patches/2018-March/496183.html>)
> > or we crash.
>
> This again, sigh. Let me take a step back.
:-(
> So basically, the output of convert_like_real in a template is a mix of
> template and non-template trees, and thus unsuitable for consumption by
> anything other than grabbing its type and throwing it away, as most callers
> do.
>
> The problem here is that cp_build_function_call_vec calls
> check_function_arguments with these trees. build_over_call, however, does
> not call check_function_arguments in a template. Preventing that call in a
> template also fixes the testcase, though it regresses diagnostic location in
> Wnonnull5.C (which it shouldn't, that's a separate bug).
Yeah, that does strike me as wrong. So I've tried
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -4038,8 +4038,10 @@ cp_build_function_call_vec (tree function, vec<tree,
va_gc> **params,
/* Check for errors in format strings and inappropriately
null parameters. */
- bool warned_p = check_function_arguments (input_location, fndecl, fntype,
- nargs, argarray, NULL);
+ bool warned_p
+ = (!processing_template_decl
+ && check_function_arguments (input_location, fndecl, fntype,
+ nargs, argarray, NULL));
ret = build_cxx_call (function, nargs, argarray, complain, orig_fndecl);
and saw no failures with it (with/out my patch). In fact, I'd like to
apply both patches.
> IMPLICIT_CONV_EXPR is a way to represent the conversion so that the result
> is still a template tree, and therefore suitable for fold_for_warn, which
> allows us to warn when parsing the template, which is generally desirable.
That sounds like a fair assessment.
> I think the approach of expanding IMPLICIT_CONV_EXPR is probably the right
> choice, but perhaps we should expand it to all non-trivial conversions, not
> just those that would use problematic tree codes.
Yeah; it's worked pretty well for classes after we've dealt with the
initial fallout. I've factored the check into a new function, and also
extended it with the case where we'd create a FLOAT_EXPR. I don't know
if that covers all non-trivial conversions.
Do we want to assert that FLOAT_EXPR never makes its way into tsubst now?
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
-- >8 --
In this test we are building a call in a template, but since neither
the function nor any of its arguments are dependent, we go down the
normal path in finish_call_expr. convert_arguments sees that we're
binding a reference to int to double and therein convert_to_integer
creates a FIX_TRUNC_EXPR. Later, we call check_function_arguments
which folds the arguments, and, in a template, fold_for_warn calls
fold_non_dependent_expr. But tsubst_copy_and_build should not see
a FIX_TRUNC_EXPR (see the patch discussed in
<https://gcc.gnu.org/pipermail/gcc-patches/2018-March/496183.html>)
or we crash.
So let's not create a FIX_TRUNC_EXPR in a template in the first place
and instead use IMPLICIT_CONV_EXPR.
gcc/cp/ChangeLog:
PR c++/97973
* call.c (conv_unsafe_in_template_p): New.
(convert_like): Use it.
gcc/testsuite/ChangeLog:
PR c++/97973
* g++.dg/conversion/real-to-int1.C: New test.
---
gcc/cp/call.c | 23 ++++++++++++++++++-
.../g++.dg/conversion/real-to-int1.C | 17 ++++++++++++++
2 files changed, 39 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/conversion/real-to-int1.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 7d12fea60f2..f450691d3f6 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8048,6 +8048,27 @@ convert_like_internal (conversion *convs, tree expr,
tree fn, int argnum,
return expr;
}
+/* Return true if converting FROM to TO is unsafe in a template. */
+
+static bool
+conv_unsafe_in_template_p (tree to, tree from)
+{
+ /* Converting classes involves TARGET_EXPR. */
+ if (CLASS_TYPE_P (to) || CLASS_TYPE_P (from))
+ return true;
+
+ /* Converting real to integer produces FIX_TRUNC_EXPR which tsubst
+ doesn't handle. */
+ if (SCALAR_FLOAT_TYPE_P (from) && INTEGRAL_OR_ENUMERATION_TYPE_P (to))
+ return true;
+
+ /* Converting integer to real isn't a trivial conversion, either. */
+ if (INTEGRAL_OR_ENUMERATION_TYPE_P (from) && SCALAR_FLOAT_TYPE_P (to))
+ return true;
+
+ return false;
+}
+
/* Wrapper for convert_like_internal that handles creating
IMPLICIT_CONV_EXPR. */
@@ -8064,7 +8085,7 @@ convert_like (conversion *convs, tree expr, tree fn, int
argnum,
tree conv_expr = NULL_TREE;
if (processing_template_decl
&& convs->kind != ck_identity
- && (CLASS_TYPE_P (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr))))
+ && conv_unsafe_in_template_p (convs->type, TREE_TYPE (expr)))
{
conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, expr);
if (convs->kind != ck_ref_bind)
diff --git a/gcc/testsuite/g++.dg/conversion/real-to-int1.C
b/gcc/testsuite/g++.dg/conversion/real-to-int1.C
new file mode 100644
index 00000000000..f7b990b3f4b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/real-to-int1.C
@@ -0,0 +1,17 @@
+// PR c++/97973
+
+void (*foo[1])(const int &);
+void (*foo2[1])(const double &);
+
+template<typename>
+void f ()
+{
+ (foo[0])(1.1);
+ (foo2[0])(1);
+}
+
+void
+g ()
+{
+ f<char> ();
+}
base-commit: bd85b4dd2dd7b00b6342ed1e33fb48035a3dcb61
--
2.29.2
