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.

Answer 1

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.

Answer 2

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.