Is this item from Effective Modern C++ still up to date?

489 views Asked by At

p.146 of effective modern C++ :

void processWidget(std::shared_ptr<Widget> spw, int priority);
void cusDel(Widget *ptr);//a custom deleter

This was an unsafe call prior to C++17 :

processWidget(std::shared_ptr<Wdiget>(new Widget, cusDel), computePriority());

It used to be unsafe because computePriority could be called after new Widget but before std::share_ptr constructor and if computePriority yielded an exception, the dynamically allcoated Widget would be leaked. Therefore you could do this :

std::shared_ptr<Widget> spw(new Widget, cusDel);
processWidget(spw, computePriority());

Now this would add a copy constructor operation on shared_ptr. So you could also do this :

std::shared_ptr<Widget> spw(new Widget, cusDel);
processWidget(std::move(spw), computePriority());

So my question is, is the following code still able to leak memory in C++17?

processWidget(std::shared_ptr<Wdiget>(new Widget, cusDel), computePriority());

I've read this and this but I'm still unsure, I believe that std::shared_ptr<Wdiget>(new Widget, cusDel) and computePriority() are both sequenced before the call to processWidget, however I think computePriority() can still throw an exception after new and before shared_ptr takes ownership of the newly created object.

2

There are 2 answers

1
Nicol Bolas On BEST ANSWER

C++17 did change the sequencing with regard to the evaluation of the expressions used to call a function. While it doesn't impose any particular order, it does say:

The initialization of a parameter, including every associated value computation and side effect, is indeterminately sequenced with respect to that of any other parameter.

"Indeterminately sequenced" is defined as:

Evaluations A and B are indeterminately sequenced when either A is sequenced before B or B is sequenced before A, but it is unspecified which.

So one parameter will be initialized before the other, including all side-effects. So either the first argument or the second argument is fully evaluated before the other.

3
einpoklum On

A suggestion rather than a direct answer to your question.

A principle I would apply to this case: If the safety of a statement is not obvious, then just don't assume it - even if language-lawyering can find you the appropriate guarantee. Code which makes subtle assumptions is usually not a good idea to write. What if someone then tries to compile this with C++14 due to some compatibility constraints? They won't get a warning.

Instead, save yourself the trouble by either using your two-line solution (with std::move), or write a version of make_shared() which also takes the custom deleter - probably something like this:

template< class T, class Deleter, class... Args >
shared_ptr<T> make_shared_with_deleter( Deleter d, Args&&... args )
{
    return std::shared_ptr<T>(new T(std::forward(args)...), d );
}

and then call:

processWidget(nonstd::make_shared_with_deleter<Widget>(cusDel), computePriority());

See also the same argument regarding tuple destruction order.