[libcxx-commits] [PATCH] D92776: [libc++] ADL-proof <algorithm> by adding _VSTD:: qualification on calls.

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

Quuxplusone added a subscriber: EricWF.
Quuxplusone added inline comments.


================
Comment at: libcxx/include/algorithm:1739
         return _VSTD::__copy(
-            __unwrap_iter(__first), __unwrap_iter(__last), __unwrap_iter(__result));
+            _VSTD::__unwrap_iter(__first), _VSTD::__unwrap_iter(__last), _VSTD::__unwrap_iter(__result));
     }
----------------
rnk wrote:
> This broke Chrome, apparently we were using this ADL extension point to enable memmove optimization for some fancy iterators:
> https://source.chromium.org/chromium/chromium/src/+/master:base/containers/checked_iterators.h;l=88?q=__unwrap_iter&ss=chromium
> 
> This std::copy optimization was added in April, maybe it's not that important:
> https://chromium-review.googlesource.com/c/chromium/src/+/1875734
> 
> Can you suggest an alternative solution to make std::copy use memmove for some custom iterator?
Oh, yuck. @ldionne @ericwf might want to take a look.
As the discussion on [that pull request](https://chromium-review.googlesource.com/c/chromium/src/+/1875734) indicated, C++20 provides a //standard// way to make a contiguous iterator that the library will recognize; however, there are two problems with that approach right now. (1) it's not implemented, and (2) it's not the best-designed, for example it requires you to overload `std::to_address(CCI)` (//not// as an ADL customization point; actually in `std`!).
I think it would be useful to users if libc++ provided a simple way to mark an iterator as contiguous, even in C++11/14/17. I suggest:
- libc++ should define the tag type `std::contiguous_iterator_tag` in all language modes, as an extension.
- libc++ should treat as contiguous, any iterator for which `std::is_base_of_v<std::contiguous_iterator_tag, std::iterator_traits<It>::iterator_category>`.
- To convert a contiguous iterator `it` to a pointer, libc++ should call `std::to_address(it)` just the way it would in C++20(??).

I've opened https://stackoverflow.com/questions/65712091/in-c20-how-do-i-write-a-contiguous-iterator to find out more about how users such as Chromium are supposed to do this kind of stuff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92776