[Bug libstdc++/55043] New: issue with nesting unordered_map containing unique_ptr into vector

[Bug libstdc++/55043] New: issue with nesting unordered_map containing unique_ptr into vector

[boris.bigott at hotmail dot com]

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

             Bug #: 55043
           Summary: issue with nesting unordered_map containing unique_ptr
                    into vector
    Classification: Unclassified
           Product: gcc
           Version: 4.7.2
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libstdc++
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: boris.bigott@hotmail.com


The program below does not compile using gcc-4.7.2 or clang-3.1 together with
libstdc++-4.7.2. Using gcc-4.6.3 with corresponding libstdc++ or clang-3.1 with
clangs libc++ works. Nesting uim in other containers like pair, tuple, or
unordered_map works too. Apparently, only vector is affected.

#include <memory>
#include <unordered_map>
#include <vector>

int main() {
    typedef std::unique_ptr<int> uip;
    typedef std::unordered_map<int, uip> uim;
    typedef std::vector<uim> uiv;
    uiv v;
    v.emplace_back(uim());
}

[Bug libstdc++/55043] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-10-23
         AssignedTo|unassigned at gcc dot       |redi at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-23 21:35:03 UTC ---
possibly allocator-related, I'll look into it ...

[Bug libstdc++/55043] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #2 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-23 22:28:52 UTC ---
This is due to the fact that _Hashtable::_Hashtable(_Hashtable&&) is not
declared noexcept.

I think we can just add 'noexcept' to the move ctor and move assignment op, at
least until I add proper C++11 allocator support to the unordered containers.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|issue with nesting          |[4.7/4.8 Regression] issue
                   |unordered_map containing    |with nesting unordered_map
                   |unique_ptr into vector      |containing unique_ptr into
                   |                            |vector

--- Comment #3 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-23 22:35:11 UTC ---
marking as a regression because it used to work until vector implemented the
correct C++11 allocator rules

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[paolo.carlini at oracle dot com]

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

--- Comment #4 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-23 22:53:06 UTC ---
Unfortunately, I don't think we can simply add noexcept to the move ctor,
because it does allocate memory. Something similar happens for std::deque. In
terms of QoI I think we could do better (with a non-trivial redesign), but I
don't think it's non-conforming for the move ctor of these containers to not be
noexcept.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #5 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-23 23:06:43 UTC ---
Oh dear.

If a type is not nothrow MoveInsertable and is not CopyInsertable, should we
just move it, with unspecified behaviour if an except is thrown?

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[paolo.carlini at oracle dot com]

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

--- Comment #6 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-23 23:14:17 UTC ---
I suppose the logic could be the same of move_if_noexcept? In any case, I guess
we have to handle such unlikely circumstances, somebody could well instantiate
with such kind of user-defined types...

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #7 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 01:08:13 UTC ---
It already does use move_if_noexcept, so should work.

The problem is that
std::is_copy_constructible<std::unordered_set<std::unique_ptr<int>>>::value is
true, so __move_if_noexcept_cond is true for unordered containers and we try to
copy them.

That happens because this is well-formed:

    typedef std::unordered_set<std::unique_ptr<int>> S;
    decltype(::new S(std::declval<const S&>())) s;

That should be ill-formed, but for some reason isn't when it occurs as an
unevaluated operand.

This is ill-formed, as expected:

    S ss;
    ::new S(static_cast<const S&>(ss));

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daniel.kruegler at
                   |                            |googlemail dot com

--- Comment #8 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 01:24:02 UTC ---
The problem reduces to this:

  #include <type_traits>
  #include <utility>

  struct M {
    M(M&&) { }
  };

  template<typename T>
    struct U
    {
      U(const U& n) : m(n.m) { }
      U(U&& n);
      T m;
    };

  static_assert( std::is_copy_constructible<U<M>>::value, "huh?" );

We need that static_assertion to fail because the type is *not* copy
constructible.

But it passes, because this compiles OK:

    decltype(::new U<M>(std::declval<const U<M>&>())) n;


Even though the definition of U(const U&) is ill-formed for a non-copyable
template argument, using that constructor as an unevaluated operand compiles
OK.

Daniel, is this a bug in is_constructible?

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[glisse at gcc dot gnu.org]

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

--- Comment #9 from Marc Glisse <glisse at gcc dot gnu.org> 2012-10-24 03:04:16 UTC ---
(In reply to comment #8)
> is this a bug in is_constructible?

I don't think so. The implementation of the copy constructor is irrelevant,
only the prototype matters to determine copy-constructibility. The "best"
changing the trait could get is a hard (not sfinae) error...

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #10 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2012-10-24 06:37:05 UTC ---
(In reply to comment #9)
> (In reply to comment #8)
> > is this a bug in is_constructible?
> 
> I don't think so. The implementation of the copy constructor is irrelevant,
> only the prototype matters to determine copy-constructibility.

I agree with Marc, the trait can only test what the compile-time interface
says. This *could* be an LWG issue and one possible solution would be to
require that implementations ensure that for a non-copyable element type a
container's copy constructor is deleted. This is technically possible, but also
has the unfortunate side-effect that existing container implementations for
std::vector, std::list, etc. could after this change *never* be used in
situations where the element type is still incomplete (albeit not granted by
the standard, many implementations allow this), like

#include <list>
#include <vector>

struct S {
  std::list<S> li;
  std::vector<S> v;
} s1, s2 = s1;

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #11 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 08:25:04 UTC ---
(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > is this a bug in is_constructible?
> > 
> > I don't think so. The implementation of the copy constructor is irrelevant,
> > only the prototype matters to determine copy-constructibility.
> 
> I agree with Marc, the trait can only test what the compile-time interface
> says.

That seems inconsistent with 20.9.4.3/6

the predicate condition for a template specialization is_constructible<T,
Args...> shall be satisfied if and only if the following variable definition
would be well-formed for some invented variable t:
    T t(create<Args>()...);

and that variable definition is not well-formed:

  const N<M>& create();
  N<M> t(create());

u.cc: In instantiation of ‘N<T>::N(const N<T>&) [with T = M]’:
u.cc:17:16:   required from here
u.cc:11:24: error: use of deleted function ‘constexpr M::M(const M&)’
   N(const N& n) : m(n.m) { }
                        ^


I guess the solution is to add enable_if-style constraints to the copy
constructors, everywhere.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #12 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2012-10-24 08:33:47 UTC ---
(In reply to comment #11)
> That seems inconsistent with 20.9.4.3/6
> 
> the predicate condition for a template specialization is_constructible<T,
> Args...> shall be satisfied if and only if the following variable definition
> would be well-formed for some invented variable t:
>     T t(create<Args>()...);
> 
> and that variable definition is not well-formed:
> 
>   const N<M>& create();
>   N<M> t(create());

I think this is covered by that wording, which was added very late to ensure
that only the immediate context is relevant:

"Only the validity of the immediate context of the variable initialization is
considered. [ Note: The evaluation of the initialization can result in side
effects such as the instantiation of class template specializations and
function template specializations, the generation of implicitly-defined
functions, and so on. Such side effects are not in the “immediate context” and
can result in the program being ill-formed. —end note ]

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #13 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 09:10:38 UTC ---
Ah yes, you're right.

So to make the testcase compile (which it should) I'd need to use sfinae to
disable the container members, which means writing constraints based on
CopyInsertable etc which depend on the allocator type, so allocator_traits
needs to also use sfinae to disable ill-formed 'construct' calls, which may not
be possible to detect, because errors may still not be in the immediate
context. Damn.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #14 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2012-10-24 09:23:22 UTC ---
(In reply to comment #13)
> So to make the testcase compile (which it should) I'd need to use sfinae to
> disable the container members, which means writing constraints based on
> CopyInsertable etc which depend on the allocator type, so allocator_traits
> needs to also use sfinae to disable ill-formed 'construct' calls, which may not
> be possible to detect, because errors may still not be in the immediate
> context. Damn.

I think it is implementable (I have done something similar to ensure that my
optional<T> realizes the same effect), but the costs are not small:

1) It will prevent the incomplete type support
2) The implementation changes are not small (my guess)

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #15 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 15:40:28 UTC ---
(In reply to comment #14)

> I think it is implementable (I have done something similar to ensure that my
> optional<T> realizes the same effect), but the costs are not small:
> 
> 1) It will prevent the incomplete type support

Ouch... but not required by the standard.

> 2) The implementation changes are not small (my guess)

Apart from the changes to allocator_traits and defining CopyInsertable etc.
(which I think isn't so hard, I can do that tonight) IIUC it would require
changing every container so that e.g.

    vector<T,A>::vector(const vector&)

becomes

    template<typename U>
      vector<T,A>::vector(const vector<U,A>&)

with constraints is_same<T,U> and CopyInsertable<A,T>, because we can't
constrain that constructor if it's not a template

Is that even allowed under the as-if rule?

Do we really want to go there?

Is there some other way to make examples like this compile?

  #include <vector>

  struct M
  {
    M() = default;
    M(M&&) = default;
    M& operator=(M&&) = default;
  };

  typedef std::vector<M> S;

  static_assert( !std::is_copy_constructible<S>::value,
                 "not CopyConstructible" );

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #16 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 17:36:52 UTC ---
I have a better idea: specialize is_copy_constructible for each std::container
to depend on whether its value type is CopyInsertable into it.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #17 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2012-10-24 17:48:07 UTC ---
(In reply to comment #15)
> > 1) It will prevent the incomplete type support
> 
> Ouch... but not required by the standard.

I agree, but I consider that as a high price.

> > 2) The implementation changes are not small (my guess)
> 
> Apart from the changes to allocator_traits and defining CopyInsertable etc.
> (which I think isn't so hard, I can do that tonight) IIUC it would require
> changing every container so that e.g.
> 
>     vector<T,A>::vector(const vector&)
> 
> becomes
> 
>     template<typename U>
>       vector<T,A>::vector(const vector<U,A>&)
> 
> with constraints is_same<T,U> and CopyInsertable<A,T>, because we can't
> constrain that constructor if it's not a template
> 
> Is that even allowed under the as-if rule?

I don't think that this works. I can elaborate what I had in my mind when
comparing with optional, but I think this is not needed to solve the issue and
would lengthen here the discussion. Looking at the concrete problem here I
think the only thing that is needed is a conditional noexcept for all
containers move constructors (which is presumably a border-case for the allowed
extensions but to me an acceptable one. I tend to suggest to open an LWG issue
for this). Now I'm reading below that the move constructor always seems to need
to allocate memory, is that really true? IMO that is the actual problem here.

> Is there some other way to make examples like this compile?
> 
>   #include <vector>
> 
>   struct M
>   {
>     M() = default;
>     M(M&&) = default;
>     M& operator=(M&&) = default;
>   };
> 
>   typedef std::vector<M> S;
> 
>   static_assert( !std::is_copy_constructible<S>::value,
>                  "not CopyConstructible" );

This is possible with my more-constraining model in mind that would really
enforce that vectors value type is a complete type during instantiation of
vector. I do not suggest to follow this path, but if you really want to do
that, here the sketch (I can show you the complete code of my optional
implementation that does so, if you want): The basic idea is to ensure that
vector becomes conditionally derived from a base-class with a deleted
copy-constructor. An empty tagging class suffices, e.g.

template<>
struct ctor_base<member_policy::move | member_policy::copy>
{
  constexpr ctor_base() noexcept = default;
  ctor_base(const ctor_base& rhs) noexcept = default;
  ctor_base(ctor_base&& rhs) noexcept = default;
  ctor_base& operator=(const ctor_base&) noexcept = default;
  ctor_base& operator=(ctor_base&&) noexcept = default;
};

template<>
struct ctor_base<member_policy::move>
{
  constexpr ctor_base() noexcept = default;
  ctor_base(const ctor_base&) = delete;
  ctor_base(ctor_base&& rhs) noexcept = default;
  ctor_base& operator=(const ctor_base&) noexcept = default;
  ctor_base& operator=(ctor_base&&) noexcept = default;
};

The condition for the base class selection is the outcome of
std::is_copy_constructible<value_type>. This design has the effect that you can
delete-away the effective copy-constructor of the container, if the value type
is not copyable.

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #18 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2012-10-24 18:12:25 UTC ---
(In reply to comment #17)
> Looking at the concrete problem here I think the only thing that is needed 
> is a conditional noexcept for all containers move constructors (which is
> presumably a border-case for the allowed extensions but to me an acceptable
> one. I tend to suggest to open an LWG issue for this)

For allocator-aware containers, it really should be unconditionally noexcept.
Could someone explain why allocations are needed within the move-constructor of
such containers?

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[paolo.carlini at oracle dot com]

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

--- Comment #19 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-24 18:51:24 UTC ---
Essentially memory allocation is needed because that is the "simplest" state
for such containers, what the default constructor also does in other terms.
Given that the moved from container must remain in a sane state and all the
normal operations must still be well defined, I don't think we can have
something "simpler", thus not allocating memory, than the default constructed
state. We can of course in principle redesign the affected containers to have a
different, simpler, default (it's already the case for containers like vector
or list, of course) but that isn't that easy to do, AFAICS. That said, I'm not
convinced this is the *real* issue, because: 1- I think a move-constructor not
noexcept is still conforming; 2- As we often say when teaching these things,
the std:: containers are just *examples* from the generic programming point of
view, and we should anyway be ready for user code with throwing move
constructors...

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #20 from Jonathan Wakely <redi at gcc dot gnu.org> 2012-10-24 20:03:52 UTC ---
It's conforming for unordered_map to have a possibly-throwing move ctor, but it
must be possible to insert it into a vector, because vector::emplace_back
doesn't require copyconstructible, and doesn't require nothrow
moveconstructible

The bug is that we "detect" the type is copyconstructible when it isn't really.

So I think specializing std::is_copy_constructible to depend on the allocator
and value type makes sense. I've implemented it and it works.

That said, I think a non-allocating default ctor and move ctor would also be
better. Allocation should happen on insertion, which could test whether there
are already buckets or not

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[paolo.carlini at oracle dot com]

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

--- Comment #21 from Paolo Carlini <paolo.carlini at oracle dot com> 2012-10-24 20:28:59 UTC ---
Totally agreed it would be better (in Italian in such cases we say something
like "you are trying to open a door which is already open" ;) We'll come to
that. Thanks Jon for all the time you are spending on this!

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.7.3

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |rejects-valid
           Priority|P3                          |P2
      Known to work|                            |4.6.3

[Bug libstdc++/55043] [4.7/4.8 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #22 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 09:20:43 UTC ---
Author: redi
Date: Wed Jan 16 09:20:34 2013
New Revision: 195231

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195231
Log:
    PR libstdc++/55043
    * include/std/unordered_map: Include alloc_traits.h
    * include/std/unordered_set: Likewise.
    * include/bits/alloc_traits.h: Define __is_copy_insertable.
    * include/bits/unordered_map.h: Use it.
    * include/bits/unordered_set.h: Likewise.
    * include/debug/unordered_map.h: Likewise.
    * include/debug/unordered_set.h: Likewise.
    * include/profile/unordered_map.h: Likewise.
    * include/profile/unordered_set.h: Likewise.
    * include/bits/hashtable.h: Fix comment typos.
    * testsuite/23_containers/unordered_map/55043.cc: New.
    * testsuite/23_containers/unordered_multimap/55043.cc: New.
    * testsuite/23_containers/unordered_multiset/55043.cc: New.
    * testsuite/23_containers/unordered_set/55043.cc: New.

Added:
    trunk/libstdc++-v3/testsuite/23_containers/unordered_map/55043.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_multimap/55043.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_multiset/55043.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_set/55043.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/alloc_traits.h
    trunk/libstdc++-v3/include/bits/hashtable.h
    trunk/libstdc++-v3/include/bits/unordered_map.h
    trunk/libstdc++-v3/include/bits/unordered_set.h
    trunk/libstdc++-v3/include/debug/unordered_map
    trunk/libstdc++-v3/include/debug/unordered_set
    trunk/libstdc++-v3/include/profile/unordered_map
    trunk/libstdc++-v3/include/profile/unordered_set
    trunk/libstdc++-v3/include/std/unordered_map
    trunk/libstdc++-v3/include/std/unordered_set

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[4.7/4.8 Regression] issue  |[4.7 Regression] issue with
                   |with nesting unordered_map  |nesting unordered_map
                   |containing unique_ptr into  |containing unique_ptr into
                   |vector                      |vector

--- Comment #23 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 09:26:39 UTC ---
Fixed on trunk so far.

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[glisse at gcc dot gnu.org]

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

Marc Glisse <glisse at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |glisse at gcc dot gnu.org

--- Comment #24 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-16 09:39:00 UTC ---
That really feels like a hack. Anyone using boost::is_copy_constructible or
whatever personal trick to detect copyable types will still be impacted. Did
your idea in comment #15 not work?

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[daniel.kruegler at googlemail dot com]

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

--- Comment #25 from Daniel Krügler <daniel.kruegler at googlemail dot com> 2013-01-16 09:43:07 UTC ---
(In reply to comment #24)
> That really feels like a hack.
It is also broken, I think. The P/R has the effect that is_copy_constructible
is now out-of-sync with is_constructible, so the difference is easily
observable.

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #26 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 10:25:57 UTC ---
(In reply to comment #24)
> That really feels like a hack.

It is a hack, to work around a throwing move ctor that I don't have time to
fix.

> Anyone using boost::is_copy_constructible or
> whatever personal trick to detect copyable types will still be impacted.

Impacted in what way? They'll get the same result as they did previously.
This changes std::is_copy_constructible to be more accurate and makes
__move_if_noexcept work for the unordered containers. How does that affect
boost::is_copy_constructible?
It gives the wrong result for unordered_containers of non-copyable types, but
already did, and it's not my responsibility to fix everyone else's traits  :-)

> Did
> your idea in comment #15 not work?

I don't think that would be conforming and would be a huge amount of work to
replace every constructor in vector and forward_list (and every other container
as they are updated to be allocator-aware) with a template constructor.  I'm
not going to work on that solution, and I won't approve patches to do that
without a lot of persuasion.

I still do want to use SFINAE to remove allocator_traits<A>::construct(args)
from participating in overload resolution when A().construct(args) is not valid
and is_constructible<A::value_type, args> is false, which I hope is conforming,
and would allow a better solution similar to comment 17, by deleting the
unordered_xxx copy ctor.

I'll revisit the patch tonight.

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #27 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 10:52:57 UTC ---
Actually, now that the unordered containers do not inherit from Hashtable it
should be much easier to implement something like comment 17. When Daniel first
suggested it I thought it would be tricky given the current design of the
containers, but François changed them three days later.

I'll revert today's patch and fix it properly by making __is_copy_insertable
more accurate and removing the unordered container constructors when
appropriate.

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #28 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 23:56:07 UTC ---
Author: redi
Date: Wed Jan 16 23:56:00 2013
New Revision: 195253

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=195253
Log:
    PR libstdc++/55043 (again)
    * include/bits/alloc_traits.h (allocator_traits::construct): Disable
    unless construction would be well-formed.
    (__allow_copy_cons, __check_copy_constructible): Define.
    * include/bits/unordered_map.h (__check_copy_constructible): Use as
    base class so copy constructor will be deleted if appropriate.
    (is_copy_constructible): Remove specialization.
    * include/bits/unordered_set.h: Likewise.
    * include/debug/unordered_map.h: Undo previous commit. Default copy
    and move constructors.
    * include/debug/unordered_set.h: Likewise.
    * include/profile/unordered_map.h: Undo previous commit.
    * include/profile/unordered_set.h: Likewise.
    * testsuite/23_containers/unordered_map/55043.cc: Fix test.
    * testsuite/23_containers/unordered_multimap/55043.cc: Likewise.
    * testsuite/23_containers/unordered_multiset/55043.cc: Likewise.
    * testsuite/23_containers/unordered_set/55043.cc: Likewise.
    * testsuite/23_containers/unordered_map/requirements/53339.cc: XFAIL,
    cannot support incomplete types.
    * testsuite/23_containers/unordered_multimap/requirements/53339.cc:
    Likewise.

Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/bits/alloc_traits.h
    trunk/libstdc++-v3/include/bits/unordered_map.h
    trunk/libstdc++-v3/include/bits/unordered_set.h
    trunk/libstdc++-v3/include/debug/unordered_map
    trunk/libstdc++-v3/include/debug/unordered_set
    trunk/libstdc++-v3/include/profile/unordered_map
    trunk/libstdc++-v3/include/profile/unordered_set
    trunk/libstdc++-v3/testsuite/23_containers/unordered_map/55043.cc
   
trunk/libstdc++-v3/testsuite/23_containers/unordered_map/requirements/53339.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_multimap/55043.cc
   
trunk/libstdc++-v3/testsuite/23_containers/unordered_multimap/requirements/53339.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_multiset/55043.cc
    trunk/libstdc++-v3/testsuite/23_containers/unordered_set/55043.cc

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #29 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-01-16 23:58:41 UTC ---
I hope this fix is more acceptable :)
Thanks, Daniel and Marc, for the comments and ideas, and pushing me to do a
better job of it!

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

--- Comment #30 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-02-20 23:55:54 UTC ---
Author: redi
Date: Wed Feb 20 23:55:48 2013
New Revision: 196187

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=196187
Log:
    PR libstdc++/55043
    * include/bits/alloc_traits.h (allocator_traits::construct): Disable
    unless construction would be well-formed.
    (__allow_copy_cons, __check_copy_constructible): Define.
    * include/bits/unordered_map.h (__check_copy_constructible): Use as
    base class so copy constructor will be deleted if appropriate.
    * include/bits/unordered_set.h: Likewise.
    * include/std/unordered_set: Include alloc_traits.h.
    * include/std/unordered_set: Likewise.
    * include/debug/unordered_map.h: Default copy and move constructors.
    * include/debug/unordered_set.h: Likewise.
    * testsuite/23_containers/unordered_map/55043.cc: Fix test.
    * testsuite/23_containers/unordered_multimap/55043.cc: Likewise.
    * testsuite/23_containers/unordered_multiset/55043.cc: Likewise.
    * testsuite/23_containers/unordered_set/55043.cc: Likewise.
    * testsuite/23_containers/unordered_map/requirements/53339.cc: XFAIL,
    cannot support incomplete types.
    * testsuite/23_containers/unordered_multimap/requirements/53339.cc:
    Likewise.
    * testsuite/23_containers/unordered_set/instantiation_neg.cc: Adjust
    dg-error line number.

Added:
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_map/55043.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_map/requirements/53339.cc
      - copied, changed from r196178,
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_multimap/55043.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_multimap/requirements/53339.cc
      - copied, changed from r196178,
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_multiset/55043.cc
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_set/55043.cc
Modified:
    branches/gcc-4_7-branch/libstdc++-v3/ChangeLog
    branches/gcc-4_7-branch/libstdc++-v3/include/bits/alloc_traits.h
    branches/gcc-4_7-branch/libstdc++-v3/include/bits/unordered_map.h
    branches/gcc-4_7-branch/libstdc++-v3/include/bits/unordered_set.h
    branches/gcc-4_7-branch/libstdc++-v3/include/debug/unordered_map
    branches/gcc-4_7-branch/libstdc++-v3/include/debug/unordered_set
    branches/gcc-4_7-branch/libstdc++-v3/include/std/unordered_map
    branches/gcc-4_7-branch/libstdc++-v3/include/std/unordered_set
   
branches/gcc-4_7-branch/libstdc++-v3/testsuite/23_containers/unordered_set/instantiation_neg.cc

[Bug libstdc++/55043] [4.7 Regression] issue with nesting unordered_map containing unique_ptr into vector

[redi at gcc dot gnu.org]

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

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

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

--- Comment #31 from Jonathan Wakely <redi at gcc dot gnu.org> 2013-02-20 23:57:35 UTC ---
fixed for 4.7.3