3

When I pass a shared_ptr to a constructor that copies that argument into a member shared_ptr, should this parameter be passed by value?

Example:

struct MyClass {
    MyClass(std::shared_ptr<MyDependency> dep) 
        : dep(dep)
    {}

    std::shared_ptr<MyDependency> dep;
};

If constructed with a temporary (MyClass(std::make_shared<...>())) the compiler should move the argument (once or twice?).

Is the compiler able to "auto" move dep to dep, or should I use : dep(std::move(dep))?

If constructed with a lvalue the value will be copied (min. one times).

On the other hand, passing the shared_ptr by const-ref will always copy the pointer.

So should constructor arguments be passed by value if they will be directly copied into a member?

Edit: The parameter/member must be a shared_ptr.

  • 1
    Doesn't clarify about constructor, but might still be interesting for reading: Herb Sutter's [GotW#91](https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/)... – Aconcagua Jul 27 '18 at 09:24
  • Are you sure `shared_ptr` is needed here? There is no shared ownership in this example. – Maxim Egorushkin Jul 27 '18 at 09:53
  • @MaximEgorushkin Yes, it is needed. The example is not the best one. –  Jul 27 '18 at 09:53
  • 1
    Possible duplicate of [The C++11 way of initializing data members from arguments](https://stackoverflow.com/questions/7138376/the-c11-way-of-initializing-data-members-from-arguments) – xskxzr Jul 27 '18 at 10:15

2 Answers2

3

Should shared_ptr constructor arguments be passed by value

If you intend to share ownershi i.e. you want to keep a copy: Yes, passing by value is the preferred way.

If constructed with a temporary ... the compiler should move the argument (once or twice?).

First the argument is move constructed, then the member is initialized (see below). The construction of the argument may be elided in some cases.

should I use : dep(std::move(dep))

Yes, you should. The argument has a name, and so it is an lvalue. To move construct the member, you need to have an rvalue.

eerorika
  • 232,697
  • 12
  • 197
  • 326
  • 1
    OK, so we'd end up in possibly moving twice? – Aconcagua Jul 27 '18 at 09:30
  • @Aconcagua yes. Maybe even likely twice. I'm starting to doubt about the possibility of elision. – eerorika Jul 27 '18 at 09:35
  • Hm, the more I think about, copy elision *should* apply in case of `MyClass(std::make_shared())`, another matter, though, would be `MyClass(std::move(somePtr))` – Aconcagua Jul 27 '18 at 09:46
  • @Aconcagua yes indeed. GCC seems to elide in the first case, but not the other. – eerorika Jul 27 '18 at 09:49
  • In the case `MyClass(std::make_shared())` the compiler will ellide the copy. It should be construction+move. But the case of passing an lvalue, the compiler will do a copy+move, which is worse than what we get if we pass by const-ref, that is one copy without a move. –  Jul 27 '18 at 09:50
  • @user2079303 The compiler can't ellide in the second case, because we def. want a copy of the object. –  Jul 27 '18 at 09:52
  • @johannes Moving is relatively cheap, in contrast to copying. So it is more interesting to compare moving twice vs. copying once... – Aconcagua Jul 27 '18 at 09:55
  • 1
    Summary so far: three situations: By value: passing l-value: copy + move. Passing r-value: move + move, in cases where copy elision can occur: only one move. By reference: passing l-value: copy; passing r-value: copy (both cases). – Aconcagua Jul 27 '18 at 10:00
  • @Aconcagua Then the single "by value" constructor performs better than the single "by const-ref". The most efficient solution would be the one you've mentioned, at the "cost" of writing 2 constructors. –  Jul 27 '18 at 10:00
  • 2
    @johannes correct. The overhead of using shared ownership, or even using just the use of dynamic allocation is so high, that I would not care about the minuscule cost of one extra move. Note that the overloading becomes unbearable when there are more than one member to deal with. – eerorika Jul 27 '18 at 10:20
3

My personal approach would be using two constructors:

struct MyClass
{
    MyClass(std::shared_ptr<MyDependency> const& dep) 
        : dep(dep)
    {}
    MyClass(std::shared_ptr<MyDependency>&& dep) 
        : dep(std::move(dep))
    {}

    std::shared_ptr<MyDependency> dep;
};

L-values being passed, with the outer pointer retaining ownership, are just copied once as normally, r-values still are only references and thus just moved only once.

Aconcagua
  • 24,880
  • 4
  • 34
  • 59