Question

I am doing it like this:

class Something;
Something f();
...
std::shared_ptr<Something> ptr(new Something(f()));

but this doesn't feel right. Moreover it needs the copy constructor. Is there a better way?

Was it helpful?

Solution

Use std::make_shared to avoid explicitly calling new. Similarly, use std::make_unique.

make_shared might be more efficient because it can allocate the counters for the smart-pointer and the object in one block together.

Still, it does not come into its own until you have at least one more way for your statement to cause an exception after construction of the object but before it is safely ensconced in its smart-pointer. Said exceptions would otherwise cause a memory-leak.

Example for bad behaviour:

void f(std::shared_ptr<int> a, std::shared_ptr<int> b);

f(std::shared_ptr<int>(new int(0)), std::shared_ptr<int>(new int(4)));

And corrected:

f(std::make_shared<int>(0), std::make_shared<int>(4));

Now, someone advises you to return Something not by value but as a dynamically allocated pointer. For your use-case, there's actually no difference with an acceptable compiler as long as Something is copyable, due to copy-ellision, aka directly constructing the returned value in the space allocated by new/make_shared/make_unique.
So, just do what you think best there.

Copy-ellision is explicitly allowed by the standard. Just be aware the copy-constructor must be accessible anyway:

12.8. Copying and moving class objects §32

When certain criteria are met, an implementation is allowed to omit the copy/move construction of a class object, even if the copy/move constructor and/or destructor for the object have side effects. In such cases, the implementation treats the source and target of the omitted copy/move operation as simply two different ways of referring to the same object, and the destruction of that object occurs at the later of the times when the two objects would have been destroyed without the optimization.123 This elision of copy/move operations, called copy elision, is permitted in the following circumstances (which may be combined to eliminate multiple copies):
— in a return statement in a function with a class return type, when the expression is the name of a non-volatile automatic object (other than a function or catch-clause parameter) with the same cvunqualified type as the function return type, the copy/move operation can be omitted by constructing the automatic object directly into the function’s return value
— in a throw-expression, when the operand is the name of a non-volatile automatic object (other than a function or catch-clause parameter) whose scope does not extend beyond the end of the innermost enclosing try-block (if there is one), the copy/move operation from the operand to the exception object (15.1) can be omitted by constructing the automatic object directly into the exception object
— when a temporary class object that has not been bound to a reference (12.2) would be copied/moved to a class object with the same cv-unqualified type, the copy/move operation can be omitted by constructing the temporary object directly into the target of the omitted copy/move
— when the exception-declaration of an exception handler (Clause 15) declares an object of the same type (except for cv-qualification) as the exception object (15.1), the copy/move operation can be omitted by treating the exception-declaration as an alias for the exception object if the meaning of the program will be unchanged except for the execution of constructors and destructors for the object declared by the exception-declaration.

OTHER TIPS

You can use std::make_shared. It is better to use it for the following reason:

This function typically allocates memory for the T object and for the shared_ptr's control block with a single memory allocation (it is a non-binding requirement in the Standard). In contrast, the declaration std::shared_ptr p(new T(Args...)) performs at least two memory allocations, which may incur unnecessary overhead.

The better way would be to have f() return Something* (allocated with new) or shared_ptr<Something>. Otherwise, the Something returned by f() will have automatic storage and putting it in a shared_ptr doesn't make sense. You could, in theory, use a shared_ptr with a custom deleter, but that wouldn't change the storage class of the underlying object, and you'd most likely just end up with a wild pointer.

If you can't change f(), your solution of making a copy with dynamic storage is really all you can do. If you can give Something a move constructor, you could at least reduce the cost of making the copy (assuming it's expensive enough to be worth reducing).

But see this answer for why the copy isn't worth worrying about. Do whatever you think makes the code most readable.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top