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

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