Search code examples
c++c++11referenceoverloadingguideline-support-library

Understanding the need for this `const&` specialization


In the Guidelines Support Library there is a class called final_action (essentially the well known ScopeGuard). There are 2 free-standing convenience functions to generate this templated class:

// finally() - convenience function to generate a final_action
template <class F>
inline final_action<F> finally(const F& f) noexcept
{
    return final_action<F>(f);
}

template <class F>
inline final_action<F> finally(F&& f) noexcept
{
    return final_action<F>(std::forward<F>(f));
}

(source: https://github.com/Microsoft/GSL/blob/64a7dae4c6fb218a23b3d48db0eec56a3c4d5234/include/gsl/gsl_util#L71-L82)

What is the need for the first one? If we only had the second one (using the forwarding , a.k.a. universal, references) wouldn't it do the same thing?


Solution

  • Let's consider the perfectly-forwarding version:

    • When called with an rvalue, it will return final_action<F>(static_cast<F&&>(f)).

    • When called with an lvalue, it will return final_action<F&>(f).

    Let's now consider the const F& overload:

    • When called both an lvalue or rvalue, it will return final_action<F>(f).

    As you can see, there is an important difference:

    • Passing a non-const lvalue reference to finally will produce a wrapper that stores a F&

    • Passing a const lvalue reference to finally will produce a wrapper that stores a F

    live example on wandbox


    I am not sure why it was deemed necessary to have the const F& overload.

    This is the implementation of final_action:

    template <class F>
    class final_action
    {
    public:
        explicit final_action(F f) noexcept : f_(std::move(f)), invoke_(true) {}
    
        final_action(final_action&& other) noexcept 
            : f_(std::move(other.f_)), invoke_(other.invoke_)
        {
            other.invoke_ = false;
        }
    
        final_action(const final_action&) = delete;
        final_action& operator=(const final_action&) = delete;
    
        ~final_action() noexcept
        {
            if (invoke_) f_();
        }
    
    private:
        F f_;
        bool invoke_;
    };
    

    Unless I am missing something, instanting final_action<F&> doesn't really make sense, as f_(std::move(f)) will not compile.

    live example on wandbox

    So I think this should have just been:

    template <class F>
    inline final_action<F> finally(F&& f) noexcept
    {
        return final_action<std::decay_t<F>>(std::forward<F>(f));
    }
    

    Ultimately, I think that the implementation of finally in GSL incorrect/unoptimal (i.e. redundant, has code repetition).