0

I know about ADL and the swap idiom:

using std::swap;
swap(x, y);

boost::swap() does the above for you. Now, I want to push it further. Specifically, Have the swap perform x.swap(y) if possible, and fallback to boost::swap() otherwise. So, you don't have to implement both a member swap and a free one, which is verbose and redundant. I tried to implement such a swap and ended up with the following. Implementing things like this can become quite tricky. So, I'm wondering whether there is any flaws in my implementation, or if more succinct implementations exist.

#include <algorithm>
#include <utility>

namespace cppu_detail_swap {

template <typename T>
void swap_impl(T& x, T& y) {
  using std::swap;
  swap(x, y);
}

} // namespace cppu_detail_swap

namespace cppu {

namespace detail {

template <typename T>
void swap(T& x, T& y, int) {
  cppu_detail_swap::swap_impl(x, y);
}

template <typename T>
auto swap(T& x, T& y, char) -> decltype(x.swap(y)) {
  return x.swap(y);
}

} // namespace detail

template <typename T>
void swap(T& x, T& y) {
  detail::swap(x, y, ' ');
}

} // namespace cppu
Lingxi
  • 14,579
  • 2
  • 37
  • 93
  • I would be more direct, and overload in `has_member_swap` traits class instead of your `int` `char` hack. This involves writing `has_member_swap`, which is easy post C++1z or post writing `can_apply – Yakk - Adam Nevraumont Jan 23 '16 at 21:36
  • @Yakk I think that would not be less code. No? – Lingxi Jan 23 '16 at 21:50
  • depends if you include the library that gives you `can_apply` or not. After that, it is less code. More importantly, it is more direct about what the dispatching is about, instead of being hacky/underhanded about it (the call to `detail::swap` is not about char to int conversion). – Yakk - Adam Nevraumont Jan 23 '16 at 21:51

1 Answers1

1

Your current solution is flawed for objects from the cppu namespace, e.g.

// [insert your code here]

namespace cppu
{
  struct X{};
  struct Y{ void swap(Y& y) { }; };
}


int main()
{
  auto x1 = cppu::X{};
  auto x2 = cppu::X{};
  swap(x1, x2);

  auto y1 = cppu::Y{};
  auto y2 = cppu::Y{};
  swap(y1, y2);
}

g++ tells me:

taste.cpp:9:7: error: call of overloaded ‘swap(cppu::X&, cppu::X&)’ is ambiguous

To get rid of this, you need to explicitly call std::swap in swap_impl, which is OK, since you arrived here through the cppu::swap implementation already. But then you do not use overloads for other types. Thus, I think you need to distinguish three cases:

  1. Has own swap member function
  2. Has no swap member function and is from namespace cppu
  3. Has no swap member function and is any other namespace (here you need to use the ADL swap idiom).

Also, I concur with @Yakk that I would be more direct instead of using the int/char hack.

So let's go for it:

A helper for checking the availability of the swap member:

namespace cppu
{
  namespace detail
  { 
    template <typename T>
    using void_t = void;

    template <typename T, typename = void>
    struct has_member_swap
    {
      static constexpr bool value = false;
    };

    template <typename T>
    struct has_member_swap<
        T,
        void_t<decltype(std::declval<T&>().swap(std::declval<T&>()))>>
    {
      static constexpr bool value = true;
    };
  }
}

And a helper to check if T is from namespace cppu, see also here:

namespace helper
{
  template <typename T, typename = void>
  struct is_member_of_cppu : std::false_type
  {
  };

  template <typename T>
  struct is_member_of_cppu<
      T,
      decltype(adl_is_member_of_cppu(std::declval<T>()))> : std::true_type
  {
  };
}   

namespace cppu
{   
  template <typename T>
  auto adl_is_member_of_cppu(T && ) -> void;
}

Now we can write all three overloads:

namespace cppu
{
  namespace detail
  {
    template <
        typename T,
        typename = std::enable_if_t<helper::is_member_of_cppu<T>::value and
                                    not has_member_swap<T>::value>>
    auto swap(T& x, T& y) 
        -> std::enable_if_t<helper::is_member_of_cppu<T>::value and
                            not has_member_swap<T>::value>
    {
      std::cout << "cppu-type without member swap";
      std::swap(x, y);
    }

    template <
        typename T,
        typename = std::enable_if_t<not helper::is_member_of_cppu<T>::value and
                                    not has_member_swap<T>::value>>
    auto swap(T& x, T& y)
        -> std::enable_if_t<not helper::is_member_of_cppu<T>::value and
                            not has_member_swap<T>::value>
    {
      std::cout << "not cppu-type without member swap";
      using std::swap;
      swap(x, y);
    }

    template <typename T, typename = std::enable_if_t<has_member_swap<T>::value>>
    auto swap(T& x, T& y) -> decltype(x.swap(y))
    {
      std::cout << "member swap";
      return x.swap(y);
    }

  }
}

Call this as you did before:

namespace cppu
{
  template <typename T>
  void swap(T& x, T& y)
  {
    detail::swap(x, y);
  }   
}

And finally: Test the whole thing.

namespace cppu
{
  struct X{};
  struct Y{ void swap(Y& y) { }; };
}

struct A{};
struct B{ void swap(B& y) { }; };
struct C{};
auto swap(C&, C&) -> void { std::cout << " with own overload"; }

static_assert(helper::is_member_of_cppu<cppu::X>::value, "");
static_assert(helper::is_member_of_cppu<cppu::Y>::value, "");
static_assert(not helper::is_member_of_cppu<A>::value, "");
static_assert(not helper::is_member_of_cppu<B>::value, "");


int main()
{
  auto x1 = cppu::X{};
  auto x2 = cppu::X{};
  std::cout << "X: "; swap(x1, x2); std::cout << std::endl;

  auto y1 = cppu::Y{};
  auto y2 = cppu::Y{};
  std::cout << "Y: "; swap(y1, y2); std::cout << std::endl;

  auto a1 = A{};
  auto a2 = A{};
  std::cout << "A: "; cppu::swap(a1, a2); std::cout << std::endl;

  auto b1 = B{};
  auto b2 = B{};
  std::cout << "B: "; cppu::swap(b1, b2); std::cout << std::endl;

  auto c1 = C{};
  auto c2 = C{};
  std::cout << "C: "; cppu::swap(c1, c2); std::cout << std::endl;
}

The output is as expected (IMHO):

X: cppu-type without member swap Y: member swap A: not cppu-type without member swap B: member swap C: not cppu-type without member swap with own overload

Community
  • 1
  • 1
Rumburak
  • 3,416
  • 16
  • 27
  • That's a very nice catch! But calling `std::swap()` within `swap_impl()` doesn't solve the issue. Cause I want `cppu::swap()` to do ADL under the hood like `boost::swap()`. Your suggested solution also has this issue. We should find a fix to this. By the way, `has_member_swap` is tricky, yet very smart! You rock! – Lingxi Jan 24 '16 at 11:04
  • 1
    @Lingxi The code I posted does not cover all three cases. It just shows how to make it work for cases 1 and 2. I posted a way to check for the namespace [here](http://stackoverflow.com/questions/34974844/check-if-a-type-is-from-a-particular-namespace) (not sure if this is the best option, though). You need to use such a check to create third overload for `cppu::detail::swap`, I think. – Rumburak Jan 24 '16 at 11:13
  • In fact, I started a GitHub project to write trinkets like this (mainly due to interest in C++). There are many pitfalls in writing such things, as you might have imagined. The path is difficult. It would be very nice to have comrades like you along the way :) With an extra pair of sharp eyes, things should be easier and more interesting. Anyway, here is a [link](https://github.com/Lingxi-Li/CPP_Utility/issues/1) to the issue tracking this bug. Hope you like writing handy trinkets too :) – Lingxi Jan 24 '16 at 11:21
  • I think a simple fix would be using a name other than `swap()`. `intro_swap()`, for example. `intro` is short for introspective. – Lingxi Jan 24 '16 at 11:28
  • @Lingxi Added the third overload and some tests. Thanks for the github link. Starred it... – Rumburak Jan 24 '16 at 11:56