public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: fixes for derived-to-base reference binding [PR107085]
@ 2022-10-05 21:27 Marek Polacek
  2022-10-06  0:25 ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-05 21:27 UTC (permalink / raw)
  To: Jason Merrill, GCC Patches

This PR reports that

  struct Base {};
  struct Derived : Base {};
  static_assert(__reference_constructs_from_temporary(Base const&, Derived));

doesn't pass, which it should: it's just like

  const Base& b(Derived{});

where we bind 'b' to the Base subobject of a temporary object of type
Derived.  The ck_base conversion didn't have ->need_temporary_p set because
we didn't need to create a temporary object just for the base, but the whole
object is a temporary so we're still binding to a temporary.  Fixed by
the conv_is_prvalue hunk.

That broke a bunch of tests.  I've distilled the issue into a simple test
in elision4.C.  Essentially, we have

  struct B { /* ... */ };
  struct D : B { };
  B b = D();

and we set force_elide in build_over_call, but we're unable to actually
elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.

<https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
elision "can only apply when the object being initialized is known not to be
a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
checking ck_base at all.

Does that make sense?  If so...

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

	PR c++/107085

gcc/cp/ChangeLog:

	* call.cc (conv_is_prvalue): Return true if the base subobject is part
	of a temporary object.
	(build_over_call): Don't force_elide when it's a derived-to-base
	conversion.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
	result.
	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
	* g++.dg/cpp0x/elision4.C: New test.
---
 gcc/cp/call.cc                                  | 17 +++++++++++++++--
 gcc/testsuite/g++.dg/cpp0x/elision4.C           | 17 +++++++++++++++++
 .../ext/reference_constructs_from_temporary1.C  |  2 +-
 .../ext/reference_converts_from_temporary1.C    |  2 +-
 4 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd04a1d309a..15e969d6429 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9186,7 +9186,11 @@ conv_is_prvalue (conversion *c)
 {
   if (c->kind == ck_rvalue)
     return true;
-  if (c->kind == ck_base && c->need_temporary_p)
+  if (c->kind == ck_base
+      /* We may not need a temporary object for the base itself, but if it
+	 is the base subobject of a temporary object, we're still dealing
+	 with a prvalue.  */
+      && (c->need_temporary_p || conv_is_prvalue (next_conversion (c))))
     return true;
   if (c->kind == ck_user && !TYPE_REF_P (c->type))
     return true;
@@ -9417,7 +9421,16 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
       && (DECL_COPY_CONSTRUCTOR_P (fn)
 	  || DECL_MOVE_CONSTRUCTOR_P (fn))
       && !unsafe_return_slot_p (first_arg)
-      && conv_binds_ref_to_prvalue (convs[0]))
+      && conv_binds_ref_to_prvalue (convs[0])
+      /* Converting a class to another class that then binds to this
+	 copy/move constructor's argument is OK, but not when it's a
+	 derived-to-base conversion.  */
+      && [convs] {
+	   for (conversion *t = convs[0]; t; t = next_conversion (t))
+	     if (t->kind == ck_base)
+	       return false;
+	   return true;
+	 }())
     {
       force_elide = true;
       goto not_really_used;
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
new file mode 100644
index 00000000000..4833b50d48e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
@@ -0,0 +1,17 @@
+// PR c++/107085
+// { dg-do compile { target c++11 } }
+
+// Must be non-trivial to exhibit the ICE.
+struct X {
+  X();
+  X(X&&);
+};
+struct Z : X {};
+X x1 = Z();
+X x2 = X(Z());
+
+// ...but let's try the trivial path in build_over_call as well.
+struct B { };
+struct D : B { };
+B b1 = D();
+B b2 = B(D());
diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
index 76de905a35d..5354b1dc4e6 100644
--- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
@@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
 SA(!__reference_constructs_from_temporary(int&&, G2));
 SA(!__reference_constructs_from_temporary(const int&, H2));
 
-SA(!__reference_constructs_from_temporary(const Base&, Der));
+SA(__reference_constructs_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
index 90196c38742..e6c159e9b00 100644
--- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
@@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
 SA(!__reference_converts_from_temporary(int&&, G2));
 SA(!__reference_converts_from_temporary(const int&, H2));
 
-SA(!__reference_converts_from_temporary(const Base&, Der));
+SA(__reference_converts_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_converts_from_temporary(int&&, id<int[3]>));

base-commit: e99dcbb54e07b798c3353124f38336f96a826d43
-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-05 21:27 [PATCH] c++: fixes for derived-to-base reference binding [PR107085] Marek Polacek
@ 2022-10-06  0:25 ` Jason Merrill
  2022-10-06 14:49   ` [PATCH v2] " Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2022-10-06  0:25 UTC (permalink / raw)
  To: Marek Polacek, GCC Patches

On 10/5/22 17:27, Marek Polacek wrote:
> This PR reports that
> 
>    struct Base {};
>    struct Derived : Base {};
>    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> 
> doesn't pass, which it should: it's just like
> 
>    const Base& b(Derived{});
> 
> where we bind 'b' to the Base subobject of a temporary object of type
> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> we didn't need to create a temporary object just for the base, but the whole
> object is a temporary so we're still binding to a temporary.  Fixed by
> the conv_is_prvalue hunk.
> 
> That broke a bunch of tests.  I've distilled the issue into a simple test
> in elision4.C.  Essentially, we have
> 
>    struct B { /* ... */ };
>    struct D : B { };
>    B b = D();
> 
> and we set force_elide in build_over_call, but we're unable to actually
> elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
> 
> <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
> elision "can only apply when the object being initialized is known not to be
> a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
> the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
> derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
> checking ck_base at all.
> 
> Does that make sense?  If so...
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/107085
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
> 	of a temporary object.

No, the base subobject of a prvalue is an xvalue.

I think the problem is that an expression being a prvalue is a subset of 
binding a reference to a temporary, and we shouldn't try to express both 
of those using the same function: you need a separate 
conv_binds_ref_to_temporary.

> 	(build_over_call): Don't force_elide when it's a derived-to-base
> 	conversion.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> 	result.
> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> 	* g++.dg/cpp0x/elision4.C: New test.
> ---
>   gcc/cp/call.cc                                  | 17 +++++++++++++++--
>   gcc/testsuite/g++.dg/cpp0x/elision4.C           | 17 +++++++++++++++++
>   .../ext/reference_constructs_from_temporary1.C  |  2 +-
>   .../ext/reference_converts_from_temporary1.C    |  2 +-
>   4 files changed, 34 insertions(+), 4 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd04a1d309a..15e969d6429 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9186,7 +9186,11 @@ conv_is_prvalue (conversion *c)
>   {
>     if (c->kind == ck_rvalue)
>       return true;
> -  if (c->kind == ck_base && c->need_temporary_p)
> +  if (c->kind == ck_base
> +      /* We may not need a temporary object for the base itself, but if it
> +	 is the base subobject of a temporary object, we're still dealing
> +	 with a prvalue.  */
> +      && (c->need_temporary_p || conv_is_prvalue (next_conversion (c))))
>       return true;
>     if (c->kind == ck_user && !TYPE_REF_P (c->type))
>       return true;
> @@ -9417,7 +9421,16 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain)
>         && (DECL_COPY_CONSTRUCTOR_P (fn)
>   	  || DECL_MOVE_CONSTRUCTOR_P (fn))
>         && !unsafe_return_slot_p (first_arg)
> -      && conv_binds_ref_to_prvalue (convs[0]))
> +      && conv_binds_ref_to_prvalue (convs[0])
> +      /* Converting a class to another class that then binds to this
> +	 copy/move constructor's argument is OK, but not when it's a
> +	 derived-to-base conversion.  */
> +      && [convs] {
> +	   for (conversion *t = convs[0]; t; t = next_conversion (t))
> +	     if (t->kind == ck_base)
> +	       return false;
> +	   return true;
> +	 }())
>       {
>         force_elide = true;
>         goto not_really_used;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> new file mode 100644
> index 00000000000..4833b50d48e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> @@ -0,0 +1,17 @@
> +// PR c++/107085
> +// { dg-do compile { target c++11 } }
> +
> +// Must be non-trivial to exhibit the ICE.
> +struct X {
> +  X();
> +  X(X&&);
> +};
> +struct Z : X {};
> +X x1 = Z();
> +X x2 = X(Z());
> +
> +// ...but let's try the trivial path in build_over_call as well.
> +struct B { };
> +struct D : B { };
> +B b1 = D();
> +B b2 = B(D());
> diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> index 76de905a35d..5354b1dc4e6 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> @@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
>   SA(!__reference_constructs_from_temporary(int&&, G2));
>   SA(!__reference_constructs_from_temporary(const int&, H2));
>   
> -SA(!__reference_constructs_from_temporary(const Base&, Der));
> +SA(__reference_constructs_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
> diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> index 90196c38742..e6c159e9b00 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> @@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
>   SA(!__reference_converts_from_temporary(int&&, G2));
>   SA(!__reference_converts_from_temporary(const int&, H2));
>   
> -SA(!__reference_converts_from_temporary(const Base&, Der));
> +SA(__reference_converts_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_converts_from_temporary(int&&, id<int[3]>));
> 
> base-commit: e99dcbb54e07b798c3353124f38336f96a826d43


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06  0:25 ` Jason Merrill
@ 2022-10-06 14:49   ` Marek Polacek
  2022-10-06 14:58     ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-06 14:49 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
> On 10/5/22 17:27, Marek Polacek wrote:
> > This PR reports that
> > 
> >    struct Base {};
> >    struct Derived : Base {};
> >    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> > 
> > doesn't pass, which it should: it's just like
> > 
> >    const Base& b(Derived{});
> > 
> > where we bind 'b' to the Base subobject of a temporary object of type
> > Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> > we didn't need to create a temporary object just for the base, but the whole
> > object is a temporary so we're still binding to a temporary.  Fixed by
> > the conv_is_prvalue hunk.
> > 
> > That broke a bunch of tests.  I've distilled the issue into a simple test
> > in elision4.C.  Essentially, we have
> > 
> >    struct B { /* ... */ };
> >    struct D : B { };
> >    B b = D();
> > 
> > and we set force_elide in build_over_call, but we're unable to actually
> > elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
> > 
> > <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
> > elision "can only apply when the object being initialized is known not to be
> > a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
> > the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
> > derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
> > checking ck_base at all.
> > 
> > Does that make sense?  If so...
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > 	PR c++/107085
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
> > 	of a temporary object.
> 
> No, the base subobject of a prvalue is an xvalue.

Ah, so this is just like T().m where T() is a prvalue but the whole thing
is an xvalue.  Duly noted.
 
> I think the problem is that an expression being a prvalue is a subset of
> binding a reference to a temporary, and we shouldn't try to express both of
> those using the same function: you need a separate
> conv_binds_ref_to_temporary.

Ack, so how about this?  Thanks,

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

-- >8 --
This PR reports that

  struct Base {};
  struct Derived : Base {};
  static_assert(__reference_constructs_from_temporary(Base const&, Derived));

doesn't pass, which it should: it's just like

  const Base& b(Derived{});

where we bind 'b' to the Base subobject of a temporary object of type
Derived.  The ck_base conversion didn't have ->need_temporary_p set because
we didn't need to create a temporary object just for the base, but the whole
object is a temporary so we're still binding to a temporary.  Since the
Base subobject is an xvalue, a new function is introduced.

	PR c++/107085

gcc/cp/ChangeLog:

	* call.cc (conv_binds_ref_to_temporary): New.
	(ref_conv_binds_directly): Use it.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
	result.
	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
	* g++.dg/cpp0x/elision4.C: New test.
---
 gcc/cp/call.cc                                | 23 ++++++++++++++++++-
 gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
 .../reference_constructs_from_temporary1.C    |  2 +-
 .../ext/reference_converts_from_temporary1.C  |  2 +-
 4 files changed, 39 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd04a1d309a..715a83f5a69 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
   return conv_is_prvalue (next_conversion (c));
 }
 
+/* True iff C is a conversion that binds a reference to a temporary.
+   This is a superset of conv_binds_ref_to_prvalue: here we're also
+   interested in xvalues.  */
+
+static bool
+conv_binds_ref_to_temporary (conversion *c)
+{
+  if (conv_binds_ref_to_prvalue (c))
+    return true;
+  if (c->kind != ck_ref_bind)
+    return false;
+  c = next_conversion (c);
+  /* This is the case for
+       struct Base {};
+       struct Derived : Base {};
+       const Base& b(Derived{});
+     where we bind 'b' to the Base subobject of a temporary object of type
+     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
+  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
+}
+
 /* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
    not involve creating a temporary.  Return tristate::TS_FALSE if converting
    EXPR to a reference type TYPE binds the reference to a temporary.  If the
@@ -9230,7 +9251,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
 					  /*c_cast_p=*/false, flags, tf_none);
   tristate ret (tristate::TS_UNKNOWN);
   if (conv && !conv->bad_p)
-    ret = tristate (!conv_binds_ref_to_prvalue (conv));
+    ret = tristate (!conv_binds_ref_to_temporary (conv));
 
   /* Free all the conversions we allocated.  */
   obstack_free (&conversion_obstack, p);
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
new file mode 100644
index 00000000000..3cc2e3afa5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
@@ -0,0 +1,15 @@
+// PR c++/107085
+// { dg-do compile { target c++11 } }
+
+struct X {
+  X();
+  X(X&&);
+};
+struct Z : X {};
+X x1 = Z();
+X x2 = X(Z());
+
+struct B { };
+struct D : B { };
+B b1 = D();
+B b2 = B(D());
diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
index 76de905a35d..5354b1dc4e6 100644
--- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
@@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
 SA(!__reference_constructs_from_temporary(int&&, G2));
 SA(!__reference_constructs_from_temporary(const int&, H2));
 
-SA(!__reference_constructs_from_temporary(const Base&, Der));
+SA(__reference_constructs_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
index 90196c38742..e6c159e9b00 100644
--- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
@@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
 SA(!__reference_converts_from_temporary(int&&, G2));
 SA(!__reference_converts_from_temporary(const int&, H2));
 
-SA(!__reference_converts_from_temporary(const Base&, Der));
+SA(__reference_converts_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_converts_from_temporary(int&&, id<int[3]>));

base-commit: 3ec926d36fbf7cb3ff45759471139f3a71d1c4de
-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 14:49   ` [PATCH v2] " Marek Polacek
@ 2022-10-06 14:58     ` Jason Merrill
  2022-10-06 17:51       ` Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2022-10-06 14:58 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 10/6/22 10:49, Marek Polacek wrote:
> On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
>> On 10/5/22 17:27, Marek Polacek wrote:
>>> This PR reports that
>>>
>>>     struct Base {};
>>>     struct Derived : Base {};
>>>     static_assert(__reference_constructs_from_temporary(Base const&, Derived));
>>>
>>> doesn't pass, which it should: it's just like
>>>
>>>     const Base& b(Derived{});
>>>
>>> where we bind 'b' to the Base subobject of a temporary object of type
>>> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
>>> we didn't need to create a temporary object just for the base, but the whole
>>> object is a temporary so we're still binding to a temporary.  Fixed by
>>> the conv_is_prvalue hunk.
>>>
>>> That broke a bunch of tests.  I've distilled the issue into a simple test
>>> in elision4.C.  Essentially, we have
>>>
>>>     struct B { /* ... */ };
>>>     struct D : B { };
>>>     B b = D();
>>>
>>> and we set force_elide in build_over_call, but we're unable to actually
>>> elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
>>>
>>> <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
>>> elision "can only apply when the object being initialized is known not to be
>>> a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
>>> the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
>>> derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
>>> checking ck_base at all.
>>>
>>> Does that make sense?  If so...
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> 	PR c++/107085
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
>>> 	of a temporary object.
>>
>> No, the base subobject of a prvalue is an xvalue.
> 
> Ah, so this is just like T().m where T() is a prvalue but the whole thing
> is an xvalue.  Duly noted.

Exactly.

>> I think the problem is that an expression being a prvalue is a subset of
>> binding a reference to a temporary, and we shouldn't try to express both of
>> those using the same function: you need a separate
>> conv_binds_ref_to_temporary.
> 
> Ack, so how about this?  Thanks,
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> This PR reports that
> 
>    struct Base {};
>    struct Derived : Base {};
>    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> 
> doesn't pass, which it should: it's just like
> 
>    const Base& b(Derived{});
> 
> where we bind 'b' to the Base subobject of a temporary object of type
> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> we didn't need to create a temporary object just for the base, but the whole
> object is a temporary so we're still binding to a temporary.  Since the
> Base subobject is an xvalue, a new function is introduced.
> 
> 	PR c++/107085
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (conv_binds_ref_to_temporary): New.
> 	(ref_conv_binds_directly): Use it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> 	result.
> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> 	* g++.dg/cpp0x/elision4.C: New test.
> ---
>   gcc/cp/call.cc                                | 23 ++++++++++++++++++-
>   gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
>   .../reference_constructs_from_temporary1.C    |  2 +-
>   .../ext/reference_converts_from_temporary1.C  |  2 +-
>   4 files changed, 39 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd04a1d309a..715a83f5a69 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
>     return conv_is_prvalue (next_conversion (c));
>   }
>   
> +/* True iff C is a conversion that binds a reference to a temporary.
> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> +   interested in xvalues.  */
> +
> +static bool
> +conv_binds_ref_to_temporary (conversion *c)
> +{
> +  if (conv_binds_ref_to_prvalue (c))
> +    return true;
> +  if (c->kind != ck_ref_bind)
> +    return false;
> +  c = next_conversion (c);
> +  /* This is the case for
> +       struct Base {};
> +       struct Derived : Base {};
> +       const Base& b(Derived{});
> +     where we bind 'b' to the Base subobject of a temporary object of type
> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));

I think you also want to check for the case of c->u.expr being a 
COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.

> +}
> +
>   /* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
>      not involve creating a temporary.  Return tristate::TS_FALSE if converting
>      EXPR to a reference type TYPE binds the reference to a temporary.  If the
> @@ -9230,7 +9251,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
>   					  /*c_cast_p=*/false, flags, tf_none);
>     tristate ret (tristate::TS_UNKNOWN);
>     if (conv && !conv->bad_p)
> -    ret = tristate (!conv_binds_ref_to_prvalue (conv));
> +    ret = tristate (!conv_binds_ref_to_temporary (conv));
>   
>     /* Free all the conversions we allocated.  */
>     obstack_free (&conversion_obstack, p);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> new file mode 100644
> index 00000000000..3cc2e3afa5d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> @@ -0,0 +1,15 @@
> +// PR c++/107085
> +// { dg-do compile { target c++11 } }
> +
> +struct X {
> +  X();
> +  X(X&&);
> +};
> +struct Z : X {};
> +X x1 = Z();
> +X x2 = X(Z());
> +
> +struct B { };
> +struct D : B { };
> +B b1 = D();
> +B b2 = B(D());
> diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> index 76de905a35d..5354b1dc4e6 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> @@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
>   SA(!__reference_constructs_from_temporary(int&&, G2));
>   SA(!__reference_constructs_from_temporary(const int&, H2));
>   
> -SA(!__reference_constructs_from_temporary(const Base&, Der));
> +SA(__reference_constructs_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
> diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> index 90196c38742..e6c159e9b00 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> @@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
>   SA(!__reference_converts_from_temporary(int&&, G2));
>   SA(!__reference_converts_from_temporary(const int&, H2));
>   
> -SA(!__reference_converts_from_temporary(const Base&, Der));
> +SA(__reference_converts_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_converts_from_temporary(int&&, id<int[3]>));
> 
> base-commit: 3ec926d36fbf7cb3ff45759471139f3a71d1c4de


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 14:58     ` Jason Merrill
@ 2022-10-06 17:51       ` Marek Polacek
  2022-10-06 18:00         ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-06 17:51 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
> On 10/6/22 10:49, Marek Polacek wrote:
> > On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
> > > On 10/5/22 17:27, Marek Polacek wrote:
> > > > This PR reports that
> > > > 
> > > >     struct Base {};
> > > >     struct Derived : Base {};
> > > >     static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> > > > 
> > > > doesn't pass, which it should: it's just like
> > > > 
> > > >     const Base& b(Derived{});
> > > > 
> > > > where we bind 'b' to the Base subobject of a temporary object of type
> > > > Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> > > > we didn't need to create a temporary object just for the base, but the whole
> > > > object is a temporary so we're still binding to a temporary.  Fixed by
> > > > the conv_is_prvalue hunk.
> > > > 
> > > > That broke a bunch of tests.  I've distilled the issue into a simple test
> > > > in elision4.C.  Essentially, we have
> > > > 
> > > >     struct B { /* ... */ };
> > > >     struct D : B { };
> > > >     B b = D();
> > > > 
> > > > and we set force_elide in build_over_call, but we're unable to actually
> > > > elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
> > > > 
> > > > <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
> > > > elision "can only apply when the object being initialized is known not to be
> > > > a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
> > > > the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
> > > > derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
> > > > checking ck_base at all.
> > > > 
> > > > Does that make sense?  If so...
> > > > 
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > 	PR c++/107085
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
> > > > 	of a temporary object.
> > > 
> > > No, the base subobject of a prvalue is an xvalue.
> > 
> > Ah, so this is just like T().m where T() is a prvalue but the whole thing
> > is an xvalue.  Duly noted.
> 
> Exactly.
> 
> > > I think the problem is that an expression being a prvalue is a subset of
> > > binding a reference to a temporary, and we shouldn't try to express both of
> > > those using the same function: you need a separate
> > > conv_binds_ref_to_temporary.
> > 
> > Ack, so how about this?  Thanks,
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > This PR reports that
> > 
> >    struct Base {};
> >    struct Derived : Base {};
> >    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> > 
> > doesn't pass, which it should: it's just like
> > 
> >    const Base& b(Derived{});
> > 
> > where we bind 'b' to the Base subobject of a temporary object of type
> > Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> > we didn't need to create a temporary object just for the base, but the whole
> > object is a temporary so we're still binding to a temporary.  Since the
> > Base subobject is an xvalue, a new function is introduced.
> > 
> > 	PR c++/107085
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* call.cc (conv_binds_ref_to_temporary): New.
> > 	(ref_conv_binds_directly): Use it.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> > 	result.
> > 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> > 	* g++.dg/cpp0x/elision4.C: New test.
> > ---
> >   gcc/cp/call.cc                                | 23 ++++++++++++++++++-
> >   gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
> >   .../reference_constructs_from_temporary1.C    |  2 +-
> >   .../ext/reference_converts_from_temporary1.C  |  2 +-
> >   4 files changed, 39 insertions(+), 3 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> > 
> > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > index bd04a1d309a..715a83f5a69 100644
> > --- a/gcc/cp/call.cc
> > +++ b/gcc/cp/call.cc
> > @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
> >     return conv_is_prvalue (next_conversion (c));
> >   }
> > +/* True iff C is a conversion that binds a reference to a temporary.
> > +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> > +   interested in xvalues.  */
> > +
> > +static bool
> > +conv_binds_ref_to_temporary (conversion *c)
> > +{
> > +  if (conv_binds_ref_to_prvalue (c))
> > +    return true;
> > +  if (c->kind != ck_ref_bind)
> > +    return false;
> > +  c = next_conversion (c);
> > +  /* This is the case for
> > +       struct Base {};
> > +       struct Derived : Base {};
> > +       const Base& b(Derived{});
> > +     where we bind 'b' to the Base subobject of a temporary object of type
> > +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> > +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
> 
> I think you also want to check for the case of c->u.expr being a
> COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.

I see.  So this would be achieved using e.g.

struct B { };
struct D : B { };
struct C {
  D d;
};

const B& b = C{}.d;

Except I'm not sure how to trigger this via the built-in, which takes two types.
Am I missing something obvious?

Marek


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 17:51       ` Marek Polacek
@ 2022-10-06 18:00         ` Jason Merrill
  2022-10-06 21:43           ` [PATCH v3] " Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2022-10-06 18:00 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 10/6/22 13:51, Marek Polacek wrote:
> On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
>> On 10/6/22 10:49, Marek Polacek wrote:
>>> On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
>>>> On 10/5/22 17:27, Marek Polacek wrote:
>>>>> This PR reports that
>>>>>
>>>>>      struct Base {};
>>>>>      struct Derived : Base {};
>>>>>      static_assert(__reference_constructs_from_temporary(Base const&, Derived));
>>>>>
>>>>> doesn't pass, which it should: it's just like
>>>>>
>>>>>      const Base& b(Derived{});
>>>>>
>>>>> where we bind 'b' to the Base subobject of a temporary object of type
>>>>> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
>>>>> we didn't need to create a temporary object just for the base, but the whole
>>>>> object is a temporary so we're still binding to a temporary.  Fixed by
>>>>> the conv_is_prvalue hunk.
>>>>>
>>>>> That broke a bunch of tests.  I've distilled the issue into a simple test
>>>>> in elision4.C.  Essentially, we have
>>>>>
>>>>>      struct B { /* ... */ };
>>>>>      struct D : B { };
>>>>>      B b = D();
>>>>>
>>>>> and we set force_elide in build_over_call, but we're unable to actually
>>>>> elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
>>>>>
>>>>> <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
>>>>> elision "can only apply when the object being initialized is known not to be
>>>>> a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
>>>>> the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
>>>>> derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
>>>>> checking ck_base at all.
>>>>>
>>>>> Does that make sense?  If so...
>>>>>
>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>>
>>>>> 	PR c++/107085
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
>>>>> 	of a temporary object.
>>>>
>>>> No, the base subobject of a prvalue is an xvalue.
>>>
>>> Ah, so this is just like T().m where T() is a prvalue but the whole thing
>>> is an xvalue.  Duly noted.
>>
>> Exactly.
>>
>>>> I think the problem is that an expression being a prvalue is a subset of
>>>> binding a reference to a temporary, and we shouldn't try to express both of
>>>> those using the same function: you need a separate
>>>> conv_binds_ref_to_temporary.
>>>
>>> Ack, so how about this?  Thanks,
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> -- >8 --
>>> This PR reports that
>>>
>>>     struct Base {};
>>>     struct Derived : Base {};
>>>     static_assert(__reference_constructs_from_temporary(Base const&, Derived));
>>>
>>> doesn't pass, which it should: it's just like
>>>
>>>     const Base& b(Derived{});
>>>
>>> where we bind 'b' to the Base subobject of a temporary object of type
>>> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
>>> we didn't need to create a temporary object just for the base, but the whole
>>> object is a temporary so we're still binding to a temporary.  Since the
>>> Base subobject is an xvalue, a new function is introduced.
>>>
>>> 	PR c++/107085
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* call.cc (conv_binds_ref_to_temporary): New.
>>> 	(ref_conv_binds_directly): Use it.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
>>> 	result.
>>> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
>>> 	* g++.dg/cpp0x/elision4.C: New test.
>>> ---
>>>    gcc/cp/call.cc                                | 23 ++++++++++++++++++-
>>>    gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
>>>    .../reference_constructs_from_temporary1.C    |  2 +-
>>>    .../ext/reference_converts_from_temporary1.C  |  2 +-
>>>    4 files changed, 39 insertions(+), 3 deletions(-)
>>>    create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
>>>
>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>> index bd04a1d309a..715a83f5a69 100644
>>> --- a/gcc/cp/call.cc
>>> +++ b/gcc/cp/call.cc
>>> @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
>>>      return conv_is_prvalue (next_conversion (c));
>>>    }
>>> +/* True iff C is a conversion that binds a reference to a temporary.
>>> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
>>> +   interested in xvalues.  */
>>> +
>>> +static bool
>>> +conv_binds_ref_to_temporary (conversion *c)
>>> +{
>>> +  if (conv_binds_ref_to_prvalue (c))
>>> +    return true;
>>> +  if (c->kind != ck_ref_bind)
>>> +    return false;
>>> +  c = next_conversion (c);
>>> +  /* This is the case for
>>> +       struct Base {};
>>> +       struct Derived : Base {};
>>> +       const Base& b(Derived{});
>>> +     where we bind 'b' to the Base subobject of a temporary object of type
>>> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
>>> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
>>
>> I think you also want to check for the case of c->u.expr being a
>> COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
> 
> I see.  So this would be achieved using e.g.
> 
> struct B { };
> struct D : B { };
> struct C {
>    D d;
> };
> 
> const B& b = C{}.d;

Yes.

> Except I'm not sure how to trigger this via the built-in, which takes two types.
> Am I missing something obvious?

Indeed, it can't be triggered by the built-in.  But I see 
ref_conv_binds_directly is also called from warn_for_range_copy, which 
ought to be able to trigger it.

Incidentally, ref_conv_binds_directly should also probably be reversed 
to ref_conv_binds_to_temporary since you can "bind directly" to an 
xvalue that refers to a temporary.

Jason


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v3] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 18:00         ` Jason Merrill
@ 2022-10-06 21:43           ` Marek Polacek
  2022-10-06 22:03             ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-06 21:43 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
> On 10/6/22 13:51, Marek Polacek wrote:
> > On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
> > > On 10/6/22 10:49, Marek Polacek wrote:
> > > > On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
> > > > > On 10/5/22 17:27, Marek Polacek wrote:
> > > > > > This PR reports that
> > > > > > 
> > > > > >      struct Base {};
> > > > > >      struct Derived : Base {};
> > > > > >      static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> > > > > > 
> > > > > > doesn't pass, which it should: it's just like
> > > > > > 
> > > > > >      const Base& b(Derived{});
> > > > > > 
> > > > > > where we bind 'b' to the Base subobject of a temporary object of type
> > > > > > Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> > > > > > we didn't need to create a temporary object just for the base, but the whole
> > > > > > object is a temporary so we're still binding to a temporary.  Fixed by
> > > > > > the conv_is_prvalue hunk.
> > > > > > 
> > > > > > That broke a bunch of tests.  I've distilled the issue into a simple test
> > > > > > in elision4.C.  Essentially, we have
> > > > > > 
> > > > > >      struct B { /* ... */ };
> > > > > >      struct D : B { };
> > > > > >      B b = D();
> > > > > > 
> > > > > > and we set force_elide in build_over_call, but we're unable to actually
> > > > > > elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
> > > > > > 
> > > > > > <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
> > > > > > elision "can only apply when the object being initialized is known not to be
> > > > > > a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
> > > > > > the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
> > > > > > derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
> > > > > > checking ck_base at all.
> > > > > > 
> > > > > > Does that make sense?  If so...
> > > > > > 
> > > > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > > > 
> > > > > > 	PR c++/107085
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
> > > > > > 	of a temporary object.
> > > > > 
> > > > > No, the base subobject of a prvalue is an xvalue.
> > > > 
> > > > Ah, so this is just like T().m where T() is a prvalue but the whole thing
> > > > is an xvalue.  Duly noted.
> > > 
> > > Exactly.
> > > 
> > > > > I think the problem is that an expression being a prvalue is a subset of
> > > > > binding a reference to a temporary, and we shouldn't try to express both of
> > > > > those using the same function: you need a separate
> > > > > conv_binds_ref_to_temporary.
> > > > 
> > > > Ack, so how about this?  Thanks,
> > > > 
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > -- >8 --
> > > > This PR reports that
> > > > 
> > > >     struct Base {};
> > > >     struct Derived : Base {};
> > > >     static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> > > > 
> > > > doesn't pass, which it should: it's just like
> > > > 
> > > >     const Base& b(Derived{});
> > > > 
> > > > where we bind 'b' to the Base subobject of a temporary object of type
> > > > Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> > > > we didn't need to create a temporary object just for the base, but the whole
> > > > object is a temporary so we're still binding to a temporary.  Since the
> > > > Base subobject is an xvalue, a new function is introduced.
> > > > 
> > > > 	PR c++/107085
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	* call.cc (conv_binds_ref_to_temporary): New.
> > > > 	(ref_conv_binds_directly): Use it.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> > > > 	result.
> > > > 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> > > > 	* g++.dg/cpp0x/elision4.C: New test.
> > > > ---
> > > >    gcc/cp/call.cc                                | 23 ++++++++++++++++++-
> > > >    gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
> > > >    .../reference_constructs_from_temporary1.C    |  2 +-
> > > >    .../ext/reference_converts_from_temporary1.C  |  2 +-
> > > >    4 files changed, 39 insertions(+), 3 deletions(-)
> > > >    create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> > > > 
> > > > diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> > > > index bd04a1d309a..715a83f5a69 100644
> > > > --- a/gcc/cp/call.cc
> > > > +++ b/gcc/cp/call.cc
> > > > @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
> > > >      return conv_is_prvalue (next_conversion (c));
> > > >    }
> > > > +/* True iff C is a conversion that binds a reference to a temporary.
> > > > +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> > > > +   interested in xvalues.  */
> > > > +
> > > > +static bool
> > > > +conv_binds_ref_to_temporary (conversion *c)
> > > > +{
> > > > +  if (conv_binds_ref_to_prvalue (c))
> > > > +    return true;
> > > > +  if (c->kind != ck_ref_bind)
> > > > +    return false;
> > > > +  c = next_conversion (c);
> > > > +  /* This is the case for
> > > > +       struct Base {};
> > > > +       struct Derived : Base {};
> > > > +       const Base& b(Derived{});
> > > > +     where we bind 'b' to the Base subobject of a temporary object of type
> > > > +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> > > > +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
> > > 
> > > I think you also want to check for the case of c->u.expr being a
> > > COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
> > 
> > I see.  So this would be achieved using e.g.
> > 
> > struct B { };
> > struct D : B { };
> > struct C {
> >    D d;
> > };
> > 
> > const B& b = C{}.d;
> 
> Yes.
> 
> > Except I'm not sure how to trigger this via the built-in, which takes two types.
> > Am I missing something obvious?
> 
> Indeed, it can't be triggered by the built-in.  But I see
> ref_conv_binds_directly is also called from warn_for_range_copy, which ought
> to be able to trigger it.

Even there, I think, it's not possible to trigger it with, say (d is an
array of D):

     for (const B &b : C{}.d)
       // ...

because warn_for_range_copy gets *__for_begin as the EXPR, which means that
we won't get to the TARGET_EXPR.  It's like

  auto&& __for_begin = C{}.d;
  const B &b = *__for_begin;

and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
original .u.expr is *__for_begin.

I could add some checking assert to conv_binds_ref_to_temporary to see if
we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...

> Incidentally, ref_conv_binds_directly should also probably be reversed to
> ref_conv_binds_to_temporary since you can "bind directly" to an xvalue that
> refers to a temporary.

Good point, I at least did that.

-- >8 --
This PR reports that

  struct Base {};
  struct Derived : Base {};
  static_assert(__reference_constructs_from_temporary(Base const&, Derived));

doesn't pass, which it should: it's just like

  const Base& b(Derived{});

where we bind 'b' to the Base subobject of a temporary object of type
Derived.  The ck_base conversion didn't have ->need_temporary_p set because
we didn't need to create a temporary object just for the base, but the whole
object is a temporary so we're still binding to a temporary.  Since the
Base subobject is an xvalue, a new function is introduced.

	PR c++/107085

gcc/cp/ChangeLog:

	* call.cc (conv_binds_ref_to_temporary): New.
	(ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.  Use
	conv_binds_ref_to_temporary.
	* cp-tree.h (ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.
	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
	* parser.cc (warn_for_range_copy): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
	result.
	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
	* g++.dg/cpp0x/elision4.C: New test.
---
 gcc/cp/call.cc                                | 33 +++++++++++++++----
 gcc/cp/cp-tree.h                              |  2 +-
 gcc/cp/method.cc                              |  2 +-
 gcc/cp/parser.cc                              |  5 +--
 gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++++
 .../reference_constructs_from_temporary1.C    |  2 +-
 .../ext/reference_converts_from_temporary1.C  |  2 +-
 7 files changed, 49 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd04a1d309a..5a3d509f6c8 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9210,15 +9210,36 @@ conv_binds_ref_to_prvalue (conversion *c)
   return conv_is_prvalue (next_conversion (c));
 }
 
-/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
-   not involve creating a temporary.  Return tristate::TS_FALSE if converting
-   EXPR to a reference type TYPE binds the reference to a temporary.  If the
-   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
+/* True iff C is a conversion that binds a reference to a temporary.
+   This is a superset of conv_binds_ref_to_prvalue: here we're also
+   interested in xvalues.  */
+
+static bool
+conv_binds_ref_to_temporary (conversion *c)
+{
+  if (conv_binds_ref_to_prvalue (c))
+    return true;
+  if (c->kind != ck_ref_bind)
+    return false;
+  c = next_conversion (c);
+  /* This is the case for
+       struct Base {};
+       struct Derived : Base {};
+       const Base& b(Derived{});
+     where we bind 'b' to the Base subobject of a temporary object of type
+     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
+  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
+}
+
+/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
+   the reference to a temporary.  Return tristate::TS_FALSE if converting
+   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
+   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
    says whether the conversion should be done in direct- or copy-initialization
    context.  */
 
 tristate
-ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
+ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
 {
   gcc_assert (TYPE_REF_P (type));
 
@@ -9230,7 +9251,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
 					  /*c_cast_p=*/false, flags, tf_none);
   tristate ret (tristate::TS_UNKNOWN);
   if (conv && !conv->bad_p)
-    ret = tristate (!conv_binds_ref_to_prvalue (conv));
+    ret = tristate (conv_binds_ref_to_temporary (conv));
 
   /* Free all the conversions we allocated.  */
   obstack_free (&conversion_obstack, p);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8bc1c2dc7fd..469eb2fdb25 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
 extern tree type_decays_to			(tree);
 extern tree extract_call_expr			(tree);
 extern tree build_trivial_dtor_call		(tree, bool = false);
-extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
+extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
 extern tree build_user_type_conversion		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 55af5c43c18..622e1b9802e 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
   tree val = build_stub_object (from);
   if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
     val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
-  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
+  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
 }
 
 /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 90e06f914fb..bceedab5cd6 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
 
   if (TYPE_REF_P (type))
     {
-      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
+      if (glvalue_p (expr)
+	  && ref_conv_binds_to_temporary (type, expr).is_true ())
 	{
 	  auto_diagnostic_group d;
 	  if (warning_at (loc, OPT_Wrange_loop_construct,
@@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
   /* If we can initialize a reference directly, suggest that to avoid the
      copy.  */
   tree rtype = cp_build_reference_type (type, /*rval*/false);
-  if (ref_conv_binds_directly (rtype, expr).is_true ())
+  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
     {
       auto_diagnostic_group d;
       if (warning_at (loc, OPT_Wrange_loop_construct,
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
new file mode 100644
index 00000000000..3cc2e3afa5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
@@ -0,0 +1,15 @@
+// PR c++/107085
+// { dg-do compile { target c++11 } }
+
+struct X {
+  X();
+  X(X&&);
+};
+struct Z : X {};
+X x1 = Z();
+X x2 = X(Z());
+
+struct B { };
+struct D : B { };
+B b1 = D();
+B b2 = B(D());
diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
index 76de905a35d..5354b1dc4e6 100644
--- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
@@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
 SA(!__reference_constructs_from_temporary(int&&, G2));
 SA(!__reference_constructs_from_temporary(const int&, H2));
 
-SA(!__reference_constructs_from_temporary(const Base&, Der));
+SA(__reference_constructs_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
index 90196c38742..e6c159e9b00 100644
--- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
@@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
 SA(!__reference_converts_from_temporary(int&&, G2));
 SA(!__reference_converts_from_temporary(const int&, H2));
 
-SA(!__reference_converts_from_temporary(const Base&, Der));
+SA(__reference_converts_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_converts_from_temporary(int&&, id<int[3]>));

base-commit: 50c35c691517291dbb77b1661761bc59950ba101
-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 21:43           ` [PATCH v3] " Marek Polacek
@ 2022-10-06 22:03             ` Jason Merrill
  2022-10-07 16:10               ` [PATCH v4] " Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2022-10-06 22:03 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 10/6/22 17:43, Marek Polacek wrote:
> On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
>> On 10/6/22 13:51, Marek Polacek wrote:
>>> On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
>>>> On 10/6/22 10:49, Marek Polacek wrote:
>>>>> On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
>>>>>> On 10/5/22 17:27, Marek Polacek wrote:
>>>>>>> This PR reports that
>>>>>>>
>>>>>>>       struct Base {};
>>>>>>>       struct Derived : Base {};
>>>>>>>       static_assert(__reference_constructs_from_temporary(Base const&, Derived));
>>>>>>>
>>>>>>> doesn't pass, which it should: it's just like
>>>>>>>
>>>>>>>       const Base& b(Derived{});
>>>>>>>
>>>>>>> where we bind 'b' to the Base subobject of a temporary object of type
>>>>>>> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
>>>>>>> we didn't need to create a temporary object just for the base, but the whole
>>>>>>> object is a temporary so we're still binding to a temporary.  Fixed by
>>>>>>> the conv_is_prvalue hunk.
>>>>>>>
>>>>>>> That broke a bunch of tests.  I've distilled the issue into a simple test
>>>>>>> in elision4.C.  Essentially, we have
>>>>>>>
>>>>>>>       struct B { /* ... */ };
>>>>>>>       struct D : B { };
>>>>>>>       B b = D();
>>>>>>>
>>>>>>> and we set force_elide in build_over_call, but we're unable to actually
>>>>>>> elide the B::B(B&&) call, and crash on gcc_assert (!force_elide);.
>>>>>>>
>>>>>>> <https://en.cppreference.com/w/cpp/language/copy_elision> says that copy
>>>>>>> elision "can only apply when the object being initialized is known not to be
>>>>>>> a potentially-overlapping subobject".  So I suppose we shouldn't force_elide
>>>>>>> the B::B(B&&) call.  I don't belive the CWG 2327 code was added to handle
>>>>>>> derived-to-base conversions, at that time conv_binds_ref_to_prvalue wasn't
>>>>>>> checking ck_base at all.
>>>>>>>
>>>>>>> Does that make sense?  If so...
>>>>>>>
>>>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>>>>
>>>>>>> 	PR c++/107085
>>>>>>>
>>>>>>> gcc/cp/ChangeLog:
>>>>>>>
>>>>>>> 	* call.cc (conv_is_prvalue): Return true if the base subobject is part
>>>>>>> 	of a temporary object.
>>>>>>
>>>>>> No, the base subobject of a prvalue is an xvalue.
>>>>>
>>>>> Ah, so this is just like T().m where T() is a prvalue but the whole thing
>>>>> is an xvalue.  Duly noted.
>>>>
>>>> Exactly.
>>>>
>>>>>> I think the problem is that an expression being a prvalue is a subset of
>>>>>> binding a reference to a temporary, and we shouldn't try to express both of
>>>>>> those using the same function: you need a separate
>>>>>> conv_binds_ref_to_temporary.
>>>>>
>>>>> Ack, so how about this?  Thanks,
>>>>>
>>>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>>>
>>>>> -- >8 --
>>>>> This PR reports that
>>>>>
>>>>>      struct Base {};
>>>>>      struct Derived : Base {};
>>>>>      static_assert(__reference_constructs_from_temporary(Base const&, Derived));
>>>>>
>>>>> doesn't pass, which it should: it's just like
>>>>>
>>>>>      const Base& b(Derived{});
>>>>>
>>>>> where we bind 'b' to the Base subobject of a temporary object of type
>>>>> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
>>>>> we didn't need to create a temporary object just for the base, but the whole
>>>>> object is a temporary so we're still binding to a temporary.  Since the
>>>>> Base subobject is an xvalue, a new function is introduced.
>>>>>
>>>>> 	PR c++/107085
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	* call.cc (conv_binds_ref_to_temporary): New.
>>>>> 	(ref_conv_binds_directly): Use it.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
>>>>> 	result.
>>>>> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
>>>>> 	* g++.dg/cpp0x/elision4.C: New test.
>>>>> ---
>>>>>     gcc/cp/call.cc                                | 23 ++++++++++++++++++-
>>>>>     gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 ++++++++++++
>>>>>     .../reference_constructs_from_temporary1.C    |  2 +-
>>>>>     .../ext/reference_converts_from_temporary1.C  |  2 +-
>>>>>     4 files changed, 39 insertions(+), 3 deletions(-)
>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
>>>>>
>>>>> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
>>>>> index bd04a1d309a..715a83f5a69 100644
>>>>> --- a/gcc/cp/call.cc
>>>>> +++ b/gcc/cp/call.cc
>>>>> @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
>>>>>       return conv_is_prvalue (next_conversion (c));
>>>>>     }
>>>>> +/* True iff C is a conversion that binds a reference to a temporary.
>>>>> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
>>>>> +   interested in xvalues.  */
>>>>> +
>>>>> +static bool
>>>>> +conv_binds_ref_to_temporary (conversion *c)
>>>>> +{
>>>>> +  if (conv_binds_ref_to_prvalue (c))
>>>>> +    return true;
>>>>> +  if (c->kind != ck_ref_bind)
>>>>> +    return false;
>>>>> +  c = next_conversion (c);
>>>>> +  /* This is the case for
>>>>> +       struct Base {};
>>>>> +       struct Derived : Base {};
>>>>> +       const Base& b(Derived{});
>>>>> +     where we bind 'b' to the Base subobject of a temporary object of type
>>>>> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
>>>>> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
>>>>
>>>> I think you also want to check for the case of c->u.expr being a
>>>> COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
>>>
>>> I see.  So this would be achieved using e.g.
>>>
>>> struct B { };
>>> struct D : B { };
>>> struct C {
>>>     D d;
>>> };
>>>
>>> const B& b = C{}.d;
>>
>> Yes.
>>
>>> Except I'm not sure how to trigger this via the built-in, which takes two types.
>>> Am I missing something obvious?
>>
>> Indeed, it can't be triggered by the built-in.  But I see
>> ref_conv_binds_directly is also called from warn_for_range_copy, which ought
>> to be able to trigger it.
> 
> Even there, I think, it's not possible to trigger it with, say (d is an
> array of D):
> 
>       for (const B &b : C{}.d)
>         // ...
> 
> because warn_for_range_copy gets *__for_begin as the EXPR, which means that
> we won't get to the TARGET_EXPR.  It's like
> 
>    auto&& __for_begin = C{}.d;
>    const B &b = *__for_begin;
> 
> and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
> original .u.expr is *__for_begin.

Ah, right, and so the C{} temporary gets lifetime-extended and there's 
no problem to warn about.

> I could add some checking assert to conv_binds_ref_to_temporary to see if
> we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...

Or try to handle it properly even if we can't exercise the code yet.

I'm thinking again about warning for e.g.

const B& b = frotz(C{}.d);

where frotz passes through its reference argument, which becomes a 
dangling reference.  Handling the C{}.d case seems relevant to such a 
warning.

I wonder how many false positives there would be for warning about this 
for any function that both takes and returns a reference?

Do you want to try that, or shall I?

>> Incidentally, ref_conv_binds_directly should also probably be reversed to
>> ref_conv_binds_to_temporary since you can "bind directly" to an xvalue that
>> refers to a temporary.
> 
> Good point, I at least did that.
> 
> -- >8 --
> This PR reports that
> 
>    struct Base {};
>    struct Derived : Base {};
>    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> 
> doesn't pass, which it should: it's just like
> 
>    const Base& b(Derived{});
> 
> where we bind 'b' to the Base subobject of a temporary object of type
> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> we didn't need to create a temporary object just for the base, but the whole
> object is a temporary so we're still binding to a temporary.  Since the
> Base subobject is an xvalue, a new function is introduced.
> 
> 	PR c++/107085
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (conv_binds_ref_to_temporary): New.
> 	(ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.  Use
> 	conv_binds_ref_to_temporary.
> 	* cp-tree.h (ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.
> 	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
> 	* parser.cc (warn_for_range_copy): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> 	result.
> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> 	* g++.dg/cpp0x/elision4.C: New test.
> ---
>   gcc/cp/call.cc                                | 33 +++++++++++++++----
>   gcc/cp/cp-tree.h                              |  2 +-
>   gcc/cp/method.cc                              |  2 +-
>   gcc/cp/parser.cc                              |  5 +--
>   gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++++
>   .../reference_constructs_from_temporary1.C    |  2 +-
>   .../ext/reference_converts_from_temporary1.C  |  2 +-
>   7 files changed, 49 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd04a1d309a..5a3d509f6c8 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9210,15 +9210,36 @@ conv_binds_ref_to_prvalue (conversion *c)
>     return conv_is_prvalue (next_conversion (c));
>   }
>   
> -/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
> -   not involve creating a temporary.  Return tristate::TS_FALSE if converting
> -   EXPR to a reference type TYPE binds the reference to a temporary.  If the
> -   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
> +/* True iff C is a conversion that binds a reference to a temporary.
> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> +   interested in xvalues.  */
> +
> +static bool
> +conv_binds_ref_to_temporary (conversion *c)
> +{
> +  if (conv_binds_ref_to_prvalue (c))
> +    return true;
> +  if (c->kind != ck_ref_bind)
> +    return false;
> +  c = next_conversion (c);
> +  /* This is the case for
> +       struct Base {};
> +       struct Derived : Base {};
> +       const Base& b(Derived{});
> +     where we bind 'b' to the Base subobject of a temporary object of type
> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
> +}
> +
> +/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
> +   the reference to a temporary.  Return tristate::TS_FALSE if converting
> +   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
> +   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
>      says whether the conversion should be done in direct- or copy-initialization
>      context.  */
>   
>   tristate
> -ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
> +ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
>   {
>     gcc_assert (TYPE_REF_P (type));
>   
> @@ -9230,7 +9251,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
>   					  /*c_cast_p=*/false, flags, tf_none);
>     tristate ret (tristate::TS_UNKNOWN);
>     if (conv && !conv->bad_p)
> -    ret = tristate (!conv_binds_ref_to_prvalue (conv));
> +    ret = tristate (conv_binds_ref_to_temporary (conv));
>   
>     /* Free all the conversions we allocated.  */
>     obstack_free (&conversion_obstack, p);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 8bc1c2dc7fd..469eb2fdb25 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
>   extern tree type_decays_to			(tree);
>   extern tree extract_call_expr			(tree);
>   extern tree build_trivial_dtor_call		(tree, bool = false);
> -extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
> +extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
>   extern tree build_user_type_conversion		(tree, tree, int,
>   						 tsubst_flags_t);
>   extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 55af5c43c18..622e1b9802e 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
>     tree val = build_stub_object (from);
>     if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
>       val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
> -  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
> +  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
>   }
>   
>   /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 90e06f914fb..bceedab5cd6 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
>   
>     if (TYPE_REF_P (type))
>       {
> -      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
> +      if (glvalue_p (expr)
> +	  && ref_conv_binds_to_temporary (type, expr).is_true ())
>   	{
>   	  auto_diagnostic_group d;
>   	  if (warning_at (loc, OPT_Wrange_loop_construct,
> @@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
>     /* If we can initialize a reference directly, suggest that to avoid the
>        copy.  */
>     tree rtype = cp_build_reference_type (type, /*rval*/false);
> -  if (ref_conv_binds_directly (rtype, expr).is_true ())
> +  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
>       {
>         auto_diagnostic_group d;
>         if (warning_at (loc, OPT_Wrange_loop_construct,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> new file mode 100644
> index 00000000000..3cc2e3afa5d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> @@ -0,0 +1,15 @@
> +// PR c++/107085
> +// { dg-do compile { target c++11 } }
> +
> +struct X {
> +  X();
> +  X(X&&);
> +};
> +struct Z : X {};
> +X x1 = Z();
> +X x2 = X(Z());
> +
> +struct B { };
> +struct D : B { };
> +B b1 = D();
> +B b2 = B(D());
> diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> index 76de905a35d..5354b1dc4e6 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> @@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
>   SA(!__reference_constructs_from_temporary(int&&, G2));
>   SA(!__reference_constructs_from_temporary(const int&, H2));
>   
> -SA(!__reference_constructs_from_temporary(const Base&, Der));
> +SA(__reference_constructs_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
> diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> index 90196c38742..e6c159e9b00 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> @@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
>   SA(!__reference_converts_from_temporary(int&&, G2));
>   SA(!__reference_converts_from_temporary(const int&, H2));
>   
> -SA(!__reference_converts_from_temporary(const Base&, Der));
> +SA(__reference_converts_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_converts_from_temporary(int&&, id<int[3]>));
> 
> base-commit: 50c35c691517291dbb77b1661761bc59950ba101


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-06 22:03             ` Jason Merrill
@ 2022-10-07 16:10               ` Marek Polacek
  2022-10-07 17:01                 ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-07 16:10 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Thu, Oct 06, 2022 at 06:03:57PM -0400, Jason Merrill wrote:
> On 10/6/22 17:43, Marek Polacek wrote:
> > On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
> > > On 10/6/22 13:51, Marek Polacek wrote:
> > > > On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
> > > > > On 10/6/22 10:49, Marek Polacek wrote:
> > > > > > On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
> > > > > > > On 10/5/22 17:27, Marek Polacek wrote:
> > > > > > --- a/gcc/cp/call.cc
> > > > > > +++ b/gcc/cp/call.cc
> > > > > > @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
> > > > > >       return conv_is_prvalue (next_conversion (c));
> > > > > >     }
> > > > > > +/* True iff C is a conversion that binds a reference to a temporary.
> > > > > > +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> > > > > > +   interested in xvalues.  */
> > > > > > +
> > > > > > +static bool
> > > > > > +conv_binds_ref_to_temporary (conversion *c)
> > > > > > +{
> > > > > > +  if (conv_binds_ref_to_prvalue (c))
> > > > > > +    return true;
> > > > > > +  if (c->kind != ck_ref_bind)
> > > > > > +    return false;
> > > > > > +  c = next_conversion (c);
> > > > > > +  /* This is the case for
> > > > > > +       struct Base {};
> > > > > > +       struct Derived : Base {};
> > > > > > +       const Base& b(Derived{});
> > > > > > +     where we bind 'b' to the Base subobject of a temporary object of type
> > > > > > +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> > > > > > +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
> > > > > 
> > > > > I think you also want to check for the case of c->u.expr being a
> > > > > COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
> > > > 
> > > > I see.  So this would be achieved using e.g.
> > > > 
> > > > struct B { };
> > > > struct D : B { };
> > > > struct C {
> > > >     D d;
> > > > };
> > > > 
> > > > const B& b = C{}.d;
> > > 
> > > Yes.
> > > 
> > > > Except I'm not sure how to trigger this via the built-in, which takes two types.
> > > > Am I missing something obvious?
> > > 
> > > Indeed, it can't be triggered by the built-in.  But I see
> > > ref_conv_binds_directly is also called from warn_for_range_copy, which ought
> > > to be able to trigger it.
> > 
> > Even there, I think, it's not possible to trigger it with, say (d is an
> > array of D):
> > 
> >       for (const B &b : C{}.d)
> >         // ...
> > 
> > because warn_for_range_copy gets *__for_begin as the EXPR, which means that
> > we won't get to the TARGET_EXPR.  It's like
> > 
> >    auto&& __for_begin = C{}.d;
> >    const B &b = *__for_begin;
> > 
> > and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
> > original .u.expr is *__for_begin.
> 
> Ah, right, and so the C{} temporary gets lifetime-extended and there's no
> problem to warn about.
> 
> > I could add some checking assert to conv_binds_ref_to_temporary to see if
> > we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...
> 
> Or try to handle it properly even if we can't exercise the code yet.

Okay, done.
 
> I'm thinking again about warning for e.g.
> 
> const B& b = frotz(C{}.d);
> 
> where frotz passes through its reference argument, which becomes a dangling
> reference.  Handling the C{}.d case seems relevant to such a warning.
> 
> I wonder how many false positives there would be for warning about this for
> any function that both takes and returns a reference?

I suppose we'd have to ask if any (or all?) of the parameters is/are
a reference, to handle std::max?

> Do you want to try that, or shall I?

So this is like the first example in https://gcc.gnu.org/PR106393, which
is assigned to...me.  Let me take a stab at the simpler example.  If I
can't make any progress within, say, a week, I'll unassign so as not to
block it.

Meanwhile, here's the updated patch:

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

-- >8 --
This PR reports that

  struct Base {};
  struct Derived : Base {};
  static_assert(__reference_constructs_from_temporary(Base const&, Derived));

doesn't pass, which it should: it's just like

  const Base& b(Derived{});

where we bind 'b' to the Base subobject of a temporary object of type
Derived.  The ck_base conversion didn't have ->need_temporary_p set because
we didn't need to create a temporary object just for the base, but the whole
object is a temporary so we're still binding to a temporary.  Since the
Base subobject is an xvalue, a new function is introduced.

	PR c++/107085

gcc/cp/ChangeLog:

	* call.cc (conv_binds_ref_to_temporary): New.
	(ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.  Use
	conv_binds_ref_to_temporary.
	* cp-tree.h (ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.
	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
	* parser.cc (warn_for_range_copy): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
	result.
	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
	* g++.dg/cpp0x/elision4.C: New test.
---
 gcc/cp/call.cc                                | 45 ++++++++++++++++---
 gcc/cp/cp-tree.h                              |  2 +-
 gcc/cp/method.cc                              |  2 +-
 gcc/cp/parser.cc                              |  5 ++-
 gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++
 .../reference_constructs_from_temporary1.C    |  2 +-
 .../ext/reference_converts_from_temporary1.C  |  2 +-
 7 files changed, 61 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd04a1d309a..833cd014b7d 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9210,15 +9210,48 @@ conv_binds_ref_to_prvalue (conversion *c)
   return conv_is_prvalue (next_conversion (c));
 }
 
-/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
-   not involve creating a temporary.  Return tristate::TS_FALSE if converting
-   EXPR to a reference type TYPE binds the reference to a temporary.  If the
-   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
+/* True iff C is a conversion that binds a reference to a temporary.
+   This is a superset of conv_binds_ref_to_prvalue: here we're also
+   interested in xvalues.  */
+
+static bool
+conv_binds_ref_to_temporary (conversion *c)
+{
+  if (conv_binds_ref_to_prvalue (c))
+    return true;
+  if (c->kind != ck_ref_bind)
+    return false;
+  c = next_conversion (c);
+  /* This is the case for
+       struct Base {};
+       struct Derived : Base {};
+       const Base& b(Derived{});
+     where we bind 'b' to the Base subobject of a temporary object of type
+     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
+  if (c->kind != ck_base)
+    return false;
+  c = next_conversion (c);
+  if (c->kind == ck_identity && c->u.expr)
+    {
+      if (TREE_CODE (c->u.expr) == TARGET_EXPR)
+	return true;
+      if ((TREE_CODE (c->u.expr) == COMPONENT_REF
+	   || TREE_CODE (c->u.expr) == ARRAY_REF)
+	  && TREE_CODE (TREE_OPERAND (c->u.expr, 0)) == TARGET_EXPR)
+	return true;
+    }
+  return false;
+}
+
+/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
+   the reference to a temporary.  Return tristate::TS_FALSE if converting
+   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
+   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
    says whether the conversion should be done in direct- or copy-initialization
    context.  */
 
 tristate
-ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
+ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
 {
   gcc_assert (TYPE_REF_P (type));
 
@@ -9230,7 +9263,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
 					  /*c_cast_p=*/false, flags, tf_none);
   tristate ret (tristate::TS_UNKNOWN);
   if (conv && !conv->bad_p)
-    ret = tristate (!conv_binds_ref_to_prvalue (conv));
+    ret = tristate (conv_binds_ref_to_temporary (conv));
 
   /* Free all the conversions we allocated.  */
   obstack_free (&conversion_obstack, p);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8bc1c2dc7fd..469eb2fdb25 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
 extern tree type_decays_to			(tree);
 extern tree extract_call_expr			(tree);
 extern tree build_trivial_dtor_call		(tree, bool = false);
-extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
+extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
 extern tree build_user_type_conversion		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 55af5c43c18..622e1b9802e 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
   tree val = build_stub_object (from);
   if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
     val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
-  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
+  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
 }
 
 /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 555476e42e7..dc3d17c416c 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
 
   if (TYPE_REF_P (type))
     {
-      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
+      if (glvalue_p (expr)
+	  && ref_conv_binds_to_temporary (type, expr).is_true ())
 	{
 	  auto_diagnostic_group d;
 	  if (warning_at (loc, OPT_Wrange_loop_construct,
@@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
   /* If we can initialize a reference directly, suggest that to avoid the
      copy.  */
   tree rtype = cp_build_reference_type (type, /*rval*/false);
-  if (ref_conv_binds_directly (rtype, expr).is_true ())
+  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
     {
       auto_diagnostic_group d;
       if (warning_at (loc, OPT_Wrange_loop_construct,
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
new file mode 100644
index 00000000000..3cc2e3afa5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
@@ -0,0 +1,15 @@
+// PR c++/107085
+// { dg-do compile { target c++11 } }
+
+struct X {
+  X();
+  X(X&&);
+};
+struct Z : X {};
+X x1 = Z();
+X x2 = X(Z());
+
+struct B { };
+struct D : B { };
+B b1 = D();
+B b2 = B(D());
diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
index 76de905a35d..5354b1dc4e6 100644
--- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
@@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
 SA(!__reference_constructs_from_temporary(int&&, G2));
 SA(!__reference_constructs_from_temporary(const int&, H2));
 
-SA(!__reference_constructs_from_temporary(const Base&, Der));
+SA(__reference_constructs_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
index 90196c38742..e6c159e9b00 100644
--- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
@@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
 SA(!__reference_converts_from_temporary(int&&, G2));
 SA(!__reference_converts_from_temporary(const int&, H2));
 
-SA(!__reference_converts_from_temporary(const Base&, Der));
+SA(__reference_converts_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_converts_from_temporary(int&&, id<int[3]>));

base-commit: f8ba88b6a811ca9bb4b8411d3f65c329fb480ee1
-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-07 16:10               ` [PATCH v4] " Marek Polacek
@ 2022-10-07 17:01                 ` Jason Merrill
  2022-10-07 21:26                   ` [PATCH v5] " Marek Polacek
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Merrill @ 2022-10-07 17:01 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 10/7/22 12:10, Marek Polacek wrote:
> On Thu, Oct 06, 2022 at 06:03:57PM -0400, Jason Merrill wrote:
>> On 10/6/22 17:43, Marek Polacek wrote:
>>> On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
>>>> On 10/6/22 13:51, Marek Polacek wrote:
>>>>> On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
>>>>>> On 10/6/22 10:49, Marek Polacek wrote:
>>>>>>> On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
>>>>>>>> On 10/5/22 17:27, Marek Polacek wrote:
>>>>>>> --- a/gcc/cp/call.cc
>>>>>>> +++ b/gcc/cp/call.cc
>>>>>>> @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
>>>>>>>        return conv_is_prvalue (next_conversion (c));
>>>>>>>      }
>>>>>>> +/* True iff C is a conversion that binds a reference to a temporary.
>>>>>>> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
>>>>>>> +   interested in xvalues.  */
>>>>>>> +
>>>>>>> +static bool
>>>>>>> +conv_binds_ref_to_temporary (conversion *c)
>>>>>>> +{
>>>>>>> +  if (conv_binds_ref_to_prvalue (c))
>>>>>>> +    return true;
>>>>>>> +  if (c->kind != ck_ref_bind)
>>>>>>> +    return false;
>>>>>>> +  c = next_conversion (c);
>>>>>>> +  /* This is the case for
>>>>>>> +       struct Base {};
>>>>>>> +       struct Derived : Base {};
>>>>>>> +       const Base& b(Derived{});
>>>>>>> +     where we bind 'b' to the Base subobject of a temporary object of type
>>>>>>> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
>>>>>>> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
>>>>>>
>>>>>> I think you also want to check for the case of c->u.expr being a
>>>>>> COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
>>>>>
>>>>> I see.  So this would be achieved using e.g.
>>>>>
>>>>> struct B { };
>>>>> struct D : B { };
>>>>> struct C {
>>>>>      D d;
>>>>> };
>>>>>
>>>>> const B& b = C{}.d;
>>>>
>>>> Yes.
>>>>
>>>>> Except I'm not sure how to trigger this via the built-in, which takes two types.
>>>>> Am I missing something obvious?
>>>>
>>>> Indeed, it can't be triggered by the built-in.  But I see
>>>> ref_conv_binds_directly is also called from warn_for_range_copy, which ought
>>>> to be able to trigger it.
>>>
>>> Even there, I think, it's not possible to trigger it with, say (d is an
>>> array of D):
>>>
>>>        for (const B &b : C{}.d)
>>>          // ...
>>>
>>> because warn_for_range_copy gets *__for_begin as the EXPR, which means that
>>> we won't get to the TARGET_EXPR.  It's like
>>>
>>>     auto&& __for_begin = C{}.d;
>>>     const B &b = *__for_begin;
>>>
>>> and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
>>> original .u.expr is *__for_begin.
>>
>> Ah, right, and so the C{} temporary gets lifetime-extended and there's no
>> problem to warn about.
>>
>>> I could add some checking assert to conv_binds_ref_to_temporary to see if
>>> we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...
>>
>> Or try to handle it properly even if we can't exercise the code yet.
> 
> Okay, done.

Thanks.

>> I'm thinking again about warning for e.g.
>>
>> const B& b = frotz(C{}.d);
>>
>> where frotz passes through its reference argument, which becomes a dangling
>> reference.  Handling the C{}.d case seems relevant to such a warning.
>>
>> I wonder how many false positives there would be for warning about this for
>> any function that both takes and returns a reference?
> 
> I suppose we'd have to ask if any (or all?) of the parameters is/are
> a reference, to handle std::max?

I was thinking to warn if, in a context that would extend temporary 
lifetime, a call argument binds a reference to a temporary and the 
function returns a reference.

>> Do you want to try that, or shall I?
> 
> So this is like the first example in https://gcc.gnu.org/PR106393, which
> is assigned to...me.  Let me take a stab at the simpler example.  If I
> can't make any progress within, say, a week, I'll unassign so as not to
> block it.
> 
> Meanwhile, here's the updated patch:
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> This PR reports that
> 
>    struct Base {};
>    struct Derived : Base {};
>    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> 
> doesn't pass, which it should: it's just like
> 
>    const Base& b(Derived{});
> 
> where we bind 'b' to the Base subobject of a temporary object of type
> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> we didn't need to create a temporary object just for the base, but the whole
> object is a temporary so we're still binding to a temporary.  Since the
> Base subobject is an xvalue, a new function is introduced.
> 
> 	PR c++/107085
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (conv_binds_ref_to_temporary): New.
> 	(ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.  Use
> 	conv_binds_ref_to_temporary.
> 	* cp-tree.h (ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.
> 	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
> 	* parser.cc (warn_for_range_copy): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> 	result.
> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> 	* g++.dg/cpp0x/elision4.C: New test.
> ---
>   gcc/cp/call.cc                                | 45 ++++++++++++++++---
>   gcc/cp/cp-tree.h                              |  2 +-
>   gcc/cp/method.cc                              |  2 +-
>   gcc/cp/parser.cc                              |  5 ++-
>   gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++
>   .../reference_constructs_from_temporary1.C    |  2 +-
>   .../ext/reference_converts_from_temporary1.C  |  2 +-
>   7 files changed, 61 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd04a1d309a..833cd014b7d 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9210,15 +9210,48 @@ conv_binds_ref_to_prvalue (conversion *c)
>     return conv_is_prvalue (next_conversion (c));
>   }
>   
> -/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
> -   not involve creating a temporary.  Return tristate::TS_FALSE if converting
> -   EXPR to a reference type TYPE binds the reference to a temporary.  If the
> -   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
> +/* True iff C is a conversion that binds a reference to a temporary.
> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> +   interested in xvalues.  */
> +
> +static bool
> +conv_binds_ref_to_temporary (conversion *c)
> +{
> +  if (conv_binds_ref_to_prvalue (c))
> +    return true;
> +  if (c->kind != ck_ref_bind)
> +    return false;
> +  c = next_conversion (c);
> +  /* This is the case for
> +       struct Base {};
> +       struct Derived : Base {};
> +       const Base& b(Derived{});
> +     where we bind 'b' to the Base subobject of a temporary object of type
> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> +  if (c->kind != ck_base)
> +    return false;
> +  c = next_conversion (c);
> +  if (c->kind == ck_identity && c->u.expr)
> +    {
> +      if (TREE_CODE (c->u.expr) == TARGET_EXPR)
> +	return true;
> +      if ((TREE_CODE (c->u.expr) == COMPONENT_REF
> +	   || TREE_CODE (c->u.expr) == ARRAY_REF)

This should be a loop.

> +	  && TREE_CODE (TREE_OPERAND (c->u.expr, 0)) == TARGET_EXPR)
> +	return true;
> +    }
> +  return false;
> +}
> +
> +/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
> +   the reference to a temporary.  Return tristate::TS_FALSE if converting
> +   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
> +   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
>      says whether the conversion should be done in direct- or copy-initialization
>      context.  */
>   
>   tristate
> -ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
> +ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
>   {
>     gcc_assert (TYPE_REF_P (type));
>   
> @@ -9230,7 +9263,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
>   					  /*c_cast_p=*/false, flags, tf_none);
>     tristate ret (tristate::TS_UNKNOWN);
>     if (conv && !conv->bad_p)
> -    ret = tristate (!conv_binds_ref_to_prvalue (conv));
> +    ret = tristate (conv_binds_ref_to_temporary (conv));
>   
>     /* Free all the conversions we allocated.  */
>     obstack_free (&conversion_obstack, p);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 8bc1c2dc7fd..469eb2fdb25 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
>   extern tree type_decays_to			(tree);
>   extern tree extract_call_expr			(tree);
>   extern tree build_trivial_dtor_call		(tree, bool = false);
> -extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
> +extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
>   extern tree build_user_type_conversion		(tree, tree, int,
>   						 tsubst_flags_t);
>   extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 55af5c43c18..622e1b9802e 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
>     tree val = build_stub_object (from);
>     if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
>       val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
> -  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
> +  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
>   }
>   
>   /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 555476e42e7..dc3d17c416c 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
>   
>     if (TYPE_REF_P (type))
>       {
> -      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
> +      if (glvalue_p (expr)
> +	  && ref_conv_binds_to_temporary (type, expr).is_true ())
>   	{
>   	  auto_diagnostic_group d;
>   	  if (warning_at (loc, OPT_Wrange_loop_construct,
> @@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
>     /* If we can initialize a reference directly, suggest that to avoid the
>        copy.  */
>     tree rtype = cp_build_reference_type (type, /*rval*/false);
> -  if (ref_conv_binds_directly (rtype, expr).is_true ())
> +  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
>       {
>         auto_diagnostic_group d;
>         if (warning_at (loc, OPT_Wrange_loop_construct,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> new file mode 100644
> index 00000000000..3cc2e3afa5d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> @@ -0,0 +1,15 @@
> +// PR c++/107085
> +// { dg-do compile { target c++11 } }
> +
> +struct X {
> +  X();
> +  X(X&&);
> +};
> +struct Z : X {};
> +X x1 = Z();
> +X x2 = X(Z());
> +
> +struct B { };
> +struct D : B { };
> +B b1 = D();
> +B b2 = B(D());
> diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> index 76de905a35d..5354b1dc4e6 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> @@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
>   SA(!__reference_constructs_from_temporary(int&&, G2));
>   SA(!__reference_constructs_from_temporary(const int&, H2));
>   
> -SA(!__reference_constructs_from_temporary(const Base&, Der));
> +SA(__reference_constructs_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
> diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> index 90196c38742..e6c159e9b00 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> @@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
>   SA(!__reference_converts_from_temporary(int&&, G2));
>   SA(!__reference_converts_from_temporary(const int&, H2));
>   
> -SA(!__reference_converts_from_temporary(const Base&, Der));
> +SA(__reference_converts_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_converts_from_temporary(int&&, id<int[3]>));
> 
> base-commit: f8ba88b6a811ca9bb4b8411d3f65c329fb480ee1


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v5] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-07 17:01                 ` Jason Merrill
@ 2022-10-07 21:26                   ` Marek Polacek
  2022-10-07 21:50                     ` Jason Merrill
  0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2022-10-07 21:26 UTC (permalink / raw)
  To: Jason Merrill; +Cc: GCC Patches

On Fri, Oct 07, 2022 at 01:01:35PM -0400, Jason Merrill wrote:
> On 10/7/22 12:10, Marek Polacek wrote:
> > On Thu, Oct 06, 2022 at 06:03:57PM -0400, Jason Merrill wrote:
> > > On 10/6/22 17:43, Marek Polacek wrote:
> > > > On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
> > > > > On 10/6/22 13:51, Marek Polacek wrote:
> > > > > > On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
> > > > > > > On 10/6/22 10:49, Marek Polacek wrote:
> > > > > > > > On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
> > > > > > > > > On 10/5/22 17:27, Marek Polacek wrote:
> > > > > > > > --- a/gcc/cp/call.cc
> > > > > > > > +++ b/gcc/cp/call.cc
> > > > > > > > @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
> > > > > > > >        return conv_is_prvalue (next_conversion (c));
> > > > > > > >      }
> > > > > > > > +/* True iff C is a conversion that binds a reference to a temporary.
> > > > > > > > +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> > > > > > > > +   interested in xvalues.  */
> > > > > > > > +
> > > > > > > > +static bool
> > > > > > > > +conv_binds_ref_to_temporary (conversion *c)
> > > > > > > > +{
> > > > > > > > +  if (conv_binds_ref_to_prvalue (c))
> > > > > > > > +    return true;
> > > > > > > > +  if (c->kind != ck_ref_bind)
> > > > > > > > +    return false;
> > > > > > > > +  c = next_conversion (c);
> > > > > > > > +  /* This is the case for
> > > > > > > > +       struct Base {};
> > > > > > > > +       struct Derived : Base {};
> > > > > > > > +       const Base& b(Derived{});
> > > > > > > > +     where we bind 'b' to the Base subobject of a temporary object of type
> > > > > > > > +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> > > > > > > > +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
> > > > > > > 
> > > > > > > I think you also want to check for the case of c->u.expr being a
> > > > > > > COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
> > > > > > 
> > > > > > I see.  So this would be achieved using e.g.
> > > > > > 
> > > > > > struct B { };
> > > > > > struct D : B { };
> > > > > > struct C {
> > > > > >      D d;
> > > > > > };
> > > > > > 
> > > > > > const B& b = C{}.d;
> > > > > 
> > > > > Yes.
> > > > > 
> > > > > > Except I'm not sure how to trigger this via the built-in, which takes two types.
> > > > > > Am I missing something obvious?
> > > > > 
> > > > > Indeed, it can't be triggered by the built-in.  But I see
> > > > > ref_conv_binds_directly is also called from warn_for_range_copy, which ought
> > > > > to be able to trigger it.
> > > > 
> > > > Even there, I think, it's not possible to trigger it with, say (d is an
> > > > array of D):
> > > > 
> > > >        for (const B &b : C{}.d)
> > > >          // ...
> > > > 
> > > > because warn_for_range_copy gets *__for_begin as the EXPR, which means that
> > > > we won't get to the TARGET_EXPR.  It's like
> > > > 
> > > >     auto&& __for_begin = C{}.d;
> > > >     const B &b = *__for_begin;
> > > > 
> > > > and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
> > > > original .u.expr is *__for_begin.
> > > 
> > > Ah, right, and so the C{} temporary gets lifetime-extended and there's no
> > > problem to warn about.
> > > 
> > > > I could add some checking assert to conv_binds_ref_to_temporary to see if
> > > > we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...
> > > 
> > > Or try to handle it properly even if we can't exercise the code yet.
> > 
> > Okay, done.
> 
> Thanks.
> 
> > > I'm thinking again about warning for e.g.
> > > 
> > > const B& b = frotz(C{}.d);
> > > 
> > > where frotz passes through its reference argument, which becomes a dangling
> > > reference.  Handling the C{}.d case seems relevant to such a warning.
> > > 
> > > I wonder how many false positives there would be for warning about this for
> > > any function that both takes and returns a reference?
> > 
> > I suppose we'd have to ask if any (or all?) of the parameters is/are
> > a reference, to handle std::max?
> 
> I was thinking to warn if, in a context that would extend temporary
> lifetime, a call argument binds a reference to a temporary and the function
> returns a reference.

Maybe store_init_value, around the call to extend_ref_init_temps, would be
a good place to start?
 
> > +  if (c->kind != ck_base)
> > +    return false;
> > +  c = next_conversion (c);
> > +  if (c->kind == ck_identity && c->u.expr)
> > +    {
> > +      if (TREE_CODE (c->u.expr) == TARGET_EXPR)
> > +	return true;
> > +      if ((TREE_CODE (c->u.expr) == COMPONENT_REF
> > +	   || TREE_CODE (c->u.expr) == ARRAY_REF)
> 
> This should be a loop.

Sigh, yes, sorry.  I suppose I could use handled_component_p like this:

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

-- >8 --
This PR reports that

  struct Base {};
  struct Derived : Base {};
  static_assert(__reference_constructs_from_temporary(Base const&, Derived));

doesn't pass, which it should: it's just like

  const Base& b(Derived{});

where we bind 'b' to the Base subobject of a temporary object of type
Derived.  The ck_base conversion didn't have ->need_temporary_p set because
we didn't need to create a temporary object just for the base, but the whole
object is a temporary so we're still binding to a temporary.  Since the
Base subobject is an xvalue, a new function is introduced.

	PR c++/107085

gcc/cp/ChangeLog:

	* call.cc (conv_binds_ref_to_temporary): New.
	(ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.  Use
	conv_binds_ref_to_temporary.
	* cp-tree.h (ref_conv_binds_directly): Rename to...
	(ref_conv_binds_to_temporary): ...this.
	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
	* parser.cc (warn_for_range_copy): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
	result.
	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
	* g++.dg/cpp0x/elision4.C: New test.
---
 gcc/cp/call.cc                                | 44 ++++++++++++++++---
 gcc/cp/cp-tree.h                              |  2 +-
 gcc/cp/method.cc                              |  2 +-
 gcc/cp/parser.cc                              |  5 ++-
 gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++
 .../reference_constructs_from_temporary1.C    |  2 +-
 .../ext/reference_converts_from_temporary1.C  |  2 +-
 7 files changed, 60 insertions(+), 12 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C

diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
index bd04a1d309a..7771d80ff31 100644
--- a/gcc/cp/call.cc
+++ b/gcc/cp/call.cc
@@ -9210,15 +9210,47 @@ conv_binds_ref_to_prvalue (conversion *c)
   return conv_is_prvalue (next_conversion (c));
 }
 
-/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
-   not involve creating a temporary.  Return tristate::TS_FALSE if converting
-   EXPR to a reference type TYPE binds the reference to a temporary.  If the
-   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
+/* True iff C is a conversion that binds a reference to a temporary.
+   This is a superset of conv_binds_ref_to_prvalue: here we're also
+   interested in xvalues.  */
+
+static bool
+conv_binds_ref_to_temporary (conversion *c)
+{
+  if (conv_binds_ref_to_prvalue (c))
+    return true;
+  if (c->kind != ck_ref_bind)
+    return false;
+  c = next_conversion (c);
+  /* This is the case for
+       struct Base {};
+       struct Derived : Base {};
+       const Base& b(Derived{});
+     where we bind 'b' to the Base subobject of a temporary object of type
+     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
+  if (c->kind != ck_base)
+    return false;
+  c = next_conversion (c);
+  if (c->kind == ck_identity && c->u.expr)
+    {
+      tree expr = c->u.expr;
+      while (handled_component_p (expr))
+	expr = TREE_OPERAND (expr, 0);
+      if (TREE_CODE (expr) == TARGET_EXPR)
+	return true;
+    }
+  return false;
+}
+
+/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
+   the reference to a temporary.  Return tristate::TS_FALSE if converting
+   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
+   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
    says whether the conversion should be done in direct- or copy-initialization
    context.  */
 
 tristate
-ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
+ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
 {
   gcc_assert (TYPE_REF_P (type));
 
@@ -9230,7 +9262,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
 					  /*c_cast_p=*/false, flags, tf_none);
   tristate ret (tristate::TS_UNKNOWN);
   if (conv && !conv->bad_p)
-    ret = tristate (!conv_binds_ref_to_prvalue (conv));
+    ret = tristate (conv_binds_ref_to_temporary (conv));
 
   /* Free all the conversions we allocated.  */
   obstack_free (&conversion_obstack, p);
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 8bc1c2dc7fd..469eb2fdb25 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
 extern tree type_decays_to			(tree);
 extern tree extract_call_expr			(tree);
 extern tree build_trivial_dtor_call		(tree, bool = false);
-extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
+extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
 extern tree build_user_type_conversion		(tree, tree, int,
 						 tsubst_flags_t);
 extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
index 55af5c43c18..622e1b9802e 100644
--- a/gcc/cp/method.cc
+++ b/gcc/cp/method.cc
@@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
   tree val = build_stub_object (from);
   if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
     val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
-  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
+  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
 }
 
 /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index 555476e42e7..dc3d17c416c 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
 
   if (TYPE_REF_P (type))
     {
-      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
+      if (glvalue_p (expr)
+	  && ref_conv_binds_to_temporary (type, expr).is_true ())
 	{
 	  auto_diagnostic_group d;
 	  if (warning_at (loc, OPT_Wrange_loop_construct,
@@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
   /* If we can initialize a reference directly, suggest that to avoid the
      copy.  */
   tree rtype = cp_build_reference_type (type, /*rval*/false);
-  if (ref_conv_binds_directly (rtype, expr).is_true ())
+  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
     {
       auto_diagnostic_group d;
       if (warning_at (loc, OPT_Wrange_loop_construct,
diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
new file mode 100644
index 00000000000..3cc2e3afa5d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
@@ -0,0 +1,15 @@
+// PR c++/107085
+// { dg-do compile { target c++11 } }
+
+struct X {
+  X();
+  X(X&&);
+};
+struct Z : X {};
+X x1 = Z();
+X x2 = X(Z());
+
+struct B { };
+struct D : B { };
+B b1 = D();
+B b2 = B(D());
diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
index 76de905a35d..5354b1dc4e6 100644
--- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
@@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
 SA(!__reference_constructs_from_temporary(int&&, G2));
 SA(!__reference_constructs_from_temporary(const int&, H2));
 
-SA(!__reference_constructs_from_temporary(const Base&, Der));
+SA(__reference_constructs_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
index 90196c38742..e6c159e9b00 100644
--- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
+++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
@@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
 SA(!__reference_converts_from_temporary(int&&, G2));
 SA(!__reference_converts_from_temporary(const int&, H2));
 
-SA(!__reference_converts_from_temporary(const Base&, Der));
+SA(__reference_converts_from_temporary(const Base&, Der));
 
 // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
 SA(!__reference_converts_from_temporary(int&&, id<int[3]>));

base-commit: f30e9fd33e56a5a721346ea6140722e1b193db42
-- 
2.37.3


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v5] c++: fixes for derived-to-base reference binding [PR107085]
  2022-10-07 21:26                   ` [PATCH v5] " Marek Polacek
@ 2022-10-07 21:50                     ` Jason Merrill
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Merrill @ 2022-10-07 21:50 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches

On 10/7/22 17:26, Marek Polacek wrote:
> On Fri, Oct 07, 2022 at 01:01:35PM -0400, Jason Merrill wrote:
>> On 10/7/22 12:10, Marek Polacek wrote:
>>> On Thu, Oct 06, 2022 at 06:03:57PM -0400, Jason Merrill wrote:
>>>> On 10/6/22 17:43, Marek Polacek wrote:
>>>>> On Thu, Oct 06, 2022 at 02:00:40PM -0400, Jason Merrill wrote:
>>>>>> On 10/6/22 13:51, Marek Polacek wrote:
>>>>>>> On Thu, Oct 06, 2022 at 10:58:44AM -0400, Jason Merrill wrote:
>>>>>>>> On 10/6/22 10:49, Marek Polacek wrote:
>>>>>>>>> On Wed, Oct 05, 2022 at 08:25:29PM -0400, Jason Merrill wrote:
>>>>>>>>>> On 10/5/22 17:27, Marek Polacek wrote:
>>>>>>>>> --- a/gcc/cp/call.cc
>>>>>>>>> +++ b/gcc/cp/call.cc
>>>>>>>>> @@ -9210,6 +9210,27 @@ conv_binds_ref_to_prvalue (conversion *c)
>>>>>>>>>         return conv_is_prvalue (next_conversion (c));
>>>>>>>>>       }
>>>>>>>>> +/* True iff C is a conversion that binds a reference to a temporary.
>>>>>>>>> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
>>>>>>>>> +   interested in xvalues.  */
>>>>>>>>> +
>>>>>>>>> +static bool
>>>>>>>>> +conv_binds_ref_to_temporary (conversion *c)
>>>>>>>>> +{
>>>>>>>>> +  if (conv_binds_ref_to_prvalue (c))
>>>>>>>>> +    return true;
>>>>>>>>> +  if (c->kind != ck_ref_bind)
>>>>>>>>> +    return false;
>>>>>>>>> +  c = next_conversion (c);
>>>>>>>>> +  /* This is the case for
>>>>>>>>> +       struct Base {};
>>>>>>>>> +       struct Derived : Base {};
>>>>>>>>> +       const Base& b(Derived{});
>>>>>>>>> +     where we bind 'b' to the Base subobject of a temporary object of type
>>>>>>>>> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
>>>>>>>>> +  return (c->kind == ck_base && conv_is_prvalue (next_conversion (c)));
>>>>>>>>
>>>>>>>> I think you also want to check for the case of c->u.expr being a
>>>>>>>> COMPONENT_REF/ARRAY_REF around a TARGET_EXPR, as you mentioned.
>>>>>>>
>>>>>>> I see.  So this would be achieved using e.g.
>>>>>>>
>>>>>>> struct B { };
>>>>>>> struct D : B { };
>>>>>>> struct C {
>>>>>>>       D d;
>>>>>>> };
>>>>>>>
>>>>>>> const B& b = C{}.d;
>>>>>>
>>>>>> Yes.
>>>>>>
>>>>>>> Except I'm not sure how to trigger this via the built-in, which takes two types.
>>>>>>> Am I missing something obvious?
>>>>>>
>>>>>> Indeed, it can't be triggered by the built-in.  But I see
>>>>>> ref_conv_binds_directly is also called from warn_for_range_copy, which ought
>>>>>> to be able to trigger it.
>>>>>
>>>>> Even there, I think, it's not possible to trigger it with, say (d is an
>>>>> array of D):
>>>>>
>>>>>         for (const B &b : C{}.d)
>>>>>           // ...
>>>>>
>>>>> because warn_for_range_copy gets *__for_begin as the EXPR, which means that
>>>>> we won't get to the TARGET_EXPR.  It's like
>>>>>
>>>>>      auto&& __for_begin = C{}.d;
>>>>>      const B &b = *__for_begin;
>>>>>
>>>>> and the conversion warn_for_range_copy sees is D -> const B -> const B&, the
>>>>> original .u.expr is *__for_begin.
>>>>
>>>> Ah, right, and so the C{} temporary gets lifetime-extended and there's no
>>>> problem to warn about.
>>>>
>>>>> I could add some checking assert to conv_binds_ref_to_temporary to see if
>>>>> we ever encounter a COMPONENT_REF/ARRAY_REF around a TARGET_EXPR...
>>>>
>>>> Or try to handle it properly even if we can't exercise the code yet.
>>>
>>> Okay, done.
>>
>> Thanks.
>>
>>>> I'm thinking again about warning for e.g.
>>>>
>>>> const B& b = frotz(C{}.d);
>>>>
>>>> where frotz passes through its reference argument, which becomes a dangling
>>>> reference.  Handling the C{}.d case seems relevant to such a warning.
>>>>
>>>> I wonder how many false positives there would be for warning about this for
>>>> any function that both takes and returns a reference?
>>>
>>> I suppose we'd have to ask if any (or all?) of the parameters is/are
>>> a reference, to handle std::max?
>>
>> I was thinking to warn if, in a context that would extend temporary
>> lifetime, a call argument binds a reference to a temporary and the function
>> returns a reference.
> 
> Maybe store_init_value, around the call to extend_ref_init_temps, would be
> a good place to start?

Sounds good, or even in extend_ref_init_temps.

>>> +  if (c->kind != ck_base)
>>> +    return false;
>>> +  c = next_conversion (c);
>>> +  if (c->kind == ck_identity && c->u.expr)
>>> +    {
>>> +      if (TREE_CODE (c->u.expr) == TARGET_EXPR)
>>> +	return true;
>>> +      if ((TREE_CODE (c->u.expr) == COMPONENT_REF
>>> +	   || TREE_CODE (c->u.expr) == ARRAY_REF)
>>
>> This should be a loop.
> 
> Sigh, yes, sorry.  I suppose I could use handled_component_p like this:
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK, thanks.

> -- >8 --
> This PR reports that
> 
>    struct Base {};
>    struct Derived : Base {};
>    static_assert(__reference_constructs_from_temporary(Base const&, Derived));
> 
> doesn't pass, which it should: it's just like
> 
>    const Base& b(Derived{});
> 
> where we bind 'b' to the Base subobject of a temporary object of type
> Derived.  The ck_base conversion didn't have ->need_temporary_p set because
> we didn't need to create a temporary object just for the base, but the whole
> object is a temporary so we're still binding to a temporary.  Since the
> Base subobject is an xvalue, a new function is introduced.
> 
> 	PR c++/107085
> 
> gcc/cp/ChangeLog:
> 
> 	* call.cc (conv_binds_ref_to_temporary): New.
> 	(ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.  Use
> 	conv_binds_ref_to_temporary.
> 	* cp-tree.h (ref_conv_binds_directly): Rename to...
> 	(ref_conv_binds_to_temporary): ...this.
> 	* method.cc (ref_xes_from_temporary): Use ref_conv_binds_to_temporary.
> 	* parser.cc (warn_for_range_copy): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/ext/reference_constructs_from_temporary1.C: Adjust expected
> 	result.
> 	* g++.dg/ext/reference_converts_from_temporary1.C: Likewise.
> 	* g++.dg/cpp0x/elision4.C: New test.
> ---
>   gcc/cp/call.cc                                | 44 ++++++++++++++++---
>   gcc/cp/cp-tree.h                              |  2 +-
>   gcc/cp/method.cc                              |  2 +-
>   gcc/cp/parser.cc                              |  5 ++-
>   gcc/testsuite/g++.dg/cpp0x/elision4.C         | 15 +++++++
>   .../reference_constructs_from_temporary1.C    |  2 +-
>   .../ext/reference_converts_from_temporary1.C  |  2 +-
>   7 files changed, 60 insertions(+), 12 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/elision4.C
> 
> diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc
> index bd04a1d309a..7771d80ff31 100644
> --- a/gcc/cp/call.cc
> +++ b/gcc/cp/call.cc
> @@ -9210,15 +9210,47 @@ conv_binds_ref_to_prvalue (conversion *c)
>     return conv_is_prvalue (next_conversion (c));
>   }
>   
> -/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE does
> -   not involve creating a temporary.  Return tristate::TS_FALSE if converting
> -   EXPR to a reference type TYPE binds the reference to a temporary.  If the
> -   conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
> +/* True iff C is a conversion that binds a reference to a temporary.
> +   This is a superset of conv_binds_ref_to_prvalue: here we're also
> +   interested in xvalues.  */
> +
> +static bool
> +conv_binds_ref_to_temporary (conversion *c)
> +{
> +  if (conv_binds_ref_to_prvalue (c))
> +    return true;
> +  if (c->kind != ck_ref_bind)
> +    return false;
> +  c = next_conversion (c);
> +  /* This is the case for
> +       struct Base {};
> +       struct Derived : Base {};
> +       const Base& b(Derived{});
> +     where we bind 'b' to the Base subobject of a temporary object of type
> +     Derived.  The subobject is an xvalue; the whole object is a prvalue.  */
> +  if (c->kind != ck_base)
> +    return false;
> +  c = next_conversion (c);
> +  if (c->kind == ck_identity && c->u.expr)
> +    {
> +      tree expr = c->u.expr;
> +      while (handled_component_p (expr))
> +	expr = TREE_OPERAND (expr, 0);
> +      if (TREE_CODE (expr) == TARGET_EXPR)
> +	return true;
> +    }
> +  return false;
> +}
> +
> +/* Return tristate::TS_TRUE if converting EXPR to a reference type TYPE binds
> +   the reference to a temporary.  Return tristate::TS_FALSE if converting
> +   EXPR to a reference type TYPE doesn't bind the reference to a temporary.  If
> +   the conversion is invalid or bad, return tristate::TS_UNKNOWN.  DIRECT_INIT_P
>      says whether the conversion should be done in direct- or copy-initialization
>      context.  */
>   
>   tristate
> -ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
> +ref_conv_binds_to_temporary (tree type, tree expr, bool direct_init_p/*=false*/)
>   {
>     gcc_assert (TYPE_REF_P (type));
>   
> @@ -9230,7 +9262,7 @@ ref_conv_binds_directly (tree type, tree expr, bool direct_init_p /*= false*/)
>   					  /*c_cast_p=*/false, flags, tf_none);
>     tristate ret (tristate::TS_UNKNOWN);
>     if (conv && !conv->bad_p)
> -    ret = tristate (!conv_binds_ref_to_prvalue (conv));
> +    ret = tristate (conv_binds_ref_to_temporary (conv));
>   
>     /* Free all the conversions we allocated.  */
>     obstack_free (&conversion_obstack, p);
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 8bc1c2dc7fd..469eb2fdb25 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6534,7 +6534,7 @@ extern bool sufficient_parms_p			(const_tree);
>   extern tree type_decays_to			(tree);
>   extern tree extract_call_expr			(tree);
>   extern tree build_trivial_dtor_call		(tree, bool = false);
> -extern tristate ref_conv_binds_directly		(tree, tree, bool = false);
> +extern tristate ref_conv_binds_to_temporary	(tree, tree, bool = false);
>   extern tree build_user_type_conversion		(tree, tree, int,
>   						 tsubst_flags_t);
>   extern tree build_new_function_call		(tree, vec<tree, va_gc> **,
> diff --git a/gcc/cp/method.cc b/gcc/cp/method.cc
> index 55af5c43c18..622e1b9802e 100644
> --- a/gcc/cp/method.cc
> +++ b/gcc/cp/method.cc
> @@ -2233,7 +2233,7 @@ ref_xes_from_temporary (tree to, tree from, bool direct_init_p)
>     tree val = build_stub_object (from);
>     if (!TYPE_REF_P (from) && TREE_CODE (from) != FUNCTION_TYPE)
>       val = CLASS_TYPE_P (from) ? force_rvalue (val, tf_none) : rvalue (val);
> -  return ref_conv_binds_directly (to, val, direct_init_p).is_false ();
> +  return ref_conv_binds_to_temporary (to, val, direct_init_p).is_true ();
>   }
>   
>   /* Worker for is_{,nothrow_}convertible.  Attempt to perform an implicit
> diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
> index 555476e42e7..dc3d17c416c 100644
> --- a/gcc/cp/parser.cc
> +++ b/gcc/cp/parser.cc
> @@ -13731,7 +13731,8 @@ warn_for_range_copy (tree decl, tree expr)
>   
>     if (TYPE_REF_P (type))
>       {
> -      if (glvalue_p (expr) && ref_conv_binds_directly (type, expr).is_false ())
> +      if (glvalue_p (expr)
> +	  && ref_conv_binds_to_temporary (type, expr).is_true ())
>   	{
>   	  auto_diagnostic_group d;
>   	  if (warning_at (loc, OPT_Wrange_loop_construct,
> @@ -13762,7 +13763,7 @@ warn_for_range_copy (tree decl, tree expr)
>     /* If we can initialize a reference directly, suggest that to avoid the
>        copy.  */
>     tree rtype = cp_build_reference_type (type, /*rval*/false);
> -  if (ref_conv_binds_directly (rtype, expr).is_true ())
> +  if (ref_conv_binds_to_temporary (rtype, expr).is_false ())
>       {
>         auto_diagnostic_group d;
>         if (warning_at (loc, OPT_Wrange_loop_construct,
> diff --git a/gcc/testsuite/g++.dg/cpp0x/elision4.C b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> new file mode 100644
> index 00000000000..3cc2e3afa5d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/elision4.C
> @@ -0,0 +1,15 @@
> +// PR c++/107085
> +// { dg-do compile { target c++11 } }
> +
> +struct X {
> +  X();
> +  X(X&&);
> +};
> +struct Z : X {};
> +X x1 = Z();
> +X x2 = X(Z());
> +
> +struct B { };
> +struct D : B { };
> +B b1 = D();
> +B b2 = B(D());
> diff --git a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> index 76de905a35d..5354b1dc4e6 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_constructs_from_temporary1.C
> @@ -201,7 +201,7 @@ SA(!__reference_constructs_from_temporary(const int&, H));
>   SA(!__reference_constructs_from_temporary(int&&, G2));
>   SA(!__reference_constructs_from_temporary(const int&, H2));
>   
> -SA(!__reference_constructs_from_temporary(const Base&, Der));
> +SA(__reference_constructs_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_constructs_from_temporary(int&&, id<int[3]>));
> diff --git a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> index 90196c38742..e6c159e9b00 100644
> --- a/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> +++ b/gcc/testsuite/g++.dg/ext/reference_converts_from_temporary1.C
> @@ -201,7 +201,7 @@ SA( __reference_converts_from_temporary(const int&, H));
>   SA(!__reference_converts_from_temporary(int&&, G2));
>   SA(!__reference_converts_from_temporary(const int&, H2));
>   
> -SA(!__reference_converts_from_temporary(const Base&, Der));
> +SA(__reference_converts_from_temporary(const Base&, Der));
>   
>   // This fails because std::is_constructible_v<int&&, id<int[3]>> is false.
>   SA(!__reference_converts_from_temporary(int&&, id<int[3]>));
> 
> base-commit: f30e9fd33e56a5a721346ea6140722e1b193db42


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-10-07 21:50 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-05 21:27 [PATCH] c++: fixes for derived-to-base reference binding [PR107085] Marek Polacek
2022-10-06  0:25 ` Jason Merrill
2022-10-06 14:49   ` [PATCH v2] " Marek Polacek
2022-10-06 14:58     ` Jason Merrill
2022-10-06 17:51       ` Marek Polacek
2022-10-06 18:00         ` Jason Merrill
2022-10-06 21:43           ` [PATCH v3] " Marek Polacek
2022-10-06 22:03             ` Jason Merrill
2022-10-07 16:10               ` [PATCH v4] " Marek Polacek
2022-10-07 17:01                 ` Jason Merrill
2022-10-07 21:26                   ` [PATCH v5] " Marek Polacek
2022-10-07 21:50                     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).