[Bug libstdc++/48476] New: [C++0x] conversion between std::tuple which have reference member is rejected

[Bug libstdc++/48476] New: [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

           Summary: [C++0x] conversion between std::tuple which have
                    reference member is rejected
           Product: gcc
           Version: 4.6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: gintensubaru@gmail.com


Created attachment 23893
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23893
Proposed patch for libstdc++-v3/include/std/tuple.hpp

gcc-4.6.0 rejects this code (bug.cc):

---------------------

#include <tuple>

int main()
{
  int i = 0;
  std::tuple<int&, int> t = std::forward_as_tuple( i, 0 );
}

---------------------

# g++ -std=c++0x bug.cc

In file included from bug.cc:1:0:
/usr/local/gcc46/lib/gcc/i686-pc-cygwin/4.6.0/../../../../include/c++/4.6.0/tuple:
In constructor 'std::_Head_base<_Idx, _Head, false>::_Head_base(_UHead&&) [with
_UHead = int, unsigned int _Idx = 0u, _Head = int&]':
/usr/local/gcc46/lib/gcc/i686-pc-cygwin/4.6.0/../../../../include/c++/4.6.0/tuple:183:35:
  instantiated from 'std::_Tuple_impl<_Idx, _Head, _Tail
...>::_Tuple_impl(std::_Tuple_impl<_Idx, _UElements ...>&&) [with _UElements =
{int&, int&&}, unsigned int _Idx = 0u, _Head = int&, _Tail = {int}]'
/usr/local/gcc46/lib/gcc/i686-pc-cygwin/4.6.0/../../../../include/c++/4.6.0/tuple:343:60:
  instantiated from 'std::tuple<_T1, _T2>::tuple(std::tuple<_U1, _U2>&&) [with
_U1 = int&, _U2 = int&&, _T1 = int&, _T2 = int]'
bug.cc:6:57:   instantiated from here
/usr/local/gcc46/lib/gcc/i686-pc-cygwin/4.6.0/../../../../include/c++/4.6.0/tuple:101:42:
error: invalid initialization of non-const reference of type 'int&' from an
rvalue of type 'int'

---------------------


I found a conversion ctor of '_Tuple_impl<_Idx, _Head, _Tail...>',

---------------------

// libstdc++-v3/include/std/tuple.hpp, line 180

template<typename... _UElements>
  _Tuple_impl(_Tuple_impl<_Idx, _UElements...>&& __in)
  : _Inherited(std::move(__in._M_tail())),
    _Base(std::move(__in._M_head())) { }

---------------------

is wrong, when the first type of '_UElements...' is lvalue reference.
It should be implemented with std::forward, not std::move, like this:

---------------------

template<typename _UHead, typename... _UTails>
  _Tuple_impl(_Tuple_impl<_Idx, _UHead, _UTails...>&& __in)
  : _Inherited(std::move(__in._M_tail())),
    _Base(std::forward<_UHead>(__in._M_head())) { }

---------------------

and it would be work better.


And I found these kinds of bugs also appear in implementations of
assignment-ops
and std::tuple_cat, so I attach a brief patch to fix them ( it might have
some problems, because I have not tested it enough ).
I'm glad if it could be some help.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-06 07:38:39 UTC ---
Hi. Not having looked into it in detail yet, your patch looks indeed
reasonable. Do you have more testcases, for at lease some of the other issues
you noticed?

Indeed, I really meant to go again through std::tuple, I was nervous about some
of those move and forward... Can you double check tuple_cat and tuple itself
about that? I'm seeing some quite suspect moves, compared to the current draft.
Thanks.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #2 from Takaya Saito <gintensubaru at gmail dot com> 2011-04-06 08:32:09 UTC ---
Created attachment 23896
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23896
simple test for operator=( Tuple&& )

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #3 from Takaya Saito <gintensubaru at gmail dot com> 2011-04-06 08:32:57 UTC ---
Created attachment 23897
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23897
simple test for std::tuple_cat

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

Takaya Saito <gintensubaru at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23896|application/octet-stream    |text/plain
          mime type|                            |

--- Comment #4 from Takaya Saito <gintensubaru at gmail dot com> 2011-04-06 09:05:19 UTC ---
Comment on attachment 23896
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23896
simple test for operator=( Tuple&& )

in gcc-4.6.0, this code fails assertion 'q == p' and 'r == q',
because references to p and q are not forwarded but moved
( see N3242 20.4.2.2 ).

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

Takaya Saito <gintensubaru at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #23897|application/octet-stream    |text/plain
          mime type|                            |

--- Comment #5 from Takaya Saito <gintensubaru at gmail dot com> 2011-04-06 09:07:03 UTC ---
Comment on attachment 23897
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=23897
simple test for std::tuple_cat

gcc-4.6.0 rejects this code.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-06 14:07:19 UTC ---
Ok, thanks. Still, I believe we have other std::move which should be turned
into forward, in std::tuple. Those in std::tuple_cat itself also seem suspect,
I see you are touching only the helper.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[gintensubaru at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #7 from Takaya Saito <gintensubaru at gmail dot com> 2011-04-06 15:32:00 UTC ---
(In reply to comment #6)
> Ok, thanks. Still, I believe we have other std::move which should be turned
> into forward, in std::tuple. Those in std::tuple_cat itself also seem suspect,
> I see you are touching only the helper.

Well, I think I had replaced all std::move that should be turned into forward ;
remaining std::move are moving _Inherited ( _Tuple_impl<_Idx + 1, _Tail...> ), 
tuple<Elements...> or rvalue reference to these classes, which are not lvalue
reference type, so they are not to be replaced ( of course, it is not wrong
to replace them with std::forward, since std::move(x) is equivalent to
std::forward<T>(x) if T is remove_reference<decltype((x))>::type ).

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011.04.06 16:29:15
     Ever Confirmed|0                           |1

--- Comment #8 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-06 16:29:15 UTC ---
Fair enough. I think the safe thing to do here is proceeding incrementally:
today I will be testing your patch (+ testcases) and I think it's small enough
to bd suited for 4_6-branch too. Then we can certainly imagine other
improvements. If you notice something else which could be improved over the
next weeks, please let us know, I mean to pay more attention to std::tuple than
I used to lately, sadly.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED

--- Comment #9 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-12 09:10:38 UTC ---
On it.

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #10 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-12 10:31:37 UTC ---
Author: paolo
Date: Tue Apr 12 10:31:33 2011
New Revision: 172309

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172309
Log:
2011-04-12  Takaya Saito  <gintensubaru@gmail.com>

    PR libstdc++/48476
    * include/std/tuple (_Tuple_impl<>::_Tuple_impl(_Tuple_impl<>&&),
    _Tuple_impl<>::operator=(_Tuple_impl&&), _Tuple_impl<>::operator=
    (_Tuple_impl<>&&), tuple_cat): Use std::forward where appropriate.
    * testsuite/20_util/tuple/cons/48476.cc: New.
    * testsuite/20_util/tuple/48476.cc: Likewise.
    * testsuite/20_util/tuple/creation_functions/48476.cc: Likewise.

Added:
    trunk/libstdc++-v3/testsuite/20_util/tuple/48476.cc
    trunk/libstdc++-v3/testsuite/20_util/tuple/cons/48476.cc
    trunk/libstdc++-v3/testsuite/20_util/tuple/creation_functions/48476.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/tuple

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

--- Comment #11 from paolo at gcc dot gnu.org <paolo at gcc dot gnu.org> 2011-04-15 14:53:02 UTC ---
Author: paolo
Date: Fri Apr 15 14:52:57 2011
New Revision: 172498

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=172498
Log:
2011-04-15  Takaya Saito  <gintensubaru@gmail.com>

    PR libstdc++/48476
    * include/std/tuple (_Tuple_impl<>::_Tuple_impl(_Tuple_impl<>&&),
    _Tuple_impl<>::operator=(_Tuple_impl&&), _Tuple_impl<>::operator=
    (_Tuple_impl<>&&), tuple_cat): Use std::forward where appropriate.
    * testsuite/20_util/tuple/cons/48476.cc: New.
    * testsuite/20_util/tuple/48476.cc: Likewise.
    * testsuite/20_util/tuple/creation_functions/48476.cc: Likewise.

Added:
    branches/gcc-4_6-branch/libstdc++-v3/testsuite/20_util/tuple/48476.cc
    branches/gcc-4_6-branch/libstdc++-v3/testsuite/20_util/tuple/cons/48476.cc
   
branches/gcc-4_6-branch/libstdc++-v3/testsuite/20_util/tuple/creation_functions/48476.cc
Modified:
    branches/gcc-4_6-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_6-branch/libstdc++-v3/include/std/tuple

[Bug libstdc++/48476] [C++0x] conversion between std::tuple which have reference member is rejected

[paolo.carlini at oracle dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=48476

Paolo Carlini <paolo.carlini at oracle dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.6.1

--- Comment #12 from Paolo Carlini <paolo.carlini at oracle dot com> 2011-04-15 14:54:10 UTC ---
Done.