Understanding the need for this `const&` specialization
Asked Answered
U

1

7

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?

Unvoice answered 25/1, 2018 at 15:5 Comment(0)
D
8

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).

Dicrotic answered 25/1, 2018 at 15:12 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.