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

[Jason Merrill]

On Tue, May 31, 2016 at 12:29 PM, Paolo Carlini
 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

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

[Paolo Carlini]

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.

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

[Jason Merrill]

OK.

Jason

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

[Paolo Carlini]

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.
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{});
+}