4

I am trying to store a noncopyable (but movable) object inside an std::pair, as follows:

#include <utility>

struct S
{
    S();
private:
    S(const S&);
    S& operator=(const S&);
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}

But I'm getting the following compiler error with gcc 4.6:

In file included from include/c++/4.6.0/bits/move.h:53:0,
                 from include/c++/4.6.0/bits/stl_pair.h:60,
                 include/c++/4.6.0/utility:71,
                 from src/test.cpp:1:
include/c++/4.6.0/type_traits: In instantiation of 'const bool std::__is_convertible_helper<S, S, false>::__value':
include/c++/4.6.0/type_traits:789:12:   instantiated from 'std::is_convertible<S, S>'
src/test.cpp:13:31:   instantiated from here
src/test.cpp:7:5: error: 'S::S(const S&)' is private
include/c++/4.6.0/type_traits:782:68: error: within this context
In file included from include/c++/4.6.0/utility:71:0,
                 from src/test.cpp:1:
src/test.cpp: In constructor 'std::pair<_T1, _T2>::pair(_U1&&, const _T2&) [with _U1 = int, <template-parameter-2-2> = void, _T1 = int, _T2 = S]':
src/test.cpp:13:31:   instantiated from here
src/test.cpp:7:5: error: 'S::S(const S&)' is private
include/c++/4.6.0/bits/stl_pair.h:121:45: error: within this context

It seems the compiler is trying to call the std::pair<_T1, _T2>::pair(_U1&&, const _T2&) constructor, which of course is problematic. Shouldn't the compiler be calling the std::pair<_T1, _T2>::pair(_U1&&, _U2&&) constructor instead? What is going on here?

EDIT: Ok, I understand that providing an explicit move constructor fixes the problem, but I'm still a little confused.

Suppose that I make the class noncopyable by inheriting from boost::noncopyable rather than declaring my own private copy constructor.

The following works fine, suggesting that a move constructor is implicitly generated:

#include <boost/noncopyable.hpp>

struct S : boost::noncopyable
{
};

void f(S&&)
{

}

int main()
{
    f(S());
    return 0;
}

However, with std::pair it still doesn't work:

#include <utility>
#include <boost/noncopyable.hpp>

struct S : boost::noncopyable
{
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}

Errors:

In file included from include/c++/4.6.0/utility:71:0,
                 from src/test.cpp:1:
/include/c++/4.6.0/bits/stl_pair.h: In constructor 'std::pair<_T1, _T2>::pair(_U1&&, const _T2&) [with _U1 = int, <template-parameter-2-2> = void, _T1 = int, _T2 = S]':
src/test.cpp:16:31:   instantiated from here
/include/c++/4.6.0/bits/stl_pair.h:121:45: error: use of deleted function 'S::S(const S&)'
src/test.cpp:4:8: error: 'S::S(const S&)' is implicitly deleted because the default definition would be ill-formed:
boost/boost/noncopyable.hpp:27:7: error: 'boost::noncopyable_::noncopyable::noncopyable(const boost::noncopyable_::noncopyable&)' is private
src/test.cpp:4:8: error: within this context

Moreover, adding = default-ed default constructor and move constructor does not help!

#include <utility>
#include <boost/noncopyable.hpp>

struct S : boost::noncopyable
{
    S() = default;
    S(S&&) = default;
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}

I get the same errors! I have to explicitly give the definition of the move constructor myself, which is annoying if the class has a lot of members:

#include <utility>
#include <boost/noncopyable.hpp>

struct S : boost::noncopyable
{
    S() = default;
    S(S&&) {}
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}
Soo Wei Tan
  • 3,262
  • 2
  • 34
  • 36
HighCommander4
  • 50,428
  • 24
  • 122
  • 194
  • You didn't define a move constructor! Also, in C++0x you can say `=delete`, so you don't have to use the `private:` constructor hack to disable copying. – Kerrek SB Jul 25 '11 at 00:14
  • @Kerrek SB: Shouldn't one be generated automatically? – HighCommander4 Jul 25 '11 at 00:15
  • 1
    No, move constructors don't get implicitly defined for you. You can request the default one with `=default`, though, if you don't want to type it out. – Kerrek SB Jul 25 '11 at 00:18
  • @Kerrek SB: They do get implicitly defined in other cases. (I've tested it by putting a cout in the move constructor of a member object). – HighCommander4 Jul 25 '11 at 00:20
  • 1
    Only if you don't define any copy constructors or assignment operators, I believe. There's an elaborate set of rules when you get implicitly defined constructors. Of course PODs have them, for example. – Kerrek SB Jul 25 '11 at 00:22
  • [See this question.](http://stackoverflow.com/questions/4819936/why-no-default-move-assignment-move-constructor) – Kerrek SB Jul 25 '11 at 00:22

1 Answers1

4

You need to provide a move constructor. The following compiles without errors.

#include <utility>

struct S
{
    S() {}
    S(S&&) {}
    S& operator=(S&&) {}

    S(const S&) =delete;
    S& operator=(const S&) =delete;
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}


EDIT:

It seems that if you inherit from another class (or struct) then the base needs to declare a move constructor. I think this is because if you default the derived class' move constructor it tries to move the base object and fails to do so.

Here's an edited boost::noncopyable that defines a move constructor.

#include <utility>

namespace boost {

namespace noncopyable_  // protection from unintended ADL
{
  class noncopyable
  {
   protected:
      noncopyable() {}
      noncopyable(noncopyable&&) {};
      ~noncopyable() {}
   private:  // emphasize the following members are private
      noncopyable( const noncopyable& );
      const noncopyable& operator=( const noncopyable& );
  };
}

typedef noncopyable_::noncopyable noncopyable;

} // namespace boost

struct S : boost::noncopyable
{
    S() = default;
    S(S&&) = default;

    S& operator=(S&&) {}
};

int main()
{
    std::pair<int, S> p{0, S()};
    return 0;
}
Praetorian
  • 106,671
  • 19
  • 240
  • 328
  • OK... but I'm still a little confused about what is going on behind the scenes (please see my edits) – HighCommander4 Jul 25 '11 at 00:56
  • @HighCommander4 I've edited my answer. Looks like the only solution for now is to define a move constructor, until boost starts defining move constructors for their classes. – Praetorian Jul 25 '11 at 01:09
  • I see, that makes sense. But it still doesn't explain why the first example in my edit (with the `void f(S&&)`) works... – HighCommander4 Jul 25 '11 at 01:11
  • @HighCommander4 Hmmm, maybe the compiler got rid of the call to `f()` since it doesn't do anything. Can you try it out with optimizations turned off. Maybe declare a volatile variable within the function and do something with it? – Praetorian Jul 25 '11 at 01:14
  • I already had optimizations turned off. Adding a volatile variable doesn't change anything either. I can even declare the move constructor as deleted and it works! – HighCommander4 Jul 25 '11 at 01:21
  • I see that! I give up, no clue why it is working. I thought maybe you were running into some sort of most vexing parse issue while calling `f()` so I created a named `S` object and `std::move`d it in the function call but it still succeeds. That should make for a good question :-) – Praetorian Jul 25 '11 at 01:33