C++ PATCH for c++/91962 - ICE with reference binding and qualification conversion

Message ID 20191024192403.GB21634@redhat.com
State New
Headers show
Series
  • C++ PATCH for c++/91962 - ICE with reference binding and qualification conversion
Related show

Commit Message

Marek Polacek Oct. 24, 2019, 7:24 p.m.
When fixing c++/91889 (r276251) I was assuming that we couldn't have a ck_qual
under a ck_ref_bind, and I was introducing it in the patch and so this
+   if (next_conversion (convs)->kind == ck_qual)
+     {
+       gcc_assert (same_type_p (TREE_TYPE (expr),
+                    next_conversion (convs)->type));
+       /* Strip the cast created by the ck_qual; cp_build_addr_expr
+          below expects an lvalue.  */
+       STRIP_NOPS (expr);
+     }
in convert_like_real was supposed to handle it.  But that assumption was wrong
as this test shows; here we have "(int *)f" where f is of type long int, and
we're converting it to "const int *const &", so we have both ck_ref_bind and
ck_qual.  That means that the new STRIP_NOPS strips an expression it shouldn't
have, and that then breaks when creating a TARGET_EXPR.  So we want to limit
the stripping to the new case only.  This I do by checking need_temporary_p,
which will be 0 in the new case.  Yes, we can set need_temporary_p when
binding a reference directly, but then we won't have a qualification
conversion.  It is possible to have a bit-field, convert it to a pointer,
and then convert that pointer to a more-qualified pointer, but in that case
we're not dealing with an lvalue, so gl_kind is 0, so we won't enter this
block in reference_binding:
 1747   if ((related_p || compatible_p) && gl_kind)

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

2019-10-24  Marek Polacek  <polacek@redhat.com>

	PR c++/91962 - ICE with reference binding and qualification conversion.
	* call.c (convert_like_real) <case ck_ref_bind>: Check need_temporary_p.

	* g++.dg/cpp0x/ref-bind7.C: New test.

Comments

Jason Merrill Oct. 30, 2019, 6:32 p.m. | #1
On 10/24/19 3:24 PM, Marek Polacek wrote:
> When fixing c++/91889 (r276251) I was assuming that we couldn't have a ck_qual

> under a ck_ref_bind, and I was introducing it in the patch and so this

> +   if (next_conversion (convs)->kind == ck_qual)

> +     {

> +       gcc_assert (same_type_p (TREE_TYPE (expr),

> +                    next_conversion (convs)->type));

> +       /* Strip the cast created by the ck_qual; cp_build_addr_expr

> +          below expects an lvalue.  */

> +       STRIP_NOPS (expr);

> +     }

> in convert_like_real was supposed to handle it.  But that assumption was wrong

> as this test shows; here we have "(int *)f" where f is of type long int, and

> we're converting it to "const int *const &", so we have both ck_ref_bind and

> ck_qual.  That means that the new STRIP_NOPS strips an expression it shouldn't

> have, and that then breaks when creating a TARGET_EXPR.


Are there two conversions in that case, such that stripping only one 
would also fix the problem?

> So we want to limit

> the stripping to the new case only.  This I do by checking need_temporary_p,

> which will be 0 in the new case.  Yes, we can set need_temporary_p when

> binding a reference directly, but then we won't have a qualification

> conversion.  It is possible to have a bit-field, convert it to a pointer,

> and then convert that pointer to a more-qualified pointer, but in that case

> we're not dealing with an lvalue, so gl_kind is 0, so we won't enter this

> block in reference_binding:

>   1747   if ((related_p || compatible_p) && gl_kind)

> 

> Bootstrapped/regtested on x86_64-linux, ok for trunk?


OK.

> 2019-10-24  Marek Polacek  <polacek@redhat.com>

> 

> 	PR c++/91962 - ICE with reference binding and qualification conversion.

> 	* call.c (convert_like_real) <case ck_ref_bind>: Check need_temporary_p.

> 

> 	* g++.dg/cpp0x/ref-bind7.C: New test.

> 

> diff --git gcc/cp/call.c gcc/cp/call.c

> index 55d2abaaddd..d4674a77078 100644

> --- gcc/cp/call.c

> +++ gcc/cp/call.c

> @@ -7386,7 +7386,8 @@ convert_like_real (conversion *convs, tree expr, tree fn, int argnum,

>   	/* direct_reference_binding might have inserted a ck_qual under

>   	   this ck_ref_bind for the benefit of conversion sequence ranking.

>   	   Ignore the conversion; we'll create our own below.  */

> -	if (next_conversion (convs)->kind == ck_qual)

> +	if (next_conversion (convs)->kind == ck_qual

> +	    && !convs->need_temporary_p)

>   	  {

>   	    gcc_assert (same_type_p (TREE_TYPE (expr),

>   				     next_conversion (convs)->type));

> diff --git gcc/testsuite/g++.dg/cpp0x/ref-bind7.C gcc/testsuite/g++.dg/cpp0x/ref-bind7.C

> new file mode 100644

> index 00000000000..e3675bc560d

> --- /dev/null

> +++ gcc/testsuite/g++.dg/cpp0x/ref-bind7.C

> @@ -0,0 +1,13 @@

> +// PR c++/91962 - ICE with reference binding and qualification conversion.

> +// { dg-do compile { target c++11 } }

> +

> +template <typename a> class b {

> +public:

> +  void c(const a &);

> +};

> +class B {

> +  void d();

> +  b<const int *> e;

> +};

> +long f;

> +void B::d() { e.c((const int *)f); }

>

Patch

diff --git gcc/cp/call.c gcc/cp/call.c
index 55d2abaaddd..d4674a77078 100644
--- gcc/cp/call.c
+++ gcc/cp/call.c
@@ -7386,7 +7386,8 @@  convert_like_real (conversion *convs, tree expr, tree fn, int argnum,
 	/* direct_reference_binding might have inserted a ck_qual under
 	   this ck_ref_bind for the benefit of conversion sequence ranking.
 	   Ignore the conversion; we'll create our own below.  */
-	if (next_conversion (convs)->kind == ck_qual)
+	if (next_conversion (convs)->kind == ck_qual
+	    && !convs->need_temporary_p)
 	  {
 	    gcc_assert (same_type_p (TREE_TYPE (expr),
 				     next_conversion (convs)->type));
diff --git gcc/testsuite/g++.dg/cpp0x/ref-bind7.C gcc/testsuite/g++.dg/cpp0x/ref-bind7.C
new file mode 100644
index 00000000000..e3675bc560d
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/ref-bind7.C
@@ -0,0 +1,13 @@ 
+// PR c++/91962 - ICE with reference binding and qualification conversion.
+// { dg-do compile { target c++11 } }
+
+template <typename a> class b {
+public:
+  void c(const a &);
+};
+class B {
+  void d();
+  b<const int *> e;
+};
+long f;
+void B::d() { e.c((const int *)f); }