[Bug libstdc++/59872] New: Cannot move std::map with move-only mapped_type

[Bug libstdc++/59872] New: Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

            Bug ID: 59872
           Summary: Cannot move std::map with move-only mapped_type
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: potswa at mac dot com

This fails because the std::map move constructor apparently invokes a copy
operation. It was working recently (on trunk), so it looks like a regression.


#include <map>
#include <memory>

std::map< int, std::unique_ptr< int > > q, r( std::move( q ) );

[Bug libstdc++/59872] Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

--- Comment #1 from David Krauss <potswa at mac dot com> ---
It looks like the allocator-aware update introduced a copying branch to the
move constructor, to handle the case of transferring an object from one
stateful allocator to another.

The branch on _Alloc_traits::_S_always_equal(), presumably a constant value,
should be split into two functions. I'll continue digging in, I don't yet
understand _Alloc_trats or why _S_always_equal is a private interface.

[Bug libstdc++/59872] Cannot move std::map with move-only mapped_type

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> ---
Yes, the branch that copies needs to be a separate function.

http://cplusplus.github.io/LWG/lwg-active.html#2108 might answer your last
question

[Bug libstdc++/59872] Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

--- Comment #3 from David Krauss <potswa at mac dot com> ---
Thanks, I'm working on it now.

Is there some precedent for this kind of SFINAE in libstdc++? The hard part is
getting the style right. Metaprogramming looks weird in 80 columns.

As for the defect report, my thoughts exactly. Maybe we can extend
__allocator_always_compares_equal to evaluate such an equality check in a
SFINAE, constant expression context. Then a user-defined

constexpr bool operator == ( my_alloc, my_alloc )
    { return true; }

will satisfy the condition, but anything else will default.

[Bug libstdc++/59872] Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

--- Comment #4 from David Krauss <potswa at mac dot com> ---
Created attachment 31884
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31884&action=edit
SFINAE based fix

This seems stylistically OK, but I did copy-paste the implementation of moving.
Perhaps the common code should be factored into a new function, or the
constructor should simply call into move assignment.

It fixes the problem but I was unable to run the testsuite. This is a brand-new
installation and I haven't used it for contribution yet. (It says "WARNING:
could not find `runtest'".)

[Bug libstdc++/59872] Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

David Krauss <potswa at mac dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31884|0                           |1
        is obsolete|                            |

--- Comment #5 from David Krauss <potswa at mac dot com> ---
Created attachment 31885
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31885&action=edit
SFINAE based fix

[Bug libstdc++/59872] [4.9 Regression] Cannot move std::map with move-only mapped_type

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2014-01-19
           Assignee|unassigned at gcc dot gnu.org      |redi at gcc dot gnu.org
   Target Milestone|---                         |4.9.0
            Summary|Cannot move std::map with   |[4.9 Regression] Cannot
                   |move-only mapped_type       |move std::map with
                   |                            |move-only mapped_type
     Ever confirmed|0                           |1

[Bug libstdc++/59872] [4.9 Regression] Cannot move std::map with move-only mapped_type

[redi at gcc dot gnu.org]

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

--- Comment #6 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I don't want to use SFINAE here, I'll fix it the same way it's done in the
other containers, tag dispatching using
  std::integral_constant<bool, _Alloc_Traits::_S_always_Equal()>

[Bug libstdc++/59872] [4.9 Regression] Cannot move std::map with move-only mapped_type

[potswa at mac dot com]

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

--- Comment #7 from David Krauss <potswa at mac dot com> ---
That's a better factoring. I was just avoiding creating a new, named function.
Thanks!

[Bug libstdc++/59872] [4.9 Regression] Cannot move std::map with move-only mapped_type

[redi at gcc dot gnu.org]

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

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

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

--- Comment #10 from Jonathan Wakely <redi at gcc dot gnu.org> ---
fixed