Proper way to upcast unique_ptr when returning from a factory

Question

I learned C++ in the 1990s, but haven't used it much since then. I'm trying to catch up on the last couple of decades by using modern techniques in a small library I maintain at work. I've come across this stylistic question, and I'd like to know what modern consensus is.

I have a pure virtual base class Base, with two implementations A and B. The implementations are not in the header file; their public API is entirely part of Base. I have factory functions to construct them, which need to call a common initializer. These classes don't support copying, and can change owners during their lifetime, so I use a unique_ptr to keep things more sane.

In combination, that leads to a somewhat awkward construct:

unique_ptr<Base> rv = make_unique<A>();

I haven't seen cases where make_unique is explicitly given a template class. I also haven't seen cases where make_unique can't be used with auto. However, if I want RVO to apply, then rv (AFAICT) needs to have type unique_ptr<Base> rather than unique_ptr<A>.

Here, I have two deviations from "what I've seen before" in one line. That set off flags in my head, and made me think that I should ask the community for collective wisdom.

Ok, enough English; here's some code:

class Base {
 public:
  static std::unique_ptr<Base> MakeA();
  static std::unique_ptr<Base> MakeB();
 private:
  void Initialize();
};

class A : public Base {};
class B : public Base {};

std::unique_ptr<Base> Base::MakeA() {
  std::unique_ptr<Base> rv = std::make_unique<A>();
  rv->Initialize();
  return rv;
}

std::unique_ptr<Base> Base::MakeB() {
  std::unique_ptr<Base> rv = std::make_unique<B>();
  rv->Initialize();
  return rv;
}

Again, here's some constraints I'm adding for myself to keep things super-clean:

Since the classes A and B are implementation details, I don't make them visible externally.
As a consequence, the factory functions need to return a unique_ptr<Base> rather than unique_ptr<A> and unique_ptr<B>.
I'm trying to make this NRVO-friendly, since I have other move-only classes to deal with and want to practice making RVO-friendly functions.
Since the factory functions have to call the Initialize method, I have to return an lvalue.
I want to use make_unique instead of unique_ptr(new ...), for the usual reasons.

What's the usual way to construct this sort of thing?

Type of `rv` doesn't matter, you will have `move` either way. In case of `unique_ptr` while initializing it, in case of `unique_ptr` on return. — Yola, Apr 05 '18 at 08:26
But if you were using `unique_ptr(new Derived(...))`, mmm... Why don't you want to use it by the way? — Yola, Apr 05 '18 at 08:35
@Yola: I don't want to use `unique_ptr(new Derived(...))` because it's not exception-safe, and hence is discouraged in modern C++. See also https://stackoverflow.com/a/22571331/1184115 and https://herbsutter.com/gotw/_102/ . — Piquan, Apr 05 '18 at 08:38
> I haven't seen cases where make_unique is explicitly given a template class How else would `make_unique` know which class to construct? What should the result of `make_unique(10)` be? — Max Langhof, Apr 05 '18 at 08:49
I didn't describe the details of the `A()` constructor. In particular, I didn't say whether it could throw an exception. This is the main reason that `unique_ptr(new Derived(...))` seems to be discouraged in C++, at least, post-C++14: if the `Derived(...)` constructor can throw, then you end up leaking memory, and potentially other resources. See also the links in my previous reply for more info. — Piquan, Apr 05 '18 at 08:55
if constructor `A()` throws memory will be freed, no memory will be leaked. About other resources, that's your responsibility to write constructor properly to not leak other resources, this potential would happen with make_unique as well. — Yola, Apr 05 '18 at 09:03

Max Langhof · Answer 1 · 2018-04-05T08:53:22.297

1

if I want RVO to apply, then rv (AFAICT) needs to have type unique_ptr rather than unique_ptr

Since you are not actually returning or assigning the constructed object (just a (unique_)pointer to it), RVO is irrelevant here. unique_ptr is basically zero-overhead, it's a single pointer (i.e. one register). It may make a difference in terms of the C++ abstract machine (the constructors/operators of std::unique_ptr), but in practice this aspect will have no impact.

More to the point: At no point is the (move) assignment operator or the move/copy constructor of A or B called, with or without (N)RVO.

Note that this is not specific to unique_ptr - if you were to return a raw pointer, the same idea would apply.

edited Apr 05 '18 at 08:53

answered Apr 05 '18 at 08:39

Max Langhof

22,398
5
38
68

I realize that, as a single word, `unique_ptr` is quite cheap, and that a raw pointer is equally cheap. I'm trying to generalize to other move-only classes that I have where the move constructor is more expensive, so I can get practice writing NRVO-friendly functions. In other words, instead of `unique_ptr`, imagine I've got a more expensive class. – Piquan Apr 05 '18 at 09:03
@Piquan In that case it would be helpful to state the question with an example of those classes. The "imagine `unique_ptr` was much more expensive" hypothetical is hard to discuss because "expensive-to-move move-only class that is returned from a factory but not polymorphic (i.e. not a pointer or a thin wrapper around it)" does not seem like a sound design (but rather contradictory on multiple levels). If you have a concrete case then we could reason about that one - I would suspect the solution is not RVO but design changes. – Max Langhof Apr 05 '18 at 09:15

score 0 · Answer 2 · answered Apr 05 '18 at 09:00

Your design is good. Still if you care to eliminate move/copy on return which is the case with your design (of course if your compiler supports such an optimization), you might also want to eliminate it at the creation site. To this end you need to use

std::unique_ptr<Base> rv(new Derived(...));

Here you should be careful with what you place instead of ellipsis. About your comment, what usage of new is discouraged to use it in modern C++. It is not, see, make_unique internally uses new and it is written in modern C++.

But again, your design is good, and you could resort to my proposal only if you really care to avoid moves.

Proper way to upcast unique_ptr when returning from a factory

2 Answers2