[libcxx-commits] [PATCH] D94807: [libc++] Rationalize our treatment of contiguous iterators and __unwrap_iter().

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/iterator:444
+// C++20 to allow optimizations for users providing wrapped iterator types.
+struct _LIBCPP_TEMPLATE_VIS contiguous_iterator_tag : public random_access_iterator_tag {};
 
----------------
Quuxplusone wrote:
> Quuxplusone wrote:
> > @ldionne wrote:
> > > Arthur, some of this stuff landed in C++20, so we can still do it, but in C++20 only. Are you OK with that?
> > 
> > Yes, I believe so, unless I run into a problem with that.
> > The important subtlety here remains, even if we ignore Chromium. The way I've got the code structured now, //in both C++03 and C++20//,
> > - The std::copy-to-memmove optimization happens only for iterators where `__unwrap_iter(it)` returns a native pointer `T*`.
> > - `__unwrap_iter(it)` returns native pointers only for contiguous iterators, i.e., `__is_cpp17_contiguous_iterator<It>::value` is true and `_VSTD::__to_address(it)` exists.
> > - We want the std::copy-to-memmove optimization to happen for `vector<int>::iterator` i.e. `__wrap_iter<int*>`.
> > 
> > So //even in C++03 mode//, where `std::contiguous_iterator_tag` does not exist, we must ensure that `__is_cpp17_contiguous_iterator<int*>::value && __is_cpp17_contiguous_iterator<__wrap_iter<int*>>::value`.
> > 
> > I think that's a good state of affairs, and I think I can physically make it happen. But if I come back and say "oops, actually that's physically impossible because...," then I'll ask to revisit this.
> @ldionne: It would be almost trivial, if there were a way to expose `contiguous_iterator_tag` //only to the implementation//, e.g.
> 
> ```
> struct __libcpp_contiguous_iterator_tag : random_access_iterator_tag {};
> #if _LIBCPP_STD_VER > 17
> using contiguous_iterator_tag = __libcpp_contiguous_iterator_tag;
> #endif
> ```
> 
> Is that snippet acceptable? and/or have you got another acceptable idea?
> If not, then it's still possible to achieve our goal, but with an increased number of codepaths that differ when `#if _LIBCPP_STD_VER > 17`.
Yes, it's acceptable IMO.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94807