20

In Herb Sutter's talk at CppCon16 he suggested writing pimpl idiom with const std::unique_ptr (roughly 10 minutes in).

How is this supposed to work with move constructors/assignments? Is there something in c++17? I couldn't find anything.

Improve this question
6
  • 7
    Why won't you want majority of your pimpl classes to be movable? It seems like a perfectly reasonable thing to do. Commented Sep 26, 2016 at 13:18
  • 1
    @DenisYaroshevskiy can only assume he has a specific use case in mind. In general, I would agree with using unique_ptras a container for pimpl. If you wanted it to be copyable, you'd have to implement that in terms of a clone operation of course. Commented Sep 26, 2016 at 13:29
  • 1
    Just watched the relevant part of the video and I disagree with him. I think for most pimpl handles, a const unique_ptr would become a limitation very quickly. Commented Sep 26, 2016 at 13:40
  • 1
    @RichardHodges - Sutter is right. If you copy a class with a pimpl, you either have two objects with the same pimpl, if the pimpl is a shared_ptr, or one object with no pimpl, if the pimpl is a unique_ptr. This breaks the contract of one object, one pimpl. On the other hand, an object with a const pimpl can still be moved. Commented Sep 26, 2016 at 16:13
  • 4
    @SamVarshavchik I cannot see how a const unique_ptr is moveable. There is no const unique_ptr&& constructor. If you can provide example code I'd be intrigued. I think you'd have to write a move-constructor that moves the implementation. That still leaves one object in an undefined state. It was noteworthy that he offered no use cases of the const unique_ptr-as-pimpl (non)-idiom. Herb's a bright guy but he's not necessarily someone I'd follow blindly. Commented Sep 26, 2016 at 16:24

2 Answers 2

Reset to default
8

If your class is supposed to be never-empty, a non-const unique ptr (with default move/assigns) is not appropriate. The move ctor and move assign will both empty the rhs.

A const unique ptr will disable these automatic methods, and if you want move you will have to write it within the impl (and a bit of glue outside).

I would personally write a value ptr with the semantics I want (then let compiler write the glue), but starting with a const unique_ptr sounds reasonable as a first pass.

If you relax the never-empty, and make it almost never-empty, you now have to reason about preconditions of a lot of methods, and possible knock-on bugs.

The biggest cost of this technique, difficulty in returning values, goes away with C++17.

Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

If you move out from an object it's suppose to be in partially formed state, so it can only be assigned and destructed so move constructor/assigment doesn't break your not_null guarantee. You can come up with smth like not_null from gsl to wrap it if you want to.
@DenisYaroshevskiy No, it is in whatever state you are willing to guarantee? It isn't "supposed to be in a partially formed state" unless that is what you choose to guarantee. One option is to be almost-never-empty and only be empty once moved-from. This is a choice, it has costs and benefits. The costs of going from never-empty to almost-never-empty should not be dismissed. Ditto the costs of going from almost-never-empty to never-empty. Never-empty is easier to guarantee correctness, and biasing towards more-often-correct code over faster code is often a good idea.
@Yakk even when a moved-from object is "never empty", using it after the move is a strong code smell. I would argue that a pimpl pointer check followed by assert/exception is preferable to allowing people to depend upon a moved-from object being in a 'default' state. This is inviting disaster when the guarantee can no longer be met due to inevitable mission creep. I would go far as to say that it is an anti-pattern that is at odds with the spirit of std::move.
@RichardHodges Sometimes-empty types are guaranteed disaster, as you will accidentally use them in a context where some possibility of them being empty. Preventing that is hard. You can fix it, but only with extensive and comprehensive QA (and if you assume perfect QA, no code is ever "dangerous"). By going with const unique ptr, your objects cannot be moved, and this problem cannot occur. By using a specific never_empty_ptr, your code cannot be empty, and this problem cannot occur. Hand-rolling a never-empty state into a class is, on the other hand, probably a bad idea.
@Yakk If preventing the use of moved-from objects is hard (for a given team?), then this argues for mandating a "no using std::move" constraint on one's (presumably substandard) developers. All the good ones will leave as soon as they can find a better job, of course, but you'll have your 'never empty' guarantee while still allowing other teams to write efficient code. I'm struggling to imagine a reasonable use case for this idiom. It undoes in one fell swoop 90% of the massive increment in utility and performance offered by trading up from c++03 to c++11.
|
3

How is this suppose to work with move constructors/assignments?

Move constructors:

The implicitly-declared or defaulted move constructor for class T is defined as deleted if any of the following conditions are true:

  • T has non-static data members that cannot be moved (have deleted, inaccessible, or ambiguous move constructors)

const std::unique_ptr is such a data member because of const.

If const is dropped the compiler generates the move constructor and assignment, but not the copying ones.

Herb explains why he uses const unique_ptr:

non-const can work too, but it is more brittle because default move semantics are probably incorrect.

With const member it is more robust because const members must be initialized in the constructor. And const documents that the implementation of the object does not change, it is not State or Strategy design pattern.

Improve this answer

9 Comments

Then in such case one would have to implement a copy constructor?
@vordhosbn If that is necessary.
That's true but my question was about move construction/assignment. Using plain unique_ptr instead of const will keep move construction/assignment alive and well while declaring it const forbids it. I used to think that const data members is a bad practise (because of this issue) but apparently Herb Sutter doesn't think this. I wondered why.
I would argue the opposite to Mr Sutter. The default behaviour of std::move on std::unique_ptr is absolutely correct and very much what we want - an efficient transfer of state, leaving the moved-from handle in a very well defined state - "invalid, do not touch". Disabling moves explicitly is one thing (one might wonder at the motives). Disabling them through cryptic use of const just looks to me like playfulness.
To me, the most important thing is to express lifetime by construction, and to get the defaults right (in this case, no brittle move by default). You can write the copy and move operations yourself, e.g., MyClass& operator=(const MyClass& that) { *pimpl = *that.pimpl; return *this; }. The Impl class can still be copyable and even movable, and you can delegate to it. Of course that only gets you 95% of the way, and there's no objection to also writing your own value_ptr type that automates the deep copy/move too.
|

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.