5

Simplified code:

#include <queue>
#include <memory>
#include <vector>

class Foo {
public:
    Foo() {};
    virtual ~Foo() {}
};

int main()
{
    std::queue<std::unique_ptr<Foo>> queue;
    auto element = std::make_unique<Foo>();
    queue.push(std::move(element));
    std::vector<std::queue<std::unique_ptr<Foo>>> vector;
    // Error 1
    vector.push_back(queue); 
    // Error 2
    vector.push_back(std::move(queue));
    // Error 3
    vector.push_back({});
    return 0;
}

Error:

'std::unique_ptr>::unique_ptr(const std::unique_ptr<_Ty,std::default_delete<_Ty>> &)': attempting to reference a deleted function

Obviously copying c~tor of unique_ptr is removed but I'm not trying to copy it. Am I?

M.M
  • 138,810
  • 21
  • 208
  • 365
  • Yes, `push_back` makes a copy. To create an object in place, use `emplace_back`. – Ben Voigt Jun 05 '18 at 01:44
  • 1
    It works if `std::queue` is replaced by `std::vector`; doesn't work with the default queue underlier of `std::deque`. But it does work fine to `move` a `deque` that isn't in a vector. Weird – M.M Jun 05 '18 at 02:43
  • @M.M Yes, you can move a `deque` that isn't in a `vector`. The operation will not be `noexcept` however. `vector` just doesn't like potentially throwing moves (see aschepler's answer for details). – Arne Vogel Jun 05 '18 at 09:05

1 Answers1

3

This is a bit tricky. All std::vector<T> functions that can increase the size of the vector have to do it in an exception-safe way if either of these two things are true:

  • T has a move constructor that guarantees it will never throw any exceptions; or,

  • T has a copy constructor.

So in most implementations, if T has a move constructor declared nothrow or equivalent, vector will use the move constructor of T for those operations. If not, and T has a copy constructor, vector will use the copy constructor, even if T has a move constructor.

And the problem here is that std::queue always declares it has a copy constructor, even if that copy constructor can't actually be instantiated, and always declares it has a move constructor that might throw, even if the container member's move constructor guarantees it won't throw.

The Standard specifies these in [queue.defn] as:

namespace std {
  template<class T, class Container = deque<T>>
  class queue {
    // ...
  public:
    explicit queue(const Container&);
    explicit queue(Container&& = Container());
    // ...
  };
}

This class template definition could be improved in a couple of ways to be more "SFINAE-friendly" and avoid issues like the one you ran into. (Maybe somebody could check for other classes with similar issues and submit a proposal to the Library Working Group.)

  1. Change the move constructor to promise not to throw if the Container type makes the same promise, typically done with language like:

    explicit queue(Container&& rhs = Container()) nothrow(see below);
    

    Remarks: The expression inside noexcept is equivalent to is_­nothrow_­move_­constructible_­v<Container>.

  2. Change the copy constructor to be deleted if the Container type is not copyable, typically done with language like:

    explicit queue(const Container&);
    

    Remarks: This constructor shall be defined as deleted unless is_­copy_­constructible_­v<Container> is true.

aschepler
  • 70,891
  • 9
  • 107
  • 161
  • I found the same issue with using `deque` directly – M.M Jun 05 '18 at 02:54
  • @M.M Not sure what you mean. Can you link to an example? – aschepler Jun 05 '18 at 02:58
  • @M.M Okay, yeah. The move constructor of `queue` is also not declared `noexcept`. Which is more of a surprise, since it ought to be possible to move a `deque` no matter what the element type is, and this is different from `vector` for no obvious reason. – aschepler Jun 05 '18 at 03:20
  • I found [this post](https://stackoverflow.com/a/12334918/1505939) talking about it... allocator traits are involved, which explains why I've been unable to reproduce the problem with a test non-nothrow_move_constructible class (instead of `deque`) – M.M Jun 05 '18 at 03:40
  • [this thread](https://gcc.gnu.org/ml/gcc-help/2017-08/msg00079.html) suggests some rationale for `deque` not being nothrow-movable – M.M Jun 05 '18 at 03:42
  • 1
    The code builds OK with container `queue< unique_ptr, vector> >`, so perhaps `deque` is the underlying issue and `queue` itself is OK – M.M Jun 05 '18 at 03:46
  • Thanks guys. I'm ok with changing queue container type from deque to vector. Saying that I'm changing my outer container to be std::vector, std::vector>>> vector; – Mikhail Shatalin Jun 05 '18 at 16:42