[Bug libstdc++/52591] New: [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[Bug libstdc++/52591] New: [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[expipiplus1 at gmail dot com]

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

             Bug #: 52591
           Summary: [C++0x] [4.7 Regression] moving std::vector relies on
                    movable elements
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: major
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: expipiplus1@gmail.com


Created attachment 26894
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26894
A simple testcase.

libstdc++ has a code branch in the move assignment operator of vector which
depends on the move assignment operator of the element type.
I'm compiling with 'g++ -std=c++11 test.cpp'

The attached code compiles with no problems using either:
'g++-4.6 -std=c++0x test.cpp' or 
'clang++ -std=c++11 -stdlib=libc++ test.cpp'


The wonderful template error message:
In file included from
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/vector:61:0,
                 from move.cpp:1:
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_algobase.h:
In instantiation of 'static _OI std::__copy_move<true, false,
std::random_access_iterator_tag>::__copy_m(_II, _II, _OI) [with _II = C*; _OI =
C*]':
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_algobase.h:384:70:
  required from '_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove =
true; _II = C*; _OI = C*]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_algobase.h:422:39:
  required from '_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove =
true; _II = __gnu_cxx::__normal_iterator<C*, std::vector<C> >; _OI = C*]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_algobase.h:454:18:
  required from '_OI std::copy(_II, _II, _OI) [with _II =
std::move_iterator<__gnu_cxx::__normal_iterator<C*, std::vector<C> > >; _OI =
C*]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/vector.tcc:275:4:
  required from 'void std::vector<_Tp, _Alloc>::_M_assign_aux(_ForwardIterator,
_ForwardIterator, std::forward_iterator_tag) [with _ForwardIterator =
std::move_iterator<__gnu_cxx::__normal_iterator<C*, std::vector<C> > >; _Tp =
C; _Alloc = std::allocator<C>]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_vector.h:1235:4:
  required from 'void std::vector<_Tp,
_Alloc>::_M_assign_dispatch(_InputIterator, _InputIterator, std::__false_type)
[with _InputIterator = std::move_iterator<__gnu_cxx::__normal_iterator<C*,
std::vector<C> > >; _Tp = C; _Alloc = std::allocator<C>]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_vector.h:506:4:
  required from 'void std::vector<_Tp, _Alloc>::assign(_InputIterator,
_InputIterator) [with _InputIterator =
std::move_iterator<__gnu_cxx::__normal_iterator<C*, std::vector<C> > >; _Tp =
C; _Alloc = std::allocator<C>]'
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_vector.h:448:6:
  required from 'std::vector<_Tp, _Alloc>& std::vector<_Tp,
_Alloc>::operator=(std::vector<_Tp, _Alloc>&&) [with _Tp = C; _Alloc =
std::allocator<C>; std::vector<_Tp, _Alloc> = std::vector<C>]'
move.cpp:17:38:   required from here
/usr/local/lib/gcc/x86_64-linux-gnu/4.7.0/../../../../include/c++/4.7.0/bits/stl_algobase.h:348:8:
error: use of deleted function 'C& C::operator=(const C&)'
move.cpp:4:7: note: 'C& C::operator=(const C&)' is implicitly deleted because
the default definition would be ill-formed:
move.cpp:4:7: error: non-static reference member 'int& C::m_r', can't use
default assignment operator

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[paolo.carlini at oracle dot com]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jwakely.gcc at gmail dot
                   |                            |com

--- Comment #1 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-14 22:43:45 UTC ---
Jon, can you have a look? I suspect it's just matter of telling apart cases in
the move-assignment operator at compile time with templates instead of a normal
conditional.

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[expipiplus1 at gmail dot com]

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

Joe Hermaszewski <expipiplus1 at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Version|4.7.0                       |4.8.0

--- Comment #2 from Joe Hermaszewski <expipiplus1 at gmail dot com> 2012-03-14 23:34:32 UTC ---
Nothing's changed in the move assignment operator in 4.8, the error remains the
same.

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[paolo.carlini at oracle dot com]

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

--- Comment #3 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-14 23:46:59 UTC ---
You bet it ;)

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-03-15
                 CC|jwakely.gcc at gmail dot    |
                   |com                         |
         AssignedTo|unassigned at gcc dot       |redi at gcc dot gnu.org
                   |gnu.org                     |
   Target Milestone|---                         |4.7.1
     Ever Confirmed|0                           |1

--- Comment #4 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 00:29:36 UTC ---
Hmm, it shouldn't be taking that branch anyway, the allocators should be equal.

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 00:36:22 UTC ---
(In reply to comment #1)
> Jon, can you have a look? I suspect it's just matter of telling apart cases in
> the move-assignment operator at compile time with templates instead of a normal
> conditional.

Oh I see what you mean, the untaken branch is instantiated anyway.

It's not possible to tell if the allocators are equal at compile-time and until
we implement DR 2103 (I have a patch) propagate_on_container_move_assignment is
false for std::allocator, so that branch will be taken in general.

Anyway, I'll look into it...

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 00:51:23 UTC ---
The allocator-aware container requirements (table 99) state that move-assigning
a vector requires that the element type be MoveAssignable

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 00:58:15 UTC ---
std::vector did not conform to the C++11 allocator-aware container requirements
in 4.6, now it does, so you can't use a non-MoveAssignable type as the element
type.

It would be possible to dispatch to a different function for the cases when
propagate_on_container_move_assignment is true or the allocators are known to
always compare equal (which I suspect libc++ does) and I'll consider doing that
as a conforming extension, but I think the testcase is invalid.

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[paolo.carlini at oracle dot com]

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

--- Comment #8 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-03-15 01:01:36 UTC ---
I see. That conforming extension seems nice to have, anyway, I agree (together
with 2103)

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #9 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 01:20:33 UTC ---
Created attachment 26895
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=26895
barely tested patch that allows testcase to compile

here's a prototype, I'll come back to it later

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|major                       |enhancement

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-03-15 01:25:34 UTC ---
N.B. that relies on the "always equal" extension for allocators which I already
implemented (and reported as DR 2108)

when we implement DR 2103 it will work because POCMA will be true for
std::allocator

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-04-01 22:04:59 UTC ---
Author: redi
Date: Sun Apr  1 22:04:54 2012
New Revision: 186057

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186057
Log:
    PR libstdc++/52591
    * include/bits/stl_vector.h (vector::operator=(vector&&)): Dispatch
    to _M_move_assign depending on whether allocator is moved.
    (vector::_M_move_assign): Add overloaded functions.
    * testsuite/23_containers/vector/52591.cc: New.
    * testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
    Adjust dg-error line number.
    * testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc:
    Likewise.
    * testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc:
    Likewise.
    * testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
    Likewise.

Added:
    trunk/libstdc++-v3/testsuite/23_containers/vector/52591.cc
      - copied, changed from r186055,
trunk/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/stl_vector.h
   
trunk/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
   
trunk/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
   
trunk/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
   
trunk/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #12 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-04-01 22:12:29 UTC ---
fixed on the trunk so far

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-04-11 21:13:50 UTC ---
Author: redi
Date: Wed Apr 11 21:13:45 2012
New Revision: 186359

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=186359
Log:
    PR libstdc++/52591
    * include/bits/stl_vector.h (vector::operator=(vector&&)): Dispatch
    to _M_move_assign depending on whether allocator is moved.
    (vector::_M_move_assign): Add overloaded functions.
    * testsuite/23_containers/vector/52591.cc: New.
    * testsuite/23_containers/vector/requirements/dr438/assign_neg.cc:
    Adjust dg-error line number.
    * testsuite/23_containers/vector/requirements/dr438/
        constructor_1_neg.cc: Likewise.
    * testsuite/23_containers/vector/requirements/dr438/
        constructor_2_neg.cc: Likewise.
    * testsuite/23_containers/vector/requirements/dr438/insert_neg.cc:
    Likewise.

Added:
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/52591.cc
      - copied, changed from r186300,
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc
Modified:
    branches/gcc-4_7-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_7-branch/libstdc++-v3/include/bits/stl_vector.h
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/assign_neg.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_1_neg.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/constructor_2_neg.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/vector/requirements/dr438/insert_neg.cc

[Bug libstdc++/52591] [C++0x] [4.7 Regression] moving std::vector relies on movable elements

[redi at gcc dot gnu.org]

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

Jonathan Wakely <redi at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED

--- Comment #14 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-04-11 21:14:44 UTC ---
fixed for 4.7.1