4

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.

Enlico
  • 23,259
  • 6
  • 48
  • 102
user
  • 934
  • 6
  • 17
  • Ok, I stand corrected. On a side note, my liking towards C++ has not grown a bit today. – tevemadar Oct 17 '20 at 14:08
  • 2
    @tevemadar I'm sorry to hear that. I'm hopeful that you only feel that way about *some* c++ programmers. The language is actually pretty cool :) – cigien Oct 17 '20 at 14:12
  • @cigien I did not say anything about programmers using C++, but that example with `int y = operator<<(S(j=1), j=2);`. There almost seems to be some actual proudness about the indeterminism of `j` at the end despite the mathematical meaning of parentheses, and perhaps common sense too. It's a bit like the `8/2(2+2)` meme, just it got real. – tevemadar Oct 17 '20 at 14:21
  • @tevemadar I'm not sure it's pride, it's just a recognition of the guarantees as provided by the language. – cigien Oct 17 '20 at 14:25
  • @cigien following your comment I just came back, and then re-read the thing. I was commenting on the wrong part here. I didn't care about the indeterminism of `j`, that's just an evaluation order, which argument (of `operator<<()`) comes first, I can live with that being "random". I care about the "random" sequencing of `S()`. Should `j` be a global variable, this indeterminately sequenced nature could mean that `S()` gets `1` as its argument, but sees `j=2` for the variable. Actually that's what I don't like. – tevemadar Oct 17 '20 at 21:24
  • @tevemadar Agreed, and that's what is fixed in c++17. Now `S()` is guaranteed to see `j=1`. – cigien Oct 17 '20 at 21:26
  • @cigien: Does the Standard mandate everything an implementation would need to do to be suitable for every purpose, for some particular purposes, or for no particular purpose? If only for some purposes, what purposes are those? Since C++ was used for 14 years before the first standard was published, I don't think it would be fair to equate the Standard with "the language" unless the Standard includes everything needed to make implementations suitable for all the purposes that were accomplished with C++ before the Standard was published. – supercat Oct 19 '20 at 20:23

2 Answers2

6

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.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
0

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.

Enlico
  • 23,259
  • 6
  • 48
  • 102
einpoklum
  • 118,144
  • 57
  • 340
  • 684
  • 3
    The whole point of standardizing evaluation of parameters is to make sure that people *can* rely on it, so that they *don't have* to write such functions. – Nicol Bolas Oct 17 '20 at 13:47
  • @NicolBolas: Actually, I would try to avoid use raw `new` in the middle of some long expression anyway. Standardizing certain subtleties may have some benefits, but relying on such subtleties is still not necessarily good practice. – einpoklum Oct 17 '20 at 14:04
  • 2
    It's not about `new` specifically. It could be any function call or evaluation. Consider pack expansions or fold expressions over range expressions. Without some basic guarantees about evaluation sequencing, you wouldn't be able to effectively use them. – Nicol Bolas Oct 17 '20 at 14:10