Why is std::swap not using swap idiom?
Asked Answered
P

3

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);
    }
}
Preface answered 19/11, 2017 at 23:49 Comment(10)
Probably would break a lot of existing code. Name clashes or change in behaviour.Toluate
Sorry, am I missing something here? What is std::internal_do_not_use_directly::swap; supposed to be?Hemline
Neil Butterworth - as the comment says in very few words :) , it has the same implementation as current std::swap;Preface
@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::swapNoreen
No, sorry, still don't understand.Hemline
@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?Bk
@NicolBolas How is either of those things possible?Asparagine
@Asparagine 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.Preface
@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.Bk
@NicolBolas No it's not. Because namelookup will find internal::swap first, and then stop.Asparagine
B
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.

Bk answered 20/11, 2017 at 0:10 Comment(2)
"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?Preface
@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.Bk
A
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().

Asparagine answered 20/11, 2017 at 2:9 Comment(5)
What if N::swap were not a function, but were instead a functor?Bk
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: godbolt.org/g/KxWvrZPreface
@Preface Infinite recursion? Also your trait needs to check using T&, not TAsparagine
@NicolBolas That would break some code which relies on std::swap being a functionAsparagine
Tnx Barry, I have updated the code. 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 :).Preface
A
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.

Assiniboine answered 20/11, 2017 at 7:34 Comment(2)
"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.Asparagine
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.Assiniboine

© 2022 - 2024 — McMap. All rights reserved.