[libcxx-commits] [PATCH] D92250: [libc++] Consistently replace `std::` qualification with `_VSTD::` or nothing. NFCI.

[Arthur O'Dwyer via Phabricator via libcxx-commits]

Quuxplusone marked an inline comment as done.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/memory:2086
   {
-    using std::swap;
+    using _VSTD::swap;
     swap(first(), __x.first());
----------------
mclow.lists wrote:
> This one may not be correct. 
>  If a user has implemented an overload for `std::swap` for `T1` or `T2`, then it's not clear to me that this wil find it.
Hmm, that's quite true, this won't find a user-defined `std::swap` inserted manually into the outer `std` namespace. https://godbolt.org/z/vxKEcq
**However...** my change here is just bringing this call-site into line with the 20 other places in libc++ where we do the std::swap two-step. All 20 of them (and now, also this 21st) do `using _VSTD::swap;`. So if this one is now wrong, those other 20 are also wrong, and have been wrong for years.

Notably, `std::pair`'s `swap` is implemented in terms of `using _VSTD::swap;` and not in terms of `using std::swap;`.

I think the correct response here is "Users shouldn't be adding their own functions into namespace std. They should be writing ADL swap functions instead."


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92250/new/

https://reviews.llvm.org/D92250