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

Question

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.

Mulvey

One question per question, please. – Ita

re #2 I don't see why not. I guess someone obsessed with auto thought this was easier to read. – Ita

@LightnessRacesinOrbit Template argument deduction. You can't actually write the type of the Final_action<???> because of the lambda. – Harpist

@Quentin: Haha oh yeah – Ita

Read the chapter again. The copy constructor and the operator= are both set to delete to prevent Final_action from being copied. Furthermore your example code misses at least one semicolon after the cout – Mira

@Harpist Does lambda have a type? Is it the same type as a function pointer? Maybe a silly question, I'm a novice :D – Mulvey

Update: This vs This – Mira

@Mulvey a lambda-expression creates a function object (functor), of a type that is generated by the compiler. The type's name is unknown to the programmer, but apart from that it's a regular class with an operator (). – Harpist

@Mulvey re #2 You can do it, but it will be very monstrous: Final_action<std::function<void()> > act1([&](){ delete p; cout<<"Goodbye,cruel worldv1\n";}); and also need include <functional> – Ulund

@Ulund Please don't. Compare this and this – Mira

@Ulund that would be abusing std::function. – Harpist

Answer 1

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.

Minim

Probably someone saying "well this is how it works on my system" and not caring enough to write even remotely portable code .. or even code that fails properly :( – Ita

Note : to fix it, make Final_action a movable-only type. – Harpist

@Quentin: Even easier fix: auto&& -- extends the lifetime of the temporary to match act1, prevents any copying – Richter

@BenVoigt This would work, but leaves the landmine in the field. – Harpist

@Quentin: Well, copy and move operations on Final_action should both be deleted, because neither makes sense (a moved-from instance will also blow up, unless that class is totally rewritten). – Richter

Looks like the chapter got an update: Here vs Here – Mira

For #2, which you may want to cover in your answer, you could have auto deleter = [&] { /* bla bla */ }; Final_action<decltype(deleter)> act1{ deleter}; – Desertion

@BenVoigt If both copy and move operations on Final_action is deleted, how will the object be returned? – Mulvey

@SimonKraemer The updated code is incorrect as well, see the answer by Arne Vogel. – Mulvey

@bsthengsheng I didn't test it, but thanks for pointing it out. – Mira

Answer 2

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.

Volotta

Works fine! For those who may wonder, the curly brace in the return statement return { std::move(f) } invokes the copy-list-initialization, see here. – Mulvey

Answer 3

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.

Dillman

The revised code by SimonKraemer indeed won't compile (I've tested with g++), because as you said, move constructor won't be auto-generated when copy constructor is explicitly declared (as deleted), see here. – Mulvey

After reading your first paragraph 6 times, I think I finally fully get what you mean :-). I'm assuming you missed a neither on the fourth line of your answer? BTW, It's quite interesting to see Stroustrup producing incorrect code over and over again... Thank you. – Mulvey