[PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Marek Polacek]

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.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10?

gcc/cp/ChangeLog:

	PR c++/95789
	* call.c (convert_like_real): Do the normal processing for
	ck_ref_bind conversion that are bad_p.

gcc/testsuite/ChangeLog:

	PR c++/95789
	* g++.dg/conversion/ref4.C: New test.
---
 gcc/cp/call.c                          |  9 ++++++++-
 gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++++++++++++
 2 files changed, 30 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 2b39a3700fc..7b16895d5db 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -7402,7 +7402,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
      function.  */
   if (processing_template_decl
       && convs->kind != ck_identity
-      && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr))))
+      && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))
+      /* Do the normal processing to give the bad_p errors in ck_ref_bind
+	 to avoid losing the fact that this conversion was bad.  Since we
+	 are going to return error_mark_node, we don't care about trees
+	 breaking in templates.  */
+      && !(convs->kind == ck_ref_bind
+	   && convs->bad_p
+	   && !next_conversion (convs)->bad_p))
     {
       expr = build1 (IMPLICIT_CONV_EXPR, totype, expr);
       return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr);
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();
+}

base-commit: 0164e59835de81d758fd4c56248ad7a46435fbfa
-- 
Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA

Re: [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Marek Polacek]

Ping.

On Mon, Jun 22, 2020 at 10:09:27PM -0400, Marek Polacek via Gcc-patches 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.
> 
> 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.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk and 10?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95789
> 	* call.c (convert_like_real): Do the normal processing for
> 	ck_ref_bind conversion that are bad_p.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95789
> 	* g++.dg/conversion/ref4.C: New test.
> ---
>  gcc/cp/call.c                          |  9 ++++++++-
>  gcc/testsuite/g++.dg/conversion/ref4.C | 22 ++++++++++++++++++++++
>  2 files changed, 30 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 2b39a3700fc..7b16895d5db 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -7402,7 +7402,14 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
>       function.  */
>    if (processing_template_decl
>        && convs->kind != ck_identity
> -      && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr))))
> +      && (CLASS_TYPE_P (totype) || CLASS_TYPE_P (TREE_TYPE (expr)))
> +      /* Do the normal processing to give the bad_p errors in ck_ref_bind
> +	 to avoid losing the fact that this conversion was bad.  Since we
> +	 are going to return error_mark_node, we don't care about trees
> +	 breaking in templates.  */
> +      && !(convs->kind == ck_ref_bind
> +	   && convs->bad_p
> +	   && !next_conversion (convs)->bad_p))
>      {
>        expr = build1 (IMPLICIT_CONV_EXPR, totype, expr);
>        return convs->kind == ck_ref_bind ? expr : convert_from_reference (expr);
> 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();
> +}
> 
> base-commit: 0164e59835de81d758fd4c56248ad7a46435fbfa
> -- 
> Marek Polacek • Red Hat, Inc. • 300 A St, Boston, MA
> 

Marek

Re: [PATCH] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Jason Merrill]

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?

Though giving an error at template parsing time is definitely preferable.

> 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.

Jason

Re: [PATCH v2] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Marek Polacek]

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

Re: [PATCH v2] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Jason Merrill]

On 7/8/20 4:35 PM, Marek Polacek wrote:
> 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.)

Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR?

> 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
> 

Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Marek Polacek]

On Sat, Jul 11, 2020 at 10:32:55AM -0400, Jason Merrill via Gcc-patches wrote:
> On 7/8/20 4:35 PM, Marek Polacek wrote:
> > 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.)
> 
> Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR?

Perhaps like this?  convert_like_real_1 is unsightly and I have 
plans to fix that (and get rid of the convert_like macro), but
that's a clean-up, not a bug fix so it's not included in this patch.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

-- >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.  This I achieved by adding a wrapper
function.

gcc/cp/ChangeLog:

	PR c++/95789
	PR c++/96104
	PR c++/96179
	* call.c (convert_like_real_1): Renamed from convert_like_real.
	(convert_like_real): New wrapper for convert_like_real_1.

gcc/testsuite/ChangeLog:

	PR c++/95789
	PR c++/96104
	PR c++/96179
	* g++.dg/conversion/ref4.C: New test.
	* g++.dg/conversion/ref5.C: New test.
	* g++.dg/conversion/ref6.C: New test.
---
 gcc/cp/call.c                          | 54 ++++++++++++++++++--------
 gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++
 gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++
 gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++
 4 files changed, 98 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C
 create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C
 create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C

diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index 5341a572980..6d5d5e801a5 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t);
 		     /*c_cast_p=*/false, (COMPLAIN))
 static tree convert_like_real (conversion *, tree, tree, int, bool,
 			       bool, tsubst_flags_t);
+static tree convert_like_real_1 (conversion *, tree, tree, int, bool,
+				 bool, tsubst_flags_t);
 static void op_error (const op_location_t &, enum tree_code, enum tree_code,
 		      tree, tree, tree, bool);
 static struct z_candidate *build_user_type_conversion_1 (tree, tree, int,
@@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
 	     "are only available with %<-std=c++20%> or %<-std=gnu++20%>");
 }
 
+/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR.  */
+
+static tree
+convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
+		   bool issue_conversion_warnings,
+		   bool c_cast_p, tsubst_flags_t complain)
+{
+  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
+     and creating a CALL_EXPR in a template breaks in finish_call_expr
+     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 (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr))))
+    {
+      conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, 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.  */
+    }
+  expr = convert_like_real_1 (convs, expr, fn, argnum,
+			      issue_conversion_warnings, c_cast_p, complain);
+  if (expr == error_mark_node)
+    return error_mark_node;
+  return conv_expr ? conv_expr : expr;
+}
+
 /* Perform the conversions in CONVS on the expression EXPR.  FN and
    ARGNUM are used for diagnostics.  ARGNUM is zero based, -1
    indicates the `this' argument of a method.  INNER is nonzero when
@@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
    conversions to inaccessible bases are permitted.  */
 
 static tree
-convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
-		   bool issue_conversion_warnings,
-		   bool c_cast_p, tsubst_flags_t complain)
+convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum,
+		     bool issue_conversion_warnings,
+		     bool c_cast_p, tsubst_flags_t complain)
 {
   tree totype = convs->type;
   diagnostic_t diag_kind;
@@ -7395,19 +7430,6 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
   if (issue_conversion_warnings && (complain & tf_warning))
     conversion_null_warnings (totype, expr, fn, argnum);
 
-  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
-     and creating a CALL_EXPR in a template breaks in finish_call_expr
-     so use an IMPLICIT_CONV_EXPR for this conversion.  We would have
-     created such codes e.g. when calling a user-defined conversion
-     function.  */
-  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);
-    }
-
   switch (convs->kind)
     {
     case ck_user:
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);
+}
diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C
new file mode 100644
index 00000000000..fc87199053c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/conversion/ref6.C
@@ -0,0 +1,24 @@
+// PR c++/96179
+// { dg-do compile { target c++11 } }
+
+template<typename T> struct vector
+{
+  void push_back(T) { }
+};
+
+struct dummy{
+        int a;
+};
+
+void Modify_Dummy(dummy &d){
+        d.a=1;
+}
+
+template <bool bla=true> void Templated_Function(){
+        vector<dummy> A;
+        A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" }
+}
+
+int main(){
+        Templated_Function();
+}

base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55
-- 
2.26.2

Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Jason Merrill]

On 7/13/20 4:48 PM, Marek Polacek wrote:
> On Sat, Jul 11, 2020 at 10:32:55AM -0400, Jason Merrill via Gcc-patches wrote:
>> On 7/8/20 4:35 PM, Marek Polacek wrote:
>>> 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.)
>>
>> Or introduce a wrapper function that handles IMPLICIT_CONV_EXPR?
> 
> Perhaps like this?  convert_like_real_1 is unsightly and I have
> plans to fix that (and get rid of the convert_like macro), but
> that's a clean-up, not a bug fix so it's not included in this patch.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?

OK.

> -- >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.  This I achieved by adding a wrapper
> function.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95789
> 	PR c++/96104
> 	PR c++/96179
> 	* call.c (convert_like_real_1): Renamed from convert_like_real.
> 	(convert_like_real): New wrapper for convert_like_real_1.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95789
> 	PR c++/96104
> 	PR c++/96179
> 	* g++.dg/conversion/ref4.C: New test.
> 	* g++.dg/conversion/ref5.C: New test.
> 	* g++.dg/conversion/ref6.C: New test.
> ---
>   gcc/cp/call.c                          | 54 ++++++++++++++++++--------
>   gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++
>   gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++
>   gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++
>   4 files changed, 98 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 5341a572980..6d5d5e801a5 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t);
>   		     /*c_cast_p=*/false, (COMPLAIN))
>   static tree convert_like_real (conversion *, tree, tree, int, bool,
>   			       bool, tsubst_flags_t);
> +static tree convert_like_real_1 (conversion *, tree, tree, int, bool,
> +				 bool, tsubst_flags_t);
>   static void op_error (const op_location_t &, enum tree_code, enum tree_code,
>   		      tree, tree, tree, bool);
>   static struct z_candidate *build_user_type_conversion_1 (tree, tree, int,
> @@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
>   	     "are only available with %<-std=c++20%> or %<-std=gnu++20%>");
>   }
>   
> +/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR.  */
> +
> +static tree
> +convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
> +		   bool issue_conversion_warnings,
> +		   bool c_cast_p, tsubst_flags_t complain)
> +{
> +  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
> +     and creating a CALL_EXPR in a template breaks in finish_call_expr
> +     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 (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr))))
> +    {
> +      conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, 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.  */
> +    }
> +  expr = convert_like_real_1 (convs, expr, fn, argnum,
> +			      issue_conversion_warnings, c_cast_p, complain);
> +  if (expr == error_mark_node)
> +    return error_mark_node;
> +  return conv_expr ? conv_expr : expr;
> +}
> +
>   /* Perform the conversions in CONVS on the expression EXPR.  FN and
>      ARGNUM are used for diagnostics.  ARGNUM is zero based, -1
>      indicates the `this' argument of a method.  INNER is nonzero when
> @@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
>      conversions to inaccessible bases are permitted.  */
>   
>   static tree
> -convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
> -		   bool issue_conversion_warnings,
> -		   bool c_cast_p, tsubst_flags_t complain)
> +convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum,
> +		     bool issue_conversion_warnings,
> +		     bool c_cast_p, tsubst_flags_t complain)
>   {
>     tree totype = convs->type;
>     diagnostic_t diag_kind;
> @@ -7395,19 +7430,6 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
>     if (issue_conversion_warnings && (complain & tf_warning))
>       conversion_null_warnings (totype, expr, fn, argnum);
>   
> -  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
> -     and creating a CALL_EXPR in a template breaks in finish_call_expr
> -     so use an IMPLICIT_CONV_EXPR for this conversion.  We would have
> -     created such codes e.g. when calling a user-defined conversion
> -     function.  */
> -  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);
> -    }
> -
>     switch (convs->kind)
>       {
>       case ck_user:
> 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);
> +}
> diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C
> new file mode 100644
> index 00000000000..fc87199053c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/conversion/ref6.C
> @@ -0,0 +1,24 @@
> +// PR c++/96179
> +// { dg-do compile { target c++11 } }
> +
> +template<typename T> struct vector
> +{
> +  void push_back(T) { }
> +};
> +
> +struct dummy{
> +        int a;
> +};
> +
> +void Modify_Dummy(dummy &d){
> +        d.a=1;
> +}
> +
> +template <bool bla=true> void Templated_Function(){
> +        vector<dummy> A;
> +        A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" }
> +}
> +
> +int main(){
> +        Templated_Function();
> +}
> 
> base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55
> 

Re: [PATCH v3] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

[Nathan Sidwell]

On 7/13/20 4:48 PM, Marek Polacek via Gcc-patches wrote:

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk/10?
> 
> -- >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.  This I achieved by adding a wrapper
> function.

LGTM, thanks!

> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95789
> 	PR c++/96104
> 	PR c++/96179
> 	* call.c (convert_like_real_1): Renamed from convert_like_real.
> 	(convert_like_real): New wrapper for convert_like_real_1.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95789
> 	PR c++/96104
> 	PR c++/96179
> 	* g++.dg/conversion/ref4.C: New test.
> 	* g++.dg/conversion/ref5.C: New test.
> 	* g++.dg/conversion/ref6.C: New test.
> ---
>   gcc/cp/call.c                          | 54 ++++++++++++++++++--------
>   gcc/testsuite/g++.dg/conversion/ref4.C | 22 +++++++++++
>   gcc/testsuite/g++.dg/conversion/ref5.C | 14 +++++++
>   gcc/testsuite/g++.dg/conversion/ref6.C | 24 ++++++++++++
>   4 files changed, 98 insertions(+), 16 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref4.C
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref5.C
>   create mode 100644 gcc/testsuite/g++.dg/conversion/ref6.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 5341a572980..6d5d5e801a5 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -171,6 +171,8 @@ static tree build_over_call (struct z_candidate *, int, tsubst_flags_t);
>   		     /*c_cast_p=*/false, (COMPLAIN))
>   static tree convert_like_real (conversion *, tree, tree, int, bool,
>   			       bool, tsubst_flags_t);
> +static tree convert_like_real_1 (conversion *, tree, tree, int, bool,
> +				 bool, tsubst_flags_t);
>   static void op_error (const op_location_t &, enum tree_code, enum tree_code,
>   		      tree, tree, tree, bool);
>   static struct z_candidate *build_user_type_conversion_1 (tree, tree, int,
> @@ -7281,6 +7283,39 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
>   	     "are only available with %<-std=c++20%> or %<-std=gnu++20%>");
>   }
>   
> +/* Wrapper for convert_like_real_1 that handles creating IMPLICIT_CONV_EXPR.  */
> +
> +static tree
> +convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
> +		   bool issue_conversion_warnings,
> +		   bool c_cast_p, tsubst_flags_t complain)
> +{
> +  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
> +     and creating a CALL_EXPR in a template breaks in finish_call_expr
> +     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 (convs->type) || CLASS_TYPE_P (TREE_TYPE (expr))))
> +    {
> +      conv_expr = build1 (IMPLICIT_CONV_EXPR, convs->type, 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.  */
> +    }
> +  expr = convert_like_real_1 (convs, expr, fn, argnum,
> +			      issue_conversion_warnings, c_cast_p, complain);
> +  if (expr == error_mark_node)
> +    return error_mark_node;
> +  return conv_expr ? conv_expr : expr;
> +}
> +
>   /* Perform the conversions in CONVS on the expression EXPR.  FN and
>      ARGNUM are used for diagnostics.  ARGNUM is zero based, -1
>      indicates the `this' argument of a method.  INNER is nonzero when
> @@ -7292,9 +7327,9 @@ maybe_warn_array_conv (location_t loc, conversion *c, tree expr)
>      conversions to inaccessible bases are permitted.  */
>   
>   static tree
> -convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
> -		   bool issue_conversion_warnings,
> -		   bool c_cast_p, tsubst_flags_t complain)
> +convert_like_real_1 (conversion *convs, tree expr, tree fn, int argnum,
> +		     bool issue_conversion_warnings,
> +		     bool c_cast_p, tsubst_flags_t complain)
>   {
>     tree totype = convs->type;
>     diagnostic_t diag_kind;
> @@ -7395,19 +7430,6 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
>     if (issue_conversion_warnings && (complain & tf_warning))
>       conversion_null_warnings (totype, expr, fn, argnum);
>   
> -  /* Creating &TARGET_EXPR<> in a template breaks when substituting,
> -     and creating a CALL_EXPR in a template breaks in finish_call_expr
> -     so use an IMPLICIT_CONV_EXPR for this conversion.  We would have
> -     created such codes e.g. when calling a user-defined conversion
> -     function.  */
> -  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);
> -    }
> -
>     switch (convs->kind)
>       {
>       case ck_user:
> 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);
> +}
> diff --git a/gcc/testsuite/g++.dg/conversion/ref6.C b/gcc/testsuite/g++.dg/conversion/ref6.C
> new file mode 100644
> index 00000000000..fc87199053c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/conversion/ref6.C
> @@ -0,0 +1,24 @@
> +// PR c++/96179
> +// { dg-do compile { target c++11 } }
> +
> +template<typename T> struct vector
> +{
> +  void push_back(T) { }
> +};
> +
> +struct dummy{
> +        int a;
> +};
> +
> +void Modify_Dummy(dummy &d){
> +        d.a=1;
> +}
> +
> +template <bool bla=true> void Templated_Function(){
> +        vector<dummy> A;
> +        A.push_back(Modify_Dummy(dummy{0})); // { dg-error "cannot bind non-const lvalue reference" }
> +}
> +
> +int main(){
> +        Templated_Function();
> +}
> 
> base-commit: 9cba898481368ce16c6a2d30ef781a82dce27c55
> 


-- 
Nathan Sidwell