Question

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?

Was it helpful?

Solution

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.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top