Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Swapping with rvalues

Suppose I want swap that works on rvalues, and don't want to write 4 versions for all combinations of rvalue/lvalue references (rvalue/rvalue version is kinda pointless but it doesn't hurt). I came up with this:

template <typename A, typename B>
struct is_same_no_ref
    : std::is_same<
        typename std::remove_reference<A>::type,
        typename std::remove_reference<B>::type
    >
{};

template <typename A, typename B,
    typename = typename std::enable_if<is_same_no_ref<A, B>::value>::type
>
inline void my_swap(A&& a, B&& b) {
    typename std::remove_reference<A>::type t = std::move(a);
    a = std::move(b);
    b = std::move(t);
}

which seems to work as expected. Is this okay? Or am I missing something important that will make me suffer later?

like image 494
yuri kilochek Avatar asked Sep 22 '13 09:09

yuri kilochek


1 Answers

While I see no inherent concept flaw in your implementation, I would make three suggestions.

(note: I'll refer here to an implementation of the concept of swaping rvalues as swap_rvalues)


Exclude const types from the template deduction

  • How?

Assuming

template <typename A, typename B>
struct is_same_no_ref
    : std::is_same<
        typename std::remove_reference<A>::type,
        typename std::remove_reference<B>::type
    >
{};

change the enable condition from std::enable_if<std::is_same_no_ref<A, B>::value> into the following.

std::enable_if<
    std::is_same_no_ref<A, B>::value &&
    !std::is_const<typename std::remove_reference<A>::type>::value &&
    !std::is_const<typename std::remove_reference<B>::type>::value
>
  • Why?

Without excluding const types from the template deduction, passing const variables to swap_rvalues, as in the following,

int const a = 0, b = 0;
swap_rvalues(a, b);

induces the compiler to flag errors regarding internal implementation details, what is not very user-friendly.


Move the enable condition to the return type declaration

  • How?

Instead of

template<typename A, typename B, typename = typename std::enable_if<...>::type>
inline void swap_rvalues(A&& a, B&& b);

declare it like the following

template<typename A, typename B>
inline typename std::enable_if<...>::type swap_rvalues(A&& a, B&& b);
  • Why?

Even though highly improbable, the explicit definition of the third template parameter of swap_rvalues is possible, effectively overriding the enable condition. This may allow code to compile which shouldn't and nastiness could follow. This is completely avoided using the return type declaration for the enable condition.

Consider the following example.

template <
        typename A, 
        typename B, 
        typename = typename std::enable_if<
            is_same_no_ref<A, B>::value &&
            !std::is_const<typename std::remove_reference<A>::type>::value &&
            !std::is_const<typename std::remove_reference<B>::type>::value
        >::type>
inline void
swap(A&& a, B&& b) {
    typename std::remove_reference<A>::type t = std::move(a);
    a = std::move(b);
    b = std::move(t);
}

struct B;
struct A{A(){} A(B const&){}};
struct B{B(){} B(A const&){}};
swap<A, B, void>(A(), B());

It compiles, even though it clearly shouldn't! A and B are not even related, they just happen to be constructable given a reference of the other.


Reuse code [aka KISS]

  • How?

Since the rvalues are already given a name, simply forward call std::swap, instead of providing a brand new implementation of swap_rvalues.

  • Why?

Why reinventing the wheel†? std::swap already provides the intended behavior once rvalues are given a name, so why not reusing it?


Conclusion

The final implementation of swap_rvalues‡ would look like follows.

template <typename A, typename B>
inline typename std::enable_if<
    is_same_no_ref<A, B>::value &&
    !std::is_const<typename std::remove_reference<A>::type>::value &&
    !std::is_const<typename std::remove_reference<B>::type>::value
>::type
swap_rvalues(A&& a, B&& b) {
    std::swap(a, b);
}

Footnotes

"To reinvent the wheel is to duplicate a basic method that has already previously been created or optimized by others."

swap_rvalues in fact would better be called swap in a real scenario.

like image 126
brunocodutra Avatar answered Nov 06 '22 00:11

brunocodutra