diff mbox

[C++] PR 70972 ("[6/7 Regression] Inheriting constructors taking parameters by value should move them, not copy")

Message ID 57432427.30806@oracle.com
State New
Headers show

Commit Message

Paolo Carlini May 23, 2016, 3:39 p.m. UTC 
Hi,

admittedly I didn't spend much time on this issue, but submitter himself 
provided a strong hint and a fix may be very simple... Essentially, he 
noticed that the implementation of forward_parm, being kind of a 
std::forward for internal uses, misses a cp_build_reference (*, true) 
when handling the testcases at issue. Adding it, indeed fixes the 
testcases (both the dg-do compile and the dg-do run). Beyond submitter's 
hint, we can't just unconditionally call cp_build_reference however, for 
example cpp0x/inh-ctor16.C would regress: that doesn't surprise me much, 
when I look at other helper functions in method.c... Then, is the below 
good enough? Tested x86_64-linux.

Thanks,
Paolo.

//////////////////////////
/cp
2016-05-23

	PR c++/70972
	* method.c (forward_parm): Use cp_build_reference_type.

/testsuite
2016-05-23

	PR c++/70972
	* g++.dg/cpp0x/inh-ctor20.C: New.
	* g++.dg/cpp0x/inh-ctor21.C: Likewise.

Comments

Jason Merrill May 23, 2016, 5:32 p.m. UTC | #1 
OK.

Jason
Paolo Carlini May 31, 2016, 4:29 p.m. UTC | #2 
Hi,

On 23/05/2016 19:32, Jason Merrill wrote:
> OK.
What about gcc-6-branch? Assuming no issues show up in mainline I think 
we want it there too in time for 6.2, right?

Thanks,
Paolo.
Jason Merrill May 31, 2016, 5:45 p.m. UTC | #3 
On Tue, May 31, 2016 at 12:29 PM, Paolo Carlini
<paolo.carlini@oracle.com> wrote:
> Hi,
>
> On 23/05/2016 19:32, Jason Merrill wrote:
>>
>> OK.
>
> What about gcc-6-branch? Assuming no issues show up in mainline I think we
> want it there too in time for 6.2, right?

Definitely.

Jason
diff mbox

Patch

Index: cp/method.c
===================================================================
--- cp/method.c	(revision 236581)
+++ cp/method.c	(working copy)
@@ -484,6 +484,8 @@  forward_parm (tree parm)
   tree type = TREE_TYPE (parm);
   if (DECL_PACK_P (parm))
     type = PACK_EXPANSION_PATTERN (type);
+  if (TREE_CODE (type) != REFERENCE_TYPE)
+    type = cp_build_reference_type (type, /*rval=*/true);
   exp = build_static_cast (type, exp, tf_warning_or_error);
   if (DECL_PACK_P (parm))
     exp = make_pack_expansion (exp);
Index: testsuite/g++.dg/cpp0x/inh-ctor20.C
===================================================================
--- testsuite/g++.dg/cpp0x/inh-ctor20.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/inh-ctor20.C	(working copy)
@@ -0,0 +1,16 @@ 
+// PR c++/70972
+// { dg-do compile { target c++11 } }
+
+struct moveonly {
+    moveonly(moveonly&&) = default;
+    moveonly() = default;
+};
+
+struct A {
+    A(moveonly) {}
+};
+struct B : A {
+    using A::A;
+};
+
+B b(moveonly{});
Index: testsuite/g++.dg/cpp0x/inh-ctor21.C
===================================================================
--- testsuite/g++.dg/cpp0x/inh-ctor21.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/inh-ctor21.C	(working copy)
@@ -0,0 +1,19 @@ 
+// PR c++/70972
+// { dg-do run { target c++11 } }
+
+struct abort_on_copy{
+    abort_on_copy(abort_on_copy&&) = default;
+    abort_on_copy(const abort_on_copy&) { __builtin_abort(); }
+    abort_on_copy() = default;
+};
+
+struct A {
+    A(abort_on_copy) {}
+};
+struct B : A {
+    using A::A;
+};
+
+int main() {
+    B b(abort_on_copy{});
+}