[libcxx-commits] [PATCH] D143327: [libc++] Introduce __is_pointer_in_range

[Louis Dionne via Phabricator via libcxx-commits]

ldionne accepted this revision.
ldionne added a comment.
This revision is now accepted and ready to land.

LGTM, but this needs to be rebased on the patch with the `__less` refactoring before going in.



================
Comment at: libcxx/include/__utility/is_pointer_in_range.h:41-42
+
+  // Checking this for unrelated pointers is technically UB, but no compiler optimizes based on it (currently).
+  return __begin <= __ptr && __ptr < __end;
+}
----------------
That way, we technically don't have to change anything but `std::less` in the future.

We could also use `__less` instead here, and make `__less` be the canonical place where we bless comparison of unrelated pointers. We would then also use `__less` from `std::less`. Let's do that part as a separate patch, it's actually a nice cleanup.


================
Comment at: libcxx/include/string:1983
+        __enable_if_t<__is_less_than_comparable<const __remove_cvref_t<_Tp>*, const value_type*>::value, int> = 0>
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __addr_in_range(_Tp&& __v) const {
+      return std::__is_pointer_in_range(data(), data() + size() + 1, std::addressof(__v));
----------------
Here and below.


================
Comment at: libcxx/include/string:1996
+      return reinterpret_cast<const char*>(data()) <= __t_ptr &&
+             __t_ptr <= reinterpret_cast<const char*>(data() + size());
     }
----------------
For symmetry with above?


================
Comment at: libcxx/test/libcxx/utilities/is_pointer_in_range.pass.cpp:34
+
+#if TEST_STD_VER >= 20
+  {
----------------
Could we move this test to `test_cv_quals`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143327