9

As proper use of std::swap is:

using std::swap;
swap(a,b);

It is a bit verbose but it makes sure that if a,b have better swap defined it gets picked.

So now my question is why std::swap is not implemented using this technique so user code would just need to call std::swap?

So something like this (ignoring noexcept and constraints for brevity):

namespace std {
    namespace internal {
        template <class T>      // normal swap implementation
        void swap(T& a, T& b) { // not intended to be called directly
            T tmp = std::move(a);
            a = std::move(b);
            b = std::move(tmp);
        }
    }

    template <class T>
    void swap(T& a, T& b) {
        using internal::swap;
        swap(a,b);
    }
}
curiousguy
  • 8,038
  • 2
  • 40
  • 58
NoSenseEtAl
  • 28,205
  • 28
  • 128
  • 277
  • 1
    Probably would break a lot of existing code. Name clashes or change in behaviour. – Richard Critten Nov 19 '17 at 23:54
  • Sorry, am I missing something here? What is `std::internal_do_not_use_directly::swap;` supposed to be? –  Nov 20 '17 at 00:02
  • Neil Butterworth - as the comment says in very few words :) , it has the same implementation as current std::swap; – NoSenseEtAl Nov 20 '17 at 00:03
  • @NeilButterworth I believe what (s)he is proposing is to be `std::swap`, and actual `std::swap` should be implemented in `std::internal_do_not_use_directly::swap` – Amadeus Nov 20 '17 at 00:03
  • No, sorry, still don't understand. –  Nov 20 '17 at 00:06
  • 2
    @NoSenseEtAl: And how exactly is this not infinite recursion? Or an overload resolution conflict? After all, the two functions have the same signature and name; what will stop the compiler from calling itself? – Nicol Bolas Nov 20 '17 at 00:06
  • @NicolBolas How is either of those things possible? – Barry Nov 20 '17 at 00:11
  • @Barry thank you for the edit. Was my code broken, or you just implemented internal so it is more obvious? In any case thank you, I would just like to know if my code was bad. I know internal is customary namespace, I just used long namespace as a way to give desrciption in the name. – NoSenseEtAl Nov 20 '17 at 00:11
  • @Barry: Because the function is called `swap`. And therefore, if you call `swap` with no qualifier, then the current function is part of the overload list. The only question is how overload resolution deals with it. [Apparently, it somehow prioritizes imported names over calling the current function.](https://www.ideone.com/rRkacC) – Nicol Bolas Nov 20 '17 at 00:13
  • @NicolBolas No it's not. Because namelookup will find `internal::swap` first, and then stop. – Barry Nov 20 '17 at 00:14

3 Answers3

6

This is getting into tautology territory, but it doesn't do it that way because that's not its purpose.

The purpose of std::swap is to be the swap function of last resort. It's not supposed to be the thing you call directly, unless what you really want is to use the swap-of-last-resort.

Now, you can argue that what you suggest is a better paradigm for customization points. Hindsight is always 20/20; not everything that STL did was the right idea (see vector<bool>).

As for why we can't change that now, that's a different matter. std::swap is the swap function of last resort. So it is theoretically possible that people are calling it and expecting it to bypass any user-defined swap code. And therefore, changing it in this way breaks their code.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • 1
    "The purpose of std::swap is to be the swap function of last resort. It's not supposed to be the thing you call directly" I really did not know this. I just assumed that it is call me if you want to swap things, I never assumed it is last resort. Is this your opinion or is this something Bjarne/Stepanov said? – NoSenseEtAl Nov 20 '17 at 00:16
  • @NoSenseEtAl: That's what the function does: it performs a hard swap on the objects via triple-move. The function you want to create gives priority to user-defined swap functions, calling the original version as a *last resort* if none exist. That is therefore the purpose of `std::swap` in the `using std::swap;` idiom: being the last resort if all others don't pan out. – Nicol Bolas Nov 20 '17 at 00:45
4

Here's one problem with this approach. The ADL "two-step" relies on the function that ADL finds to be a better match than the normal one (otherwise, you'd get an overload resolution failure). This is fine most of the time, since when you write your swap() for your user-defined types in your user-defined namespaces, you're writing functions specific to those types.

But for standard library types, that may not have a more efficient swap() than the simple algorithm, this breaks down. Example:

namespace N {
    namespace internal {
        template <typename T>
        void swap(T&, T&); // normal swap impl
    }

    template <typename T>
    void swap(T& a, T& b) {
        using internal::swap;
        swap(a, b); // (*)
    }

    struct C { };
}

N::C c;
swap(c, c);    // error
N::swap(c, c); // error

Both of these calls fail, for the same reason. The unqualified call to swap() marked (*) will find N::internal::swap() through normal unqualified lookup, and then find N::swap() through ADL. There is no way to differentiate between these calls, since you very much need both calls to work for all types that meet swap's constraints. The resulting call is ambiguous.

So such a design would necessitate adding a new swap() function for every type in namespace std, even if it would just forward to std::internal::swap().

Barry
  • 286,269
  • 29
  • 621
  • 977
  • 1
    What if `N::swap` were not a function, but were instead a functor? – Nicol Bolas Nov 20 '17 at 03:53
  • Your answer raises important point I did not consider, and answers my question by exposing a bug in the code. But if you are interested to chase down a bit further: does if constexpr help here? Please see: https://godbolt.org/g/KxWvrZ – NoSenseEtAl Nov 20 '17 at 04:41
  • @NoSenseEtAl Infinite recursion? Also your trait needs to check using `T&`, not `T` – Barry Nov 20 '17 at 13:01
  • @NicolBolas That would break some code which relies on `std::swap` being a function – Barry Nov 20 '17 at 13:01
  • Tnx Barry, I have updated the code. http://coliru.stacked-crooked.com/a/c9b5d7186889bcde Seems to work ok on simple example... Regarding why the detection idiom needs & I have I assumed it would work with just values, but I guess the problem is that binding of reference to a temporary is bad :). – NoSenseEtAl Nov 21 '17 at 03:47
3

Historically, there was not much thought on name resolution, it seems. std::swap was designed as a customization point, but also wound up as the function that one would call in generic code to swap things. Hence, if std::swap didn't work, or was too slow, then one might have had to overload it, even if there already was a perfectly good swap to be found with ADL. This design cannot be changed without breaking or changing the meaning of existing code. Now there have been cases where the committee gladly decided to change the meaning of existing code for the sake of performance, such as implicit move semantics (e.g. when passing temporaries by value, or RVO where elision is not implemented). So this is not entirely consistent. Nonetheless, changing std::swap from a customization point to a name resolution wrapper with all the existing std::swap overloads in existence is dubious. This could absolutely cause catastrophic bugs to be triggered in poorly written legacy code.

Another important reason is, IMO, that you cannot move this idiom to the std namespace, while maintaining its generality. E.g.:

namespace A { struct X{}; }
namespace B {
   using std::swap;
   void swap(A::X&, A::X&);

   template<typename T>
   void reverse(T (&ts)[4])
   {
       swap(ts[0], ts[3]);
       swap(ts[1], ts[2]);
   }

   void silly(A::X (&xs)[4])
   {
       reverse(xs);
   }
}

Here, silly winds up using B::swap. The reason for this is that std::swap (via using) and B::swap are both visible with the same precedence, but the latter is a better match. Now, you may argue that this is a silly example, so here's another that is a bit less contrived:

namespace types { /*...*/ }
namespace algorithms { /*including some swap implementations for types from above...*/ }

template<typename T>
void reverse(T (&ts)[4])
{
    using std::swap;
    using algorithms::swap;
    swap(ts[0], ts[3]);
    swap(ts[1], ts[2]);
}

This will use a swap function from algorithms if it is a better match than any std::swap overload, but algorithms::swap will not be found by ADL, therefore the lookup cannot happen inside std::swap, generically. An arbitrary number of namespaces could be involved here.

Arne Vogel
  • 6,346
  • 2
  • 18
  • 31
  • 1
    "The point mentioned by Barry could be fixed simply by..." No, it couldn't. Lots of user code customizes on the name `swap`, you can't rename the customization point. – Barry Nov 20 '17 at 13:03
  • 1
    You're right, it wouldn't be able to find `swap` functions by ADL. There ought be a SFINAE way to do it, though. I'm going to reconsider the problem. – Arne Vogel Nov 20 '17 at 14:23