How to implement Scope Guard that restores value upon scope exit?

Question

Would the following be an idiomatic C++11 implementation of a Scope Guard that restores a value upon scope exit?

template<typename T>
class ValueScopeGuard
{
public:
    template<typename U>
    ValueScopeGuard(T& value, U&& newValue):
        _valuePtr(&value),
        _oldValue(std::forward<U>(newValue))
    {
        using std::swap;
        swap(*_valuePtr, _oldValue);
    }
    ~ValueScopeGuard()
    {
        if(_valuePtr)
        {
            using std::swap;
            swap(*_valuePtr, _oldValue);
        }
    }

    // Copy
    ValueScopeGuard(ValueScopeGuard const& other) = delete;
    ValueScopeGuard& operator=(ValueScopeGuard const& other) = delete;

    // Move
    ValueScopeGuard(ValueScopeGuard&& other):
        _valuePtr(nullptr)
    {
        swap(*this, other);
    }
    ValueScopeGuard& operator=(ValueScopeGuard&& other)
    {
        ValueScopeGuard(std::move(other)).swap(*this);
        return *this;
    }

private:
    T* _valuePtr;
    T _oldValue;

    friend void swap(ValueScopeGuard& lhs, ValueScopeGuard& rhs)
    {
        using std::swap;
        swap(lhs._valuePtr, rhs._valuePtr);
        swap(lhs._oldValue, rhs._oldValue);
    }
};

template<typename T, typename U>
ValueScopeGuard<T> makeValueScopeGuard(T& value, U&& newValue)
{
    return {value, std::forward<U>(newValue)};
}

It could be used to temporarily change a value as follows:

int main(int argc, char* argv[])
{
    // Value Type
    int i = 0;
    {
        auto guard = makeValueScopeGuard(i, 1);
        std::cout << i << std::endl; // 1
    }
    std::cout << i << std::endl; // 0

    // Movable Type
    std::unique_ptr<int> a{new int(0)};
    {
        auto guard = makeValueScopeGuard(a, std::unique_ptr<int>{new int(1)});
        std::cout << *a << std::endl; // 1
    }
    std::cout << *a << std::endl; // 0

    return 0;
}

Is a simple utility like this already implemented in a library somewhere? I had a look at Boost.ScopeExit, but its intended usage seems different and more complex.

Use ’template ValueScopeGuard(T& value, U&& newValue)’ for the constructor. Similarly use a separate template type for ’makeValueScopeGuard’. That will allow perfect forwarding to work and allow both l-values and r-values. — John5342, Mar 26 '14 at 15:35
@John5342: Ah, I see what's going on now! `T&&` is not a 'universal' reference since `T` is being deduced from the first argument. Suggestion incorporated. — kloffy, Mar 26 '14 at 15:44

score 3 · Answer 1 · edited May 23 '17 at 11:49

3

Assuming makeValueScopeGuard to be implemented as :

template< typename T >
ValueScopeGuard<T> makeValueScopeGuard( T& t, T&& v )
{
    return ValueScopeGuard<T>(t,std::move(v));
}

no, it is not very good implementation of scope guard, because it is going to fail when you pass l-values as the 2nd parameter :

int kk=1;
auto guard = makeValueScopeGuard(i, kk);

The second problem is that you used std::forward, when you should have used std::move.

As this question and answers show, people are usually using lambdas to implement scope guard.

edited May 23 '17 at 11:49

Community

1
1

answered Mar 26 '14 at 15:10

BЈовић

62,405
41
173
273

Good point about the l-values, that could definitely be an issue. However, I do not quite follow why `std::move` is preferable in this case? My intention was to just forward the argument to the constructor... – kloffy Mar 26 '14 at 15:26
Regarding your edit: I am aware of the lambda solutions. However, for my actual use case it seems overly complicated, since I just want to set the value and restore it upon scope exit. That was my motivation for taking a shot at the implementation... – kloffy Mar 26 '14 at 15:32
1

@kloffy You need `std::move`, and not `std::forward`, because that is a rvalue - not universal reference. – BЈовић Mar 26 '14 at 16:05
1

@kloffy Maybe the best would be to change the constructor to this : `ValueScopeGuard(T& value, U newValue):` and use move. – BЈовић Mar 26 '14 at 16:06
I should have probably made it more explicit in the question, but I would like the guard to work with non-copyable objects like `std::unique_ptr` in the example code. – kloffy Mar 26 '14 at 16:32

Jonathan Wakely · Accepted Answer · 2014-03-26T15:14:59.440

Your move constructor leaves the pointer member uninitialized, so the rvalue object ends up holding a junk pointer, which it dereferences in its destructor. That's a bug. You should initialize it to nullptr and check for nullptr in the destructor.

For a type like this I would not expect move assignment to be a simple swap, I would expect the rvalue to end up not owning anything. So I would implement the move like this instead, so the rvalue ends up empty:

ValueScopeGuard& operator=(ValueScopeGuard&& other)
{
    ValueScopeGuard(std::move(other)).swap(*this);
    return *this;
}

The name makeValueScopeGuard isn't clear to me that it changes the value itself, I'd expect it to just copy the current value and restore it in the destructor.

As far as existing types go, the closest I can think of is the Boost I/O state savers, which do not alter the current state they just copy it and restore it.

Good catches! I have fixed the (retrospectively) obvious mistakes. Thank you for the suggestions. I could definitely use more practice with move semantics. — kloffy, Mar 26 '14 at 16:35

How to implement Scope Guard that restores value upon scope exit?

2 Answers2