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

2020-07-03 Thread Jason Merrill via Gcc-patches

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] c++: Make convert_like complain about bad ck_ref_bind again [PR95789]

2020-06-29 Thread Marek Polacek via Gcc-patches
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 000..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 
> +struct A {
> +B& get() const { return f; } // { dg-error "binding reference" }
> +
> +B f;
> +};
> +
> +int main() {
> +A 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



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

2020-06-22 Thread Marek Polacek via Gcc-patches
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 000..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 
+struct A {
+B& get() const { return f; } // { dg-error "binding reference" }
+
+B f;
+};
+
+int main() {
+A 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