[libcxx-commits] [PATCH] D135248: [libc++] implement move_iterator<T*> should be a random access iterator

[Nikolas Klauser via Phabricator via libcxx-commits]

philnik added inline comments.


================
Comment at: libcxx/include/__iterator/move_iterator.h:64
+   
+   static constexpr auto _Get_Iterator_Tag() {
+     if constexpr (__is_cpp17_random_access_iterator<_Iter>::value) {
----------------
phyBrackets wrote:
> huixie90 wrote:
> > philnik wrote:
> > > You can make this function `consteval`. IMO Something like `__get_iterator_concept()` would be clearer, since iterator tags are used for both `iterator_category` and `iterator_concept`, but this function is only correct for `iterator_concept`. You also have to guard the functions with `_LIBCPP_STD_VER >= 20`.
> > either add `_LIBCPP_HIDE_FROM_ABI` macro, or change the function to `consteval`
> doesn't _LIBCPP_STD_VER  > 17 means the same?
Yes, but you didn't guard the function at all. We also decided to switch to `_LIBCPP_STD_VER >= xy` in new code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135248