CODEWITHSUNDEEP
c++raii
btshengsheng
btshengsheng

Reputation: 723

How is destructor called for temporary objects returned from a function in C++?

Here's a code from Stroustrup's "The C++ Programming Language" that implements a finally which I cannot quiet understand where the destructor gets called.

template<typename F> struct Final_action
{
  Final_action(F f): clean{f} {}
  ~Final_action() { clean(); }
  F clean;
}

template<class F> 
Final_action<F> finally(F f)
{
  return Final_action<F>(f);
}

void test(){
  int* p=new int{7};
  auto act1 = finally( [&]{delete p;cout<<"Goodbye,cruel world\n";} );
}

I have two questions around this:

  1. According to the author, the delete p only gets called once: when act1 goes out of scope. But from my understanding: first, act1 will be initialized with the copy constructor, then the temporary object Final_action<F>(f) in the function finally gets destructed, calling delete p for the first time, then a second time at the end of the function test when act1 is out of scope. Where am I getting it wrong?

  2. Why is the finally function needed? Can't I just define Final_action act1([&]{delete p;cout<<"Goodbye,cruel world\n"})? Is that the same?

Also, if anyone can think of a better title, please modify the current one.

UPDATE: After some further thinking, I'm now convinced that the destructor may be called thrice. The additional one is for the temporary objected auto-generated in the calling function void test() used as the argument to the copy constructor of act1. This can be verified with -fno-elide-constructors option in g++. For those who have the same question as me, see Copy elision as well as Return value optimization as pointed out in the answer by Bill Lynch.

Upvotes: 16

Views: 1517

Answers (3)

T.C.
T.C.

Reputation: 137310

Simplest fix is

template<typename F> 
struct Final_action
{
  Final_action(F f): clean{std::move(f)} {}
  Final_action(const Final_action&) = delete;
  void operator=(const Final_action&) = delete;
  ~Final_action() { clean(); }
  F clean;
};

template<class F> 
Final_action<F> finally(F f)
{
  return { std::move(f) };
}

And use as

auto&& act1 = finally( [&]{delete p;cout<<"Goodbye,cruel world\n";} );

The use of copy-list-initialization and a lifetime-extending forwarding reference avoid any copy/moving of the Final_action object. Copy-list-initialization constructs the temporary Final_action return value directly, and the temporary returned by finally has its lifetime extended by binding to act1 - also without any copying or moving.

Upvotes: 5

Arne Vogel
Arne Vogel

Reputation: 6666

The code is broken. The revised code mentioned by SimonKraemer is also broken – it doesn't compile (the return statement in finally is illegal because Final_action is neither copyable nor moveable). Making Final_action move-only with generated move constructor also doesn't work because is F guaranteed to have a move constructor (if it hasn't, then Final_action's generated move constructor will silently use F's copy constructor as a fallback), nor is F guaranteed to be a no-op after the move. In fact, the lambda from the example will not turn into a no-op.

There is a relatively easy and portable solution:

Add a flag bool valid = true; to Final_action and overwrite move c'tor and move assignment to clear the flag in the source object. Only invoke clean() if valid. This prevents the generation of copy c'tor and copy assignment, so they do not need to be explicitly deleted. (Bonus points: Put the flag into a reusable move-only wrapper so that you don't have to implement the move c'tor and move assignment of Final_action. You don't need the explicit deletes in this case either.)

Alternatively, remove Final_action's template argument and change it to use std::function<void()> instead. Check that clean is not empty before invoking it. Add move c'tor and move assignment that set the original std::function to nullptr. (Yes, this is necessary to be portable. Moving a std::function does not guarantee that the source will be empty.) Advantage: The usual benefits of type erasure, such as being able to return the scope guard into an outer stack frame without exposing F. Disadvantage: may add significant run time overhead.

In my current work project, I basically combined the two approaches with a ScopeGuard<F> and an AnyScopeGuard using a type erasing function object. The former uses a boost::optional<F> and can be converted to the latter. As an added benefit of allowing scope guards to be empty, I can explicitly dismiss() them as well. This allows using the scope guard to set up the rollback part of a transaction and then dismissing it upon commit (with non-throwing code).

UPDATE: Stroustrup's new example doesn't even compile. I missed that explicitly deleting the copy c'tor also disables the generation of the move c'tor.

Upvotes: 3

Bill Lynch
Bill Lynch

Reputation: 81916

You are correct, this code is broken. It only works correctly when return value optimizations are applied. This line:

auto act1 = finally([&]{delete p;cout<<"Goodbye,cruel world\n"})

May or may not invoke the copy constructor. If it does, then you will have two objects of type Final_action and you will thus call that lambda twice.

Upvotes: 13

Related Questions