c++: fixes for derived-to-base reference binding [PR107085]
Checks
Commit Message
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
base-commit: e99dcbb54e07b798c3353124f38336f96a826d43
Comments
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
@@ -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;
new file mode 100644
@@ -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());
@@ -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]>));
@@ -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]>));