[v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Ville Voutilainen]

[-- Attachment #1: Type: text/plain, Size: 416 bytes --]

Tested on Linux-PPC64.

2016-05-08  Ville Voutilainen  <ville.voutilainen@gmail.com>

    Avoid endless run-time recursion for copying single-element
    tuples where the element type is by-value constructible
    from any type.
     * include/std/tuple (_TC<>::_NotSameTuple): New.
     * include/std/tuple (tuple(_UElements&&...): Use it.
    * testsuite/20_util/tuple/cons/element_accepts_anything_byval.cc: New.

[-- Attachment #2: stl-tuple-issue.diff --]
[-- Type: text/plain, Size: 3301 bytes --]

diff --git a/libstdc++-v3/include/std/tuple b/libstdc++-v3/include/std/tuple
index 53f3184..7522e43 100644
--- a/libstdc++-v3/include/std/tuple
+++ b/libstdc++-v3/include/std/tuple
@@ -500,6 +500,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                      __not_<is_constructible<_Elements..., _SrcTuple>>
               >::value;
     }
+    template<typename... _UElements>
+    static constexpr bool _NotSameTuple()
+    {
+      return  __not_<is_same<tuple<_Elements...>,
+			     typename remove_const<
+			       typename remove_reference<_UElements...>::type
+			       >::type>>::value;
+    }
   };
 
   template<typename... _Elements>
@@ -534,6 +542,11 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
     {
       return true;
     }
+    template<typename... _UElements>
+    static constexpr bool _NotSameTuple()
+    {
+      return  true;
+    }
   };
 
   /// Primary class template, tuple
@@ -611,7 +624,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
                       _Elements...>;
 
       template<typename... _UElements, typename
-        enable_if<_TMC<_UElements...>::template
+	       enable_if<
+		  _TC<sizeof...(_UElements) == 1, _Elements...>::template
+		    _NotSameTuple<_UElements...>()
+		  && _TMC<_UElements...>::template
                     _MoveConstructibleTuple<_UElements...>()
                   && _TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>()
@@ -621,7 +637,10 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
         : _Inherited(std::forward<_UElements>(__elements)...) { }
 
       template<typename... _UElements, typename
-        enable_if<_TMC<_UElements...>::template
+        enable_if<
+		  _TC<sizeof...(_UElements) == 1, _Elements...>::template
+		    _NotSameTuple<_UElements...>()
+		  && _TMC<_UElements...>::template
                     _MoveConstructibleTuple<_UElements...>()
                   && !_TMC<_UElements...>::template
                     _ImplicitlyMoveConvertibleTuple<_UElements...>()
diff --git a/libstdc++-v3/testsuite/20_util/tuple/cons/element_accepts_anything_byval.cc b/libstdc++-v3/testsuite/20_util/tuple/cons/element_accepts_anything_byval.cc
new file mode 100644
index 0000000..fe9bea6
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/tuple/cons/element_accepts_anything_byval.cc
@@ -0,0 +1,30 @@
+// Copyright (C) 2016 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library.  This library is free
+// software; you can redistribute it and/or modify it under the
+// terms of the GNU General Public License as published by the
+// Free Software Foundation; either version 3, or (at your option)
+// any later version.
+
+// This library is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License along
+// with this library; see the file COPYING3.  If not see
+// <http://www.gnu.org/licenses/>.
+
+#include <tuple>
+using namespace std;
+
+struct Something {
+    Something() { }
+    template <typename T> Something(T) { }
+};
+
+int main() {
+    tuple<Something> t1;
+    tuple<Something> t2 = t1;
+}
+

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Daniel Krügler]

Have you considered to test against decay instead of
remove_reference/remove_const? That would be similar to other places
in the standard. (I also believe that your fix actually should be
submitted as an LWG issue)

- Daniel

2016-05-08 13:43 GMT+02:00 Ville Voutilainen <ville.voutilainen@gmail.com>:
> Tested on Linux-PPC64.
>
> 2016-05-08  Ville Voutilainen  <ville.voutilainen@gmail.com>
>
>     Avoid endless run-time recursion for copying single-element
>     tuples where the element type is by-value constructible
>     from any type.
>      * include/std/tuple (_TC<>::_NotSameTuple): New.
>      * include/std/tuple (tuple(_UElements&&...): Use it.
>     * testsuite/20_util/tuple/cons/element_accepts_anything_byval.cc: New.



-- 

________________________________
SavedURI :Show URLShow URLSavedURI :
SavedURI :Hide URLHide URLSavedURI :
https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.de.LEt2fN4ilLE.O/m=m_i,t,it/am=OCMOBiHj9kJxhnelj6j997_NLil29vVAOBGeBBRgJwD-m_0_8B_AD-qOEw/rt=h/d=1/rs=AItRSTODy9wv1JKZMABIG3Ak8ViC4kuOWA?random=1395770800154https://mail.google.com/_/scs/mail-static/_/js/k=gmail.main.de.LEt2fN4ilLE.O/m=m_i,t,it/am=OCMOBiHj9kJxhnelj6j997_NLil29vVAOBGeBBRgJwD-m_0_8B_AD-qOEw/rt=h/d=1/rs=AItRSTODy9wv1JKZMABIG3Ak8ViC4kuOWA?random=1395770800154
________________________________

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Ville Voutilainen]

On 8 May 2016 at 14:48, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
> Have you considered to test against decay instead of
> remove_reference/remove_const? That would be similar to other places
> in the standard. (I also believe that your fix actually should be
> submitted as an LWG issue)


STL is about to submit the fix as an LWG issue. He seemed to agree
that the fix is what
he intends to submit. Using decay instead of
remove_reference/remove_const is fine by
me, but I suppose it shouldn't make a difference here other than
notational brevity?

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Ville Voutilainen]

On 8 May 2016 at 14:51, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
> On 8 May 2016 at 14:48, Daniel Krügler <daniel.kruegler@gmail.com> wrote:
>> Have you considered to test against decay instead of
>> remove_reference/remove_const? That would be similar to other places
>> in the standard. (I also believe that your fix actually should be
>> submitted as an LWG issue)
>
>
> STL is about to submit the fix as an LWG issue. He seemed to agree
> that the fix is what
> he intends to submit. Using decay instead of
> remove_reference/remove_const is fine by
> me, but I suppose it shouldn't make a difference here other than
> notational brevity?

For what it's worth, I have the tiniest preference against using decay
here; whenever I see
decay, I wonder whether array/function decay is significant. While it
doesn't make a difference
here, I still prefer just doing remove_reference+remove_const here.
It's up to Jonathan, I'll change
it to decay if he so advises.

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Jonathan Wakely]

On 08/05/16 15:55 +0300, Ville Voutilainen wrote:
>For what it's worth, I have the tiniest preference against using decay
>here; whenever I see
>decay, I wonder whether array/function decay is significant. While it
>doesn't make a difference
>here, I still prefer just doing remove_reference+remove_const here.
>It's up to Jonathan, I'll change
>it to decay if he so advises.

At one point I felt the same about being more precise with
remove_ref+remove_const, but I just use decay now.

I don't have a preference either way (although if we added a __decay_t
alias for use in C++11 then I might be swayed to use that, as it would
be even more concise).

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Jonathan Wakely]

On 08/05/16 14:43 +0300, Ville Voutilainen wrote:
>Tested on Linux-PPC64.

OK for trunk, and we should backport to gcc-6-branch (and
gcc-5-branch?) soon.

Thanks.

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Ville Voutilainen]

On 9 May 2016 at 12:31, Jonathan Wakely <jwakely@redhat.com> wrote:
> On 08/05/16 14:43 +0300, Ville Voutilainen wrote:
>>
>> Tested on Linux-PPC64.
>
>
> OK for trunk, and we should backport to gcc-6-branch (and
> gcc-5-branch?) soon.


gcc-5-branch doesn't have the "Improving pair and tuple"
implementation at all yet, afaik.
I'm somewhat hesitant to backport all that.

Re: [v3 PATCH] Avoid endless run-time recursion for copying single-element tuples where the element type is by-value constructible from any type

[Jonathan Wakely]

On 09/05/16 13:41 +0300, Ville Voutilainen wrote:
>On 9 May 2016 at 12:31, Jonathan Wakely <jwakely@redhat.com> wrote:
>> On 08/05/16 14:43 +0300, Ville Voutilainen wrote:
>>>
>>> Tested on Linux-PPC64.
>>
>>
>> OK for trunk, and we should backport to gcc-6-branch (and
>> gcc-5-branch?) soon.
>
>
>gcc-5-branch doesn't have the "Improving pair and tuple"
>implementation at all yet, afaik.

OK, I couldn't remember when those changes happened.

>I'm somewhat hesitant to backport all that.

Yes, let's not do that. So this fix is only needed for gcc-6.