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

[Louis Dionne via Phabricator via libcxx-commits]

ldionne requested changes to this revision.
ldionne added inline comments.
This revision now requires changes to proceed.


================
Comment at: libcxx/include/module.modulemap.in:1585
       module cmp                 { private header "__utility/cmp.h" }
       module convert_to_integral { private header "__utility/convert_to_integral.h" }
       module declval             { private header "__utility/declval.h" }
----------------
By the way, I think this is highly relevant to the existing issue that `std::less` is supposed to be blessed to compare unrelated pointers, but we currently don't do anything special with it. I think we should have a single solution for both of those issues.


================
Comment at: libcxx/include/string:1974
+    _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX20 bool __addr_in_range(_Tp&& __v) const {
+      return std::__is_pointer_in_range(data(), data() + size(), std::addressof(__v));
     }
----------------
I would handle the `!is-less-than-comparable` case here instead of in `__is_pointer_in_range`. `__is_pointer_in_range` is more general purpose, and it doesn't make sense for it to return `false` when trying to compare unrelated pointers -- instead it should be a compiler error to do so.

In other words, `__is_pointer_in_range` should be equivalent to `first <= p < last`, except blessed by the compiler -- nothing less, nothing more. If that expression would not be well formed, then `__is_pointer_in_range` should also not be well formed, and we shouldn't try to give it a value.


================
Comment at: libcxx/include/string:1978
-        const volatile void *__p = std::addressof(__t);
-        return data() <= __p && __p <= data() + size();
     }
----------------
The new code is not equivalent to the old one. The old code does `data() <= __p && __p <= data() + size()`, the new code looks for `data() <= __p && __p < data() + size()` (notice `<=` vs `<`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D143327