On Fri, Jul 03, 2020 at 05:24:34PM -0400, Jason Merrill via Gcc-patches wrote:
> On 6/22/20 10:09 PM, Marek Polacek wrote:
> > convert_like issues errors about bad_p conversions at the beginning
> > of the function, but in the ck_ref_bind case, it only issues them
> > after we've called convert_like on the next conversion.
> >
> > This doesn't work as expected since r10-7096 because when we see
> > a conversion from/to class type in a template, we return early, thereby
> > missing the error, and a bad_p conversion goes by undetected. That
> > made the attached test to compile even though it should not.
>
> Hmm, why isn't there an error at instantiation time?
We threw away the result: we're called from
12213 if (complain & tf_error)
12214 {
12215 if (conv)
12216 convert_like (conv, expr, complain);
...
12228 return error_mark_node;
and convert_like never saw converting this->f to B& again when instantiating.
> Though giving an error at template parsing time is definitely preferable.
Yup.
> > I had thought that I could just move the ck_ref_bind/bad_p errors
> > above to the rest of them, but that regressed diagnostics because
> > expr then wasn't converted yet by the nested convert_like_real call.
>
> Yeah, the early section is really just for scalar conversions.
>
> It would probably be good to do normal processing for all other bad
> conversions and only afterward build the IMPLICIT_CONV_EXPR if we aren't
> returning error_mark_node.
Ok, so that if we add more bad_p errors, we won't run into this again.
Unfortunately it's a bit ugly. I could introduce a RETURN macro to
use RETURN (expr); instead of what I have now, but it wouldn't be simply
"conv_expr ? conv_expr : expr", because if we have error_mark_node, we
want to return that, not conv_expr. Does that seem worth it?
(I wish I could at least use the op0 ?: op1 GNU extension.)
I've also added another test.
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
-- >8 --
convert_like issues errors about bad_p conversions at the beginning
of the function, but in the ck_ref_bind case, it only issues them
after we've called convert_like on the next conversion.
This doesn't work as expected since r10-7096 because when we see
a conversion from/to class type in a template, we return early, thereby
missing the error, and a bad_p conversion goes by undetected. That
made the attached test to compile even though it should not.
I had thought that I could just move the ck_ref_bind/bad_p errors
above to the rest of them, but that regressed diagnostics because
expr then wasn't converted yet by the nested convert_like_real call.
So, for bad_p conversions, do the normal processing, but still return
the IMPLICIT_CONV_EXPR to avoid introducing trees that the template
processing can't handle well.
gcc/cp/ChangeLog:
PR c++/95789
* call.c (convert_like_real): Do the normal processing for
conversion that are bad_p. Return the IMPLICIT_CONV_EXPR
instead of EXPR if we're processing a bad_p conversion in
a template.
gcc/testsuite/ChangeLog:
PR c++/95789
* g++.dg/conversion/ref4.C: New test.
* g++.dg/conversion/ref5.C: New test.
---
gcc/cp/call.c | 47 +++++++++++++++-----------
gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++
gcc/testsuite/g++.dg/conversion/ref5.C | 14 ++++++++
3 files changed, 64 insertions(+), 19 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C
create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 5341a572980..65565fc90a8 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7400,12 +7400,19 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
so use an IMPLICIT_CONV_EXPR for this conversion. We would have
created such codes e.g. when calling a user-defined conversion
function. */
+ tree conv_expr = NULL_TREE;
if (processing_template_decl
&& convs->kind != ck_identity
&& (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr))))
{
- expr = build1 (IMPLICIT_CONV_EXPR, totype, expr);
- return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr);
+ conv_expr = build1 (IMPLICIT_CONV_EXPR, totype, expr);
+ if (convs->kind != ck_ref_bind)
+ conv_expr = convert_from_reference (conv_expr);
+ if (!convs->bad_p)
+ return conv_expr;
+ /* Do the normal processing to give the bad_p errors. But we still
+ need to return the IMPLICIT_CONV_EXPR, unless we're returning
+ error_mark_node. */
}
switch (convs->kind)
@@ -7465,7 +7472,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
TARGET_EXPR_LIST_INIT_P (expr) = true;
TARGET_EXPR_DIRECT_INIT_P (expr) = direct;
}
- return expr;
+ return conv_expr ? conv_expr : expr;
}
/* We don't know here whether EXPR is being used as an lvalue or
@@ -7488,7 +7495,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
TARGET_EXPR_LIST_INIT_P (expr) = true;
}
- return expr;
+ return conv_expr ? conv_expr : expr;
}
case ck_identity:
if (BRACE_ENCLOSED_INITIALIZER_P (expr))
@@ -7513,7 +7520,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
continue to warn about uses of EXPR as an integer, rather than as a
pointer. */
expr = build_int_cst (totype, 0);
- return expr;
+ return conv_expr ? conv_expr : expr;
case ck_ambig:
/* We leave bad_p off ck_ambig because overload resolution considers
it valid, it just fails when we try to perform it. So we need to
@@ -7585,7 +7592,8 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
field = next_initializable_field (DECL_CHAIN (field));
CONSTRUCTOR_APPEND_ELT (vec, field, size_int (len));
tree new_ctor = build_constructor (totype, vec);
- return get_target_expr_sfinae (new_ctor, complain);
+ return (conv_expr ? conv_expr
+ : get_target_expr_sfinae (new_ctor, complain));
}
case ck_aggr:
@@ -7598,14 +7606,14 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
imag = perform_implicit_conversion (TREE_TYPE (totype),
imag, complain);
expr = build2 (COMPLEX_EXPR, totype, real, imag);
- return expr;
+ return conv_expr ? conv_expr : expr;
}
expr = reshape_init (totype, expr, complain);
expr = get_target_expr_sfinae (digest_init (totype, expr, complain),
complain);
if (expr != error_mark_node)
TARGET_EXPR_LIST_INIT_P (expr) = true;
- return expr;
+ return conv_expr ? conv_expr : expr;
default:
break;
@@ -7634,19 +7642,19 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
}
if (! MAYBE_CLASS_TYPE_P (totype))
- return expr;
+ return conv_expr ? conv_expr : expr;
/* Don't introduce copies when passing arguments along to the inherited
constructor. */
if (current_function_decl
&& flag_new_inheriting_ctors
&& DECL_INHERITED_CTOR (current_function_decl))
- return expr;
+ return conv_expr ? conv_expr : expr;
if (TREE_CODE (expr) == TARGET_EXPR
&& TARGET_EXPR_LIST_INIT_P (expr))
/* Copy-list-initialization doesn't actually involve a copy. */
- return expr;
+ return conv_expr ? conv_expr : expr;
/* Fall through. */
case ck_base:
@@ -7657,7 +7665,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
/* Build an expression for `*((base*) &expr)'. */
expr = convert_to_base (expr, totype,
!c_cast_p, /*nonnull=*/true, complain);
- return expr;
+ return conv_expr ? conv_expr : expr;
}
/* Copy-initialization where the cv-unqualified version of the source
@@ -7686,7 +7694,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
maybe_inform_about_fndecl_for_bogus_argument_init (fn, argnum);
}
- return build_cplus_new (totype, expr, complain);
+ return conv_expr ? conv_expr : build_cplus_new (totype, expr, complain);
case ck_ref_bind:
{
@@ -7821,16 +7829,16 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
expr = cp_convert (build_pointer_type (TREE_TYPE (ref_type)),
expr, complain);
/* Convert the pointer to the desired reference type. */
- return build_nop (ref_type, expr);
+ return conv_expr ? conv_expr : build_nop (ref_type, expr);
}
case ck_lvalue:
- return decay_conversion (expr, complain);
+ return conv_expr ? conv_expr : decay_conversion (expr, complain);
case ck_fnptr:
/* ??? Should the address of a transaction-safe pointer point to the TM
clone, and this conversion look up the primary function? */
- return build_nop (totype, expr);
+ return conv_expr ? conv_expr : build_nop (totype, expr);
case ck_qual:
/* Warn about deprecated conversion if appropriate. */
@@ -7845,11 +7853,12 @@ convert_like_real (conversion *convs, tree expr, tree
fn, int argnum,
if (convs->base_p)
expr = convert_to_base (expr, totype, !c_cast_p,
/*nonnull=*/false, complain);
- return build_nop (totype, expr);
+ return conv_expr ? conv_expr : build_nop (totype, expr);
case ck_pmem:
- return convert_ptrmem (totype, expr, /*allow_inverse_p=*/false,
+ expr = convert_ptrmem (totype, expr, /*allow_inverse_p=*/false,
c_cast_p, complain);
+ return conv_expr ? conv_expr : expr;
default:
break;
@@ -7866,7 +7875,7 @@ convert_like_real (conversion *convs, tree expr, tree fn,
int argnum,
else
expr = cp_convert (totype, expr, complain);
- return expr;
+ return conv_expr ? conv_expr : expr;
}
/* ARG is being passed to a varargs function. Perform any conversions
diff --git a/gcc/testsuite/g++.dg/conversion/ref4.C
b/gcc/testsuite/g++.dg/conversion/ref4.C
new file mode 100644
index 00000000000..464a4cf6c0f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ref4.C
@@ -0,0 +1,22 @@
+// PR c++/95789
+// { dg-do compile { target c++11 } }
+
+struct B {
+ int n;
+};
+
+template <typename T>
+struct A {
+ B& get() const { return f; } // { dg-error "binding reference" }
+
+ B f;
+};
+
+int main() {
+ A<int> a;
+ a.f = {};
+
+ a.get().n = 10;
+ if (a.f.n != 0)
+ __builtin_abort();
+}
diff --git a/gcc/testsuite/g++.dg/conversion/ref5.C
b/gcc/testsuite/g++.dg/conversion/ref5.C
new file mode 100644
index 00000000000..0042acd0670
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ref5.C
@@ -0,0 +1,14 @@
+// PR c++/96104
+
+template <typename T> void fn(T &);
+class E {};
+struct F {
+ template <typename T> void mfn(T t) { t, fn(E()); } // { dg-error "cannot
bind non-const lvalue reference" }
+};
+int
+main()
+{
+ E e;
+ F f;
+ f.mfn(e);
+}
base-commit: 760df6d296b8fc59796f42dca5eb14012fbfa28b
--
2.26.2
