18

I'm busy testing an implementation of various generic algorithms and I'm using types with minimal support of provided functions. I came across this weird setup when using a std::pair<T, movable> with some type T (e.g., int) and a movable type defined like this:

struct movable
{
    movable() {}
    movable(movable&&) = default;
    // movable(movable const&) = delete;
    movable(movable&) = delete;
};

The idea is have a type which is movable but not copyable. That works great, e.g., with expressions like this:

movable m1 = movable();
movable m2 = std::move(m1);

However, when trying to use this type as a member of std::pair<...> it fails! To make get the code to compile it is necessary to add the deleted(!) copy constructor taking a movable const& (or have only that version). The copy constructor taking a non-const reference is insufficient:

#include <utility>
auto f() -> std::pair<int, movable> {
    return std::pair<int, movable>(int(), movable());
}

What is going on here? Is std::pair<...> overspecified by mandating that std::pair(std::pair const&) is = defaulted?

The problem seems to be down to the specification of std::pair's copy constructor (in 20.3.2 [pairs.pair] synopsis):

 namespace std {
     template <class T1, class T2>
     struct pair {
         ...
         pair(const pair&) = default;
         ...
     };
 }

A quick check with my implementation implies that the obvious implementation copying the two members does not require the const& version of the movable copy constructor. That is, the offensive part is the = default on pair's copy constructor!

Dietmar Kühl
  • 150,225
  • 13
  • 225
  • 380
  • 1
    possible duplicate of [std::pair: too restrictive constructor?](http://stackoverflow.com/questions/23723744/stdpair-too-restrictive-constructor) – Borgleader Dec 23 '14 at 19:49
  • 1
    @Borgleader: I'd agree that it is related but this question doesn't address the problem of the `= default` being mandated being a problem! ... or, for that matter, why using `= default` actually causes this problem. The proposed resolution to the issue also leaves the offensive `pair(pair const&) = default` in place. – Dietmar Kühl Dec 23 '14 at 19:56
  • @Dietmar: My reading of that question/answer suggested that `=default` was not a problem at all. It should not be instantiated unless used. – Puppy Dec 23 '14 at 19:59
  • Oh.. is it that time of the year again, when you start posting interesting questions? :D – dyp Dec 23 '14 at 20:56
  • 2
    It seems not to have been cited yet: [dcl.fct.def.default]/1.2 says why the defaulted copy-ctor of `pair` is illegal - and I interpret it as saying that *its declaration is ill-formed*, so no instantiation is required to make the program ill-formed. – dyp Dec 23 '14 at 21:03
  • @dyp: yes, carefully rereading this clauses implies, indeed, that it causes the defaulted copy constructor of `std::pair` to become ill-formed if there is a `= delete`d copy constructor taking a non-`const` reference for one of the members as it _adds_ rather than _removes_ a cv-qualifier. It seems `std::pair`'s copy constructor is over-specified in this case! Seems I need to raise an LWG issue. – Dietmar Kühl Dec 23 '14 at 21:31
  • Do you happen to know *why* the assignment-operators are declared explicitly? If that was not necessary, it seems as if the copy ctor could just be left implicitly declared (and the issue should disappear). – dyp Dec 23 '14 at 21:43
  • @dyp: The assignment operators are explicitly defined to deal with the `noexcept(...)` qualification. I doubt that these will be dropped. – Dietmar Kühl Dec 23 '14 at 21:49
  • As far as I understand the specification in [except.spec]/16, an implicitly-declared move assignment-operator should have the same exception-specification as the one required for `std::pair` ([live example](http://coliru.stacked-crooked.com/a/e0356f76a9f1819f)). The effects of this implicitly defined operator also match (as far as I can see) the required effects of `std::pair`'s move op. Both libc++ and libstdc++ implement the move assignment-op manually, though (why?). – dyp Dec 24 '14 at 00:01

1 Answers1

8

std::pair copy constructor is declared as follows:

pair(const pair&) = default;

By declaring this copy constructor for movable:

movable(movable&) = delete;

you inhibit implicit creation of movable(const movable&) (so it's not even deleted, there's just no such constructor), thus this is the only copy constructor you have. But std::pair copy constructor requires a copy constructor of its members to take const reference, so you get compile error.

If you add this:

movable(movable const&) = delete;

or (better) just remove movable(movable&) = delete; declaration, you now have the movable(movable const&) constructor, and because it's deleted, the std::pair copy constructor also becomes deleted.

Update: Let's consider a simpler example demonstrating the same issue. This doesn't compile:

template <typename T>
struct holder {
    T t;
    // will compile if you comment the next line
    holder(holder const&) = default;
    // adding or removing move constructor changes nothing WRT compile errors
    // holder(holder&&) = default;
};

struct movable {
    movable() {}
    movable(movable&&) = default;
    // will also compile if you uncomment the next line
    //movable(movable const&) = delete;
    movable(movable&) = delete;
};

holder<movable> h{movable()};

It will compile if you comment the copy constructor of holder, because this is how implicit copy constructor generation works ([class.copy]/8:

The implicitly-declared copy constructor for a class X will have the form

X::X(const X&)

if each potentially constructed subobject of a class type M (or array thereof) has a copy constructor whose first parameter is of type const M& or const volatile M&. Otherwise, the implicitly-declared copy constructor will have the form

X::X(X&)

That is, when you comment out the declaration holder(holder const&) = default; the implicitly declared copy constructor of holder will have the form holder(holder&). But if you don't, T's copy constructor has take const T& (or const volatile T&) because this is what will be called in memberwise copy procedure described in [class.copy]/15.

And if holder has a move constructor, it's even easier - if you comment out holder(holder const&) = default;, the implicitly declared copy constructor of holder will be just deleted.

Community
  • 1
  • 1
Anton Savin
  • 40,838
  • 8
  • 54
  • 90
  • 1
    Well, I understand things this far - except that my code obviously never actually copies the pair! It only moves the pair. ... as is shown by the code compiling when adding a `= delete`d copy constructor causing it to compile. Also, defining `std::pair`'s copy constructor with the obvious implementation instead of using `=default` make the code compile! – Dietmar Kühl Dec 23 '14 at 20:01
  • @DietmarKühl yes, but the compiler has to instantiate `std::pair` copy constructor anyway (and if it becomes deleted it's ok, but not when it can't compile) – Anton Savin Dec 23 '14 at 20:06
  • 1
    Why does it need to do so? Implementing `pair`'s copy constructor using the obvious approach removes this need. Shouldn't the `= default` version behave the same? – Dietmar Kühl Dec 23 '14 at 20:08
  • 2
    Well, your `holder` is clearly not move-constructible as it declares a copy constructor thereby inhibiting an automatically created move constructor but also doesn't declare a move construct. `std::pair` defines a move constructor (it is also `= default`ed). I still don't want to copy my object but only move it. – Dietmar Kühl Dec 23 '14 at 21:21
  • 1
    The issue seems to be what @dyp pointed out: the declaration of defaulted constructor is ill-formed if it differs from the one which would be implicitly declared (except for adding `const`), independent on whether it is used or not. – Dietmar Kühl Dec 23 '14 at 21:51
  • @DietmarKühl I think the real issue is `movable` having the constructor `movable(movable&)` (even deleted). It doesn't make any sense to add it. – Anton Savin Dec 23 '14 at 21:53
  • 1
    Well, for move-only types it makes sense to explicitly disable copy construction. Sure, it _could_ be done using `movable(movable const&)` but since this constructor isn't to be used either variation _should_ work. I can clearly fix the issue for _my_ code but creating libraries I'd like to be friendly to users who use the library. If `std::pair` wouldn't be defined to have an `= default`ed copy constructor, both versions would work (and a standard library defaulting the copy constructor would be wrong). – Dietmar Kühl Dec 23 '14 at 21:59