Re: tuple move constructor

Re: tuple move constructor

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1543 bytes --]

On Thu, 21 Apr 2016, Jonathan Wakely wrote:

> On 20 April 2016 at 21:42, Marc Glisse wrote:
>> Hello,
>>
>> does anyone remember why the move constructor of _Tuple_impl is not
>> defaulted? The attached patch does not cause any test to fail (whitespace
>> kept to avoid line number changes). Maybe something about tuples of
>> references?
>
> I don't know/remember why. It's possible it was to workaround a
> front-end bug that required it, or maybe just a mistake and it should
> always have been defaulted.

Ok, then how about something like this? In order to suppress the move
constructor in tuple (when there is a non-movable element), we need to
either declare it with suitable constraints, or keep it defaulted and
ensure that we don't bypass a missing move constructor anywhere along
the way (_Tuple_impl, _Head_base). There is a strange mix of 2
strategies in the patch, I prefer the tag class, but I started using
enable_if before I realized how many places needed those horrors.

Bootstrap+regtest on powerpc64le-unknown-linux-gnu.


2016-04-22  Marc Glisse  <marc.glisse@inria.fr>

 	* include/std/tuple (__element_arg_t): New class.
 	(_Head_base(const _Head&), _Tuple_impl(const _Head&, const _Tail&...):
 	Remove.
 	(_Head_base(_UHead&&)): Add __element_arg_t argument...
 	(_Tuple_impl): ... and adjust callers.
 	(_Tuple_impl(_Tuple_impl&&)): Default.
 	(_Tuple_impl(const _Tuple_impl<other>&),
 	_Tuple_impl(_Tuple_impl<other>&&), _Tuple_impl(_UHead&&): Constrain.
 	* testsuite/20_util/tuple/nomove.cc: New.

-- 
Marc Glisse

[-- Attachment #2: Type: TEXT/PLAIN, Size: 9306 bytes --]

Index: libstdc++-v3/include/std/tuple
===================================================================
--- libstdc++-v3/include/std/tuple	(revision 235346)
+++ libstdc++-v3/include/std/tuple	(working copy)
@@ -41,38 +41,37 @@
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /**
    *  @addtogroup utilities
    *  @{
    */
 
+  struct __element_arg_t { };
+
   template<std::size_t _Idx, typename _Head, bool _IsEmptyNotFinal>
     struct _Head_base;
 
   template<std::size_t _Idx, typename _Head>
     struct _Head_base<_Idx, _Head, true>
     : public _Head
     {
       constexpr _Head_base()
       : _Head() { }
 
-      constexpr _Head_base(const _Head& __h)
-      : _Head(__h) { }
-
       constexpr _Head_base(const _Head_base&) = default;
       constexpr _Head_base(_Head_base&&) = default;
 
       template<typename _UHead>
-        constexpr _Head_base(_UHead&& __h)
+        constexpr _Head_base(__element_arg_t, _UHead&& __h)
 	: _Head(std::forward<_UHead>(__h)) { }
 
       _Head_base(allocator_arg_t, __uses_alloc0)
       : _Head() { }
 
       template<typename _Alloc>
 	_Head_base(allocator_arg_t, __uses_alloc1<_Alloc> __a)
 	: _Head(allocator_arg, *__a._M_a) { }
 
       template<typename _Alloc>
@@ -97,28 +96,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static constexpr const _Head&
       _M_head(const _Head_base& __b) noexcept { return __b; }
     };
 
   template<std::size_t _Idx, typename _Head>
     struct _Head_base<_Idx, _Head, false>
     {
       constexpr _Head_base()
       : _M_head_impl() { }
 
-      constexpr _Head_base(const _Head& __h)
-      : _M_head_impl(__h) { }
-
       constexpr _Head_base(const _Head_base&) = default;
       constexpr _Head_base(_Head_base&&) = default;
 
       template<typename _UHead>
-        constexpr _Head_base(_UHead&& __h)
+        constexpr _Head_base(__element_arg_t, _UHead&& __h)
 	: _M_head_impl(std::forward<_UHead>(__h)) { }
 
       _Head_base(allocator_arg_t, __uses_alloc0)
       : _M_head_impl() { }
 
       template<typename _Alloc>
 	_Head_base(allocator_arg_t, __uses_alloc1<_Alloc> __a)
 	: _M_head_impl(allocator_arg, *__a._M_a) { }
 
       template<typename _Alloc>
@@ -194,50 +190,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       static constexpr _Inherited&
       _M_tail(_Tuple_impl& __t) noexcept { return __t; }
 
       static constexpr const _Inherited&
       _M_tail(const _Tuple_impl& __t) noexcept { return __t; }
 
       constexpr _Tuple_impl()
       : _Inherited(), _Base() { }
 
-      explicit 
-      constexpr _Tuple_impl(const _Head& __head, const _Tail&... __tail)
-      : _Inherited(__tail...), _Base(__head) { }
-
       template<typename _UHead, typename... _UTail, typename = typename
                enable_if<sizeof...(_Tail) == sizeof...(_UTail)>::type> 
         explicit
         constexpr _Tuple_impl(_UHead&& __head, _UTail&&... __tail)
 	: _Inherited(std::forward<_UTail>(__tail)...),
-	  _Base(std::forward<_UHead>(__head)) { }
+	  _Base(__element_arg_t(), std::forward<_UHead>(__head)) { }
 
       constexpr _Tuple_impl(const _Tuple_impl&) = default;
+      constexpr _Tuple_impl(_Tuple_impl&&) = default;
 
-      constexpr
-      _Tuple_impl(_Tuple_impl&& __in)
-      noexcept(__and_<is_nothrow_move_constructible<_Head>,
-	              is_nothrow_move_constructible<_Inherited>>::value)
-      : _Inherited(std::move(_M_tail(__in))), 
-	_Base(std::forward<_Head>(_M_head(__in))) { }
-
-      template<typename... _UElements>
+      template<typename... _UElements,
+	typename enable_if<
+	  !is_same<_Tuple_impl,
+		   _Tuple_impl<_Idx, _UElements...>>::value,
+	  bool>::type = false>
         constexpr _Tuple_impl(const _Tuple_impl<_Idx, _UElements...>& __in)
 	: _Inherited(_Tuple_impl<_Idx, _UElements...>::_M_tail(__in)),
-	  _Base(_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
+	  _Base(__element_arg_t(),
+		_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
 
-      template<typename _UHead, typename... _UTails>
+      template<typename _UHead, typename... _UTails,
+	       typename enable_if<
+		 !is_same<_Tuple_impl,
+			  _Tuple_impl<_Idx, _UHead, _UTails...>>::value,
+		 bool>::type = false>
         constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
 	: _Inherited(std::move
 		     (_Tuple_impl<_Idx, _UHead, _UTails...>::_M_tail(__in))),
-	  _Base(std::forward<_UHead>
+	  _Base(__element_arg_t(), std::forward<_UHead>
 		(_Tuple_impl<_Idx, _UHead, _UTails...>::_M_head(__in))) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a)
 	: _Inherited(__tag, __a),
           _Base(__tag, __use_alloc<_Head>(__a)) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a,
 		    const _Head& __head, const _Tail&... __tail)
@@ -344,43 +339,42 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       static constexpr _Head&
       _M_head(_Tuple_impl& __t) noexcept { return _Base::_M_head(__t); }
 
       static constexpr const _Head&
       _M_head(const _Tuple_impl& __t) noexcept { return _Base::_M_head(__t); }
 
       constexpr _Tuple_impl()
       : _Base() { }
 
-      explicit
-      constexpr _Tuple_impl(const _Head& __head)
-      : _Base(__head) { }
-
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_Tuple_impl,
+		  typename remove_cv<
+		    typename remove_reference<_UHead>::type>::type>::value,
+		bool>::type = false>
         explicit
         constexpr _Tuple_impl(_UHead&& __head)
-	: _Base(std::forward<_UHead>(__head)) { }
+	: _Base(__element_arg_t(), std::forward<_UHead>(__head)) { }
 
       constexpr _Tuple_impl(const _Tuple_impl&) = default;
+      constexpr _Tuple_impl(_Tuple_impl&&) = default;
 
-      constexpr
-      _Tuple_impl(_Tuple_impl&& __in)
-      noexcept(is_nothrow_move_constructible<_Head>::value)
-      : _Base(std::forward<_Head>(_M_head(__in))) { }
-
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_UHead, _Head>::value, bool>::type = false>
         constexpr _Tuple_impl(const _Tuple_impl<_Idx, _UHead>& __in)
-	: _Base(_Tuple_impl<_Idx, _UHead>::_M_head(__in)) { }
+	: _Base(__element_arg_t(), _Tuple_impl<_Idx, _UHead>::_M_head(__in)) { }
 
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_UHead, _Head>::value, bool>::type = false>
         constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead>&& __in)
-	: _Base(std::forward<_UHead>(_Tuple_impl<_Idx, _UHead>::_M_head(__in)))
+	: _Base(__element_arg_t(),
+		std::forward<_UHead>(_Tuple_impl<_Idx, _UHead>::_M_head(__in)))
 	{ }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a)
 	: _Base(__tag, __use_alloc<_Head>(__a)) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a,
 		    const _Head& __head)
 	: _Base(__use_alloc<_Head, _Alloc, _Head>(__a), __head) { }
Index: libstdc++-v3/testsuite/20_util/tuple/nomove.cc
===================================================================
--- libstdc++-v3/testsuite/20_util/tuple/nomove.cc	(revision 0)
+++ libstdc++-v3/testsuite/20_util/tuple/nomove.cc	(working copy)
@@ -0,0 +1,39 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }
+
+// 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>
+#include <type_traits>
+
+struct A { A(A&&)=delete; };
+struct B { int i; B(B&&)=delete; };
+
+static_assert(!std::is_copy_constructible<std::tuple<A>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<B>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<A,int>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<B,int>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<int,A>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<int,B>>::value);
+static_assert(!std::is_move_constructible<std::tuple<A>>::value);
+static_assert(!std::is_move_constructible<std::tuple<B>>::value);
+static_assert(!std::is_move_constructible<std::tuple<A,int>>::value);
+static_assert(!std::is_move_constructible<std::tuple<B,int>>::value);
+static_assert(!std::is_move_constructible<std::tuple<int,A>>::value);
+static_assert(!std::is_move_constructible<std::tuple<int,B>>::value);
+

Re: tuple move constructor

[Marc Glisse]

Hi Ville,

since you wrote the latest patches on tuple constructors, do you have an 
opinion on this patch, or alternate strategies to achieve the same goal?

https://gcc.gnu.org/ml/libstdc++/2016-04/msg00041.html


On Thu, 21 Apr 2016, Marc Glisse wrote:

> On Thu, 21 Apr 2016, Jonathan Wakely wrote:
>
>> On 20 April 2016 at 21:42, Marc Glisse wrote:
>>> Hello,
>>> 
>>> does anyone remember why the move constructor of _Tuple_impl is not
>>> defaulted? The attached patch does not cause any test to fail (whitespace
>>> kept to avoid line number changes). Maybe something about tuples of
>>> references?
>> 
>> I don't know/remember why. It's possible it was to workaround a
>> front-end bug that required it, or maybe just a mistake and it should
>> always have been defaulted.
>
> Ok, then how about something like this? In order to suppress the move
> constructor in tuple (when there is a non-movable element), we need to
> either declare it with suitable constraints, or keep it defaulted and
> ensure that we don't bypass a missing move constructor anywhere along
> the way (_Tuple_impl, _Head_base). There is a strange mix of 2
> strategies in the patch, I prefer the tag class, but I started using
> enable_if before I realized how many places needed those horrors.
>
> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>
>
> 2016-04-22  Marc Glisse  <marc.glisse@inria.fr>
>
> 	* include/std/tuple (__element_arg_t): New class.
> 	(_Head_base(const _Head&), _Tuple_impl(const _Head&, const 
> _Tail&...):
> 	Remove.
> 	(_Head_base(_UHead&&)): Add __element_arg_t argument...
> 	(_Tuple_impl): ... and adjust callers.
> 	(_Tuple_impl(_Tuple_impl&&)): Default.
> 	(_Tuple_impl(const _Tuple_impl<other>&),
> 	_Tuple_impl(_Tuple_impl<other>&&), _Tuple_impl(_UHead&&): Constrain.
> 	* testsuite/20_util/tuple/nomove.cc: New.

-- 
Marc Glisse

Re: tuple move constructor

[Ville Voutilainen]

On 6 May 2016 at 20:51, Marc Glisse <marc.glisse@inria.fr> wrote:
> Hi Ville,
>
> since you wrote the latest patches on tuple constructors, do you have an
> opinion on this patch, or alternate strategies to achieve the same goal?
>
> https://gcc.gnu.org/ml/libstdc++/2016-04/msg00041.html

I have fairly mixed feelings about the approach; it's adding a tag
type and more enable_ifs into the
base classes of tuple, which I'd rather not do unless absolutely
necessary. Then again, the testcase
you add looks like something we want to support properly. I haven't
analyzed your patch in a very detailed
manner; my initial thought was "can't we do this in the constraints of
tuple's constructors", but looking
at the patch and knowing the constructors of tuple, I don't think we can.

I think the patch is ok, but I think it would be a good idea to have a
comment on the added tag type and
its purpose.

Minor point: the technique that looks like

typename enable_if<
!is_same<_UHead, _Head>::value, bool>::type = false

isn't necessary unless we have another overload that we need to
distinguish with true/false. For a single overload,
just using typename = typename enable_if<
!is_same<_UHead, _Head>::value, bool>::type
works equally well. The amount of boilerplate is more or less the
same, so that's really not a significant matter,
just an fyi. :)

Re: tuple move constructor

[Marc Glisse]

On Fri, 6 May 2016, Ville Voutilainen wrote:

> On 6 May 2016 at 20:51, Marc Glisse <marc.glisse@inria.fr> wrote:
>> Hi Ville,
>>
>> since you wrote the latest patches on tuple constructors, do you have an
>> opinion on this patch, or alternate strategies to achieve the same goal?
>>
>> https://gcc.gnu.org/ml/libstdc++/2016-04/msg00041.html
>
> I have fairly mixed feelings about the approach; it's adding a tag type 
> and more enable_ifs into the base classes of tuple, which I'd rather not 
> do unless absolutely necessary. Then again, the testcase you add looks 
> like something we want to support properly. I haven't analyzed your 
> patch in a very detailed manner; my initial thought was "can't we do 
> this in the constraints of tuple's constructors", but looking at the 
> patch and knowing the constructors of tuple, I don't think we can.

Assuming we want the copy constructor to be defaulted, I think we still 
could with concepts:

tuple(tuple const&)
requires(__and_<is_copy_constructible<_Elements>...>::value)
= default;

While there is precedent for enabling C++11 features in C++03 mode inside 
system headers, I guess maintainers might be more reluctant for something 
that is only heading for a TS for now.

> I think the patch is ok, but I think it would be a good idea to have a 
> comment on the added tag type and its purpose.

Indeed. I wasn't sure if people preferred more tags or more enable_if...

> Minor point: the technique that looks like
>
> typename enable_if<
> !is_same<_UHead, _Head>::value, bool>::type = false
>
> isn't necessary unless we have another overload that we need to
> distinguish with true/false. For a single overload,
> just using typename = typename enable_if<
> !is_same<_UHead, _Head>::value, bool>::type
> works equally well. The amount of boilerplate is more or less the
> same, so that's really not a significant matter,
> just an fyi. :)

Thanks. There are several variants on how to use enable_if, I copied one 
from some neighboring code without checking why this specific variant was 
used there.

-- 
Marc Glisse

Re: tuple move constructor

[Ville Voutilainen]

On 7 May 2016 at 00:39, Marc Glisse <marc.glisse@inria.fr> wrote:
> Assuming we want the copy constructor to be defaulted, I think we still
> could with concepts:
>
> tuple(tuple const&)
> requires(__and_<is_copy_constructible<_Elements>...>::value)
> = default;
>
> While there is precedent for enabling C++11 features in C++03 mode inside
> system headers, I guess maintainers might be more reluctant for something
> that is only heading for a TS for now.

Much as I'd like to go towards that direction, I don't think we can,
yet, at least not as our default
implementation, because front-ends like clang wouldn't be able to
compile our library.

>> I think the patch is ok, but I think it would be a good idea to have a
>> comment on the added tag type and its purpose.
> Indeed. I wasn't sure if people preferred more tags or more enable_if...

I don't have a strong opinion if there's implementation choice between those.

Re: tuple move constructor

[Marc Glisse]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1727 bytes --]

Ping

(re-attaching, I just added a one-line comment before the tag class as 
asked by Ville)

On Thu, 21 Apr 2016, Marc Glisse wrote:

> On Thu, 21 Apr 2016, Jonathan Wakely wrote:
>
>> On 20 April 2016 at 21:42, Marc Glisse wrote:
>>> Hello,
>>> 
>>> does anyone remember why the move constructor of _Tuple_impl is not
>>> defaulted? The attached patch does not cause any test to fail (whitespace
>>> kept to avoid line number changes). Maybe something about tuples of
>>> references?
>> 
>> I don't know/remember why. It's possible it was to workaround a
>> front-end bug that required it, or maybe just a mistake and it should
>> always have been defaulted.
>
> Ok, then how about something like this? In order to suppress the move
> constructor in tuple (when there is a non-movable element), we need to
> either declare it with suitable constraints, or keep it defaulted and
> ensure that we don't bypass a missing move constructor anywhere along
> the way (_Tuple_impl, _Head_base). There is a strange mix of 2
> strategies in the patch, I prefer the tag class, but I started using
> enable_if before I realized how many places needed those horrors.
>
> Bootstrap+regtest on powerpc64le-unknown-linux-gnu.
>
>
> 2016-04-22  Marc Glisse  <marc.glisse@inria.fr>
>
> 	* include/std/tuple (__element_arg_t): New class.
> 	(_Head_base(const _Head&), _Tuple_impl(const _Head&, const _Tail&...):
> 	Remove.
> 	(_Head_base(_UHead&&)): Add __element_arg_t argument...
> 	(_Tuple_impl): ... and adjust callers.
> 	(_Tuple_impl(_Tuple_impl&&)): Default.
> 	(_Tuple_impl(const _Tuple_impl<other>&),
> 	_Tuple_impl(_Tuple_impl<other>&&), _Tuple_impl(_UHead&&): Constrain.
> 	* testsuite/20_util/tuple/nomove.cc: New.

-- 
Marc Glisse

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/x-diff; name=tuple.diff, Size: 9377 bytes --]

Index: libstdc++-v3/include/std/tuple
===================================================================
--- libstdc++-v3/include/std/tuple	(revision 236338)
+++ libstdc++-v3/include/std/tuple	(working copy)
@@ -41,38 +41,38 @@
 
 namespace std _GLIBCXX_VISIBILITY(default)
 {
 _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
   /**
    *  @addtogroup utilities
    *  @{
    */
 
+  // Tag type to distinguish forwarding constructors from copy/move.
+  struct __element_arg_t { };
+
   template<std::size_t _Idx, typename _Head, bool _IsEmptyNotFinal>
     struct _Head_base;
 
   template<std::size_t _Idx, typename _Head>
     struct _Head_base<_Idx, _Head, true>
     : public _Head
     {
       constexpr _Head_base()
       : _Head() { }
 
-      constexpr _Head_base(const _Head& __h)
-      : _Head(__h) { }
-
       constexpr _Head_base(const _Head_base&) = default;
       constexpr _Head_base(_Head_base&&) = default;
 
       template<typename _UHead>
-        constexpr _Head_base(_UHead&& __h)
+        constexpr _Head_base(__element_arg_t, _UHead&& __h)
 	: _Head(std::forward<_UHead>(__h)) { }
 
       _Head_base(allocator_arg_t, __uses_alloc0)
       : _Head() { }
 
       template<typename _Alloc>
 	_Head_base(allocator_arg_t, __uses_alloc1<_Alloc> __a)
 	: _Head(allocator_arg, *__a._M_a) { }
 
       template<typename _Alloc>
@@ -97,28 +97,25 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
       static constexpr const _Head&
       _M_head(const _Head_base& __b) noexcept { return __b; }
     };
 
   template<std::size_t _Idx, typename _Head>
     struct _Head_base<_Idx, _Head, false>
     {
       constexpr _Head_base()
       : _M_head_impl() { }
 
-      constexpr _Head_base(const _Head& __h)
-      : _M_head_impl(__h) { }
-
       constexpr _Head_base(const _Head_base&) = default;
       constexpr _Head_base(_Head_base&&) = default;
 
       template<typename _UHead>
-        constexpr _Head_base(_UHead&& __h)
+        constexpr _Head_base(__element_arg_t, _UHead&& __h)
 	: _M_head_impl(std::forward<_UHead>(__h)) { }
 
       _Head_base(allocator_arg_t, __uses_alloc0)
       : _M_head_impl() { }
 
       template<typename _Alloc>
 	_Head_base(allocator_arg_t, __uses_alloc1<_Alloc> __a)
 	: _M_head_impl(allocator_arg, *__a._M_a) { }
 
       template<typename _Alloc>
@@ -194,50 +191,49 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       static constexpr _Inherited&
       _M_tail(_Tuple_impl& __t) noexcept { return __t; }
 
       static constexpr const _Inherited&
       _M_tail(const _Tuple_impl& __t) noexcept { return __t; }
 
       constexpr _Tuple_impl()
       : _Inherited(), _Base() { }
 
-      explicit 
-      constexpr _Tuple_impl(const _Head& __head, const _Tail&... __tail)
-      : _Inherited(__tail...), _Base(__head) { }
-
       template<typename _UHead, typename... _UTail, typename = typename
                enable_if<sizeof...(_Tail) == sizeof...(_UTail)>::type> 
         explicit
         constexpr _Tuple_impl(_UHead&& __head, _UTail&&... __tail)
 	: _Inherited(std::forward<_UTail>(__tail)...),
-	  _Base(std::forward<_UHead>(__head)) { }
+	  _Base(__element_arg_t(), std::forward<_UHead>(__head)) { }
 
       constexpr _Tuple_impl(const _Tuple_impl&) = default;
+      constexpr _Tuple_impl(_Tuple_impl&&) = default;
 
-      constexpr
-      _Tuple_impl(_Tuple_impl&& __in)
-      noexcept(__and_<is_nothrow_move_constructible<_Head>,
-	              is_nothrow_move_constructible<_Inherited>>::value)
-      : _Inherited(std::move(_M_tail(__in))), 
-	_Base(std::forward<_Head>(_M_head(__in))) { }
-
-      template<typename... _UElements>
+      template<typename... _UElements,
+	typename enable_if<
+	  !is_same<_Tuple_impl,
+		   _Tuple_impl<_Idx, _UElements...>>::value,
+	  bool>::type = false>
         constexpr _Tuple_impl(const _Tuple_impl<_Idx, _UElements...>& __in)
 	: _Inherited(_Tuple_impl<_Idx, _UElements...>::_M_tail(__in)),
-	  _Base(_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
+	  _Base(__element_arg_t(),
+		_Tuple_impl<_Idx, _UElements...>::_M_head(__in)) { }
 
-      template<typename _UHead, typename... _UTails>
+      template<typename _UHead, typename... _UTails,
+	       typename enable_if<
+		 !is_same<_Tuple_impl,
+			  _Tuple_impl<_Idx, _UHead, _UTails...>>::value,
+		 bool>::type = false>
         constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
 	: _Inherited(std::move
 		     (_Tuple_impl<_Idx, _UHead, _UTails...>::_M_tail(__in))),
-	  _Base(std::forward<_UHead>
+	  _Base(__element_arg_t(), std::forward<_UHead>
 		(_Tuple_impl<_Idx, _UHead, _UTails...>::_M_head(__in))) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a)
 	: _Inherited(__tag, __a),
           _Base(__tag, __use_alloc<_Head>(__a)) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a,
 		    const _Head& __head, const _Tail&... __tail)
@@ -344,43 +340,42 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
 
       static constexpr _Head&
       _M_head(_Tuple_impl& __t) noexcept { return _Base::_M_head(__t); }
 
       static constexpr const _Head&
       _M_head(const _Tuple_impl& __t) noexcept { return _Base::_M_head(__t); }
 
       constexpr _Tuple_impl()
       : _Base() { }
 
-      explicit
-      constexpr _Tuple_impl(const _Head& __head)
-      : _Base(__head) { }
-
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_Tuple_impl,
+		  typename remove_cv<
+		    typename remove_reference<_UHead>::type>::type>::value,
+		bool>::type = false>
         explicit
         constexpr _Tuple_impl(_UHead&& __head)
-	: _Base(std::forward<_UHead>(__head)) { }
+	: _Base(__element_arg_t(), std::forward<_UHead>(__head)) { }
 
       constexpr _Tuple_impl(const _Tuple_impl&) = default;
+      constexpr _Tuple_impl(_Tuple_impl&&) = default;
 
-      constexpr
-      _Tuple_impl(_Tuple_impl&& __in)
-      noexcept(is_nothrow_move_constructible<_Head>::value)
-      : _Base(std::forward<_Head>(_M_head(__in))) { }
-
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_UHead, _Head>::value, bool>::type = false>
         constexpr _Tuple_impl(const _Tuple_impl<_Idx, _UHead>& __in)
-	: _Base(_Tuple_impl<_Idx, _UHead>::_M_head(__in)) { }
+	: _Base(__element_arg_t(), _Tuple_impl<_Idx, _UHead>::_M_head(__in)) { }
 
-      template<typename _UHead>
+      template<typename _UHead, typename enable_if<
+		!is_same<_UHead, _Head>::value, bool>::type = false>
         constexpr _Tuple_impl(_Tuple_impl<_Idx, _UHead>&& __in)
-	: _Base(std::forward<_UHead>(_Tuple_impl<_Idx, _UHead>::_M_head(__in)))
+	: _Base(__element_arg_t(),
+		std::forward<_UHead>(_Tuple_impl<_Idx, _UHead>::_M_head(__in)))
 	{ }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a)
 	: _Base(__tag, __use_alloc<_Head>(__a)) { }
 
       template<typename _Alloc>
 	_Tuple_impl(allocator_arg_t __tag, const _Alloc& __a,
 		    const _Head& __head)
 	: _Base(__use_alloc<_Head, _Alloc, _Head>(__a), __head) { }
Index: libstdc++-v3/testsuite/20_util/tuple/nomove.cc
===================================================================
--- libstdc++-v3/testsuite/20_util/tuple/nomove.cc	(revision 0)
+++ libstdc++-v3/testsuite/20_util/tuple/nomove.cc	(working copy)
@@ -0,0 +1,39 @@
+// { dg-do compile }
+// { dg-options "-std=gnu++11" }
+
+// 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>
+#include <type_traits>
+
+struct A { A(A&&)=delete; };
+struct B { int i; B(B&&)=delete; };
+
+static_assert(!std::is_copy_constructible<std::tuple<A>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<B>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<A,int>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<B,int>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<int,A>>::value);
+static_assert(!std::is_copy_constructible<std::tuple<int,B>>::value);
+static_assert(!std::is_move_constructible<std::tuple<A>>::value);
+static_assert(!std::is_move_constructible<std::tuple<B>>::value);
+static_assert(!std::is_move_constructible<std::tuple<A,int>>::value);
+static_assert(!std::is_move_constructible<std::tuple<B,int>>::value);
+static_assert(!std::is_move_constructible<std::tuple<int,A>>::value);
+static_assert(!std::is_move_constructible<std::tuple<int,B>>::value);
+

Re: tuple move constructor

[Jonathan Wakely]

On 23/05/16 20:39 +0200, Marc Glisse wrote:
>Ping
>
>(re-attaching, I just added a one-line comment before the tag class as 
>asked by Ville)

This is OK for trunk - thanks.

Re: tuple move constructor

[Jonathan Wakely]

On 25/05/16 14:54 +0100, Jonathan Wakely wrote:
>On 23/05/16 20:39 +0200, Marc Glisse wrote:
>>Ping
>>
>>(re-attaching, I just added a one-line comment before the tag class 
>>as asked by Ville)
>
>This is OK for trunk - thanks.

On second thoughts - does this change the passing conventions for
std::tuple if it gets a trivial move ctor?

Re: tuple move constructor

[Marc Glisse]

On Thu, 26 May 2016, Jonathan Wakely wrote:

> On 25/05/16 14:54 +0100, Jonathan Wakely wrote:
>> On 23/05/16 20:39 +0200, Marc Glisse wrote:
>>> Ping
>>> 
>>> (re-attaching, I just added a one-line comment before the tag class as 
>>> asked by Ville)
>> 
>> This is OK for trunk - thanks.
>
> On second thoughts - does this change the passing conventions for
> std::tuple if it gets a trivial move ctor?

Note that this part of the ABI is ill-defined
http://sourcerytools.com/pipermail/cxx-abi-dev/2016-February/002884.html

but yes, good catch, it does change the passing convention (by value), and 
not just for weirdo types, it even changes for tuple<int>. It is clearly a 
change in the right direction, not passing tuple<int> in a register is 
weird, but yeah, compatibility :-(

I don't even want to think of trying to fix this issue in C++11 while 
artificially preserving the non-triviality of tuple, the headache is not 
worth it. I guess I'll open an entry in bugzilla with the ABI tag and let 
it rot there...

Maybe we could

#if __cpp_concepts >= 201500
the alternative discussed with Ville
#endif

but that won't fix the fact that tuple<int> should be trivially move 
constructible...

We could add __attribute__(non_trivial_for_purpose_of_passing_convention), 
but I think abi_tag has already stretched enough the idea that gcc is 
following the itanium abi.

Bah, forget this patch. Thanks for noticing early, that spares me the 
trouble of reverting later.

-- 
Marc Glisse

Re: tuple move constructor

[Jonathan Wakely]

On 26/05/16 19:49 +0200, Marc Glisse wrote:
>On Thu, 26 May 2016, Jonathan Wakely wrote:
>
>>On 25/05/16 14:54 +0100, Jonathan Wakely wrote:
>>>On 23/05/16 20:39 +0200, Marc Glisse wrote:
>>>>Ping
>>>>
>>>>(re-attaching, I just added a one-line comment before the tag 
>>>>class as asked by Ville)
>>>
>>>This is OK for trunk - thanks.
>>
>>On second thoughts - does this change the passing conventions for
>>std::tuple if it gets a trivial move ctor?
>
>Note that this part of the ABI is ill-defined
>http://sourcerytools.com/pipermail/cxx-abi-dev/2016-February/002884.html
>
>but yes, good catch, it does change the passing convention (by value), 
>and not just for weirdo types, it even changes for tuple<int>. It is 
>clearly a change in the right direction, not passing tuple<int> in a 
>register is weird, but yeah, compatibility :-(
>
>I don't even want to think of trying to fix this issue in C++11 while 
>artificially preserving the non-triviality of tuple, the headache is 
>not worth it. I guess I'll open an entry in bugzilla with the ABI tag 
>and let it rot there...

Please do.

>Maybe we could
>
>#if __cpp_concepts >= 201500
>the alternative discussed with Ville
>#endif
>
>but that won't fix the fact that tuple<int> should be trivially move 
>constructible...
>
>We could add 
>__attribute__(non_trivial_for_purpose_of_passing_convention), but I 
>think abi_tag has already stretched enough the idea that gcc is 
>following the itanium abi.
>
>Bah, forget this patch. Thanks for noticing early, that spares me the 
>trouble of reverting later.

I have a dream of resurrecting the gnu-versioned-namespace config (and
bumping from std::__7 / libstdc++.so.7 to std::__8 / libstdc++.so.8)
so that people who don't want backwards compatibility can use that
mode, which would enable various nice optimizations that can't be made
in the default mode because of compatibility.

This would be something we should change when that config is in use.
So hopefully if you open a bugzilla entry it won't rot forever, but
only until I realise my dream.