[libcxx-commits] [PATCH] D149832: [libc++][ranges] Implement the changes to `basic_string` from P1206 (`ranges::to`):

[Konstantin Varlamov via Phabricator via libcxx-commits]

var-const added inline comments.


================
Comment at: libcxx/include/string:2627
+#if _LIBCPP_STD_VER >= 17
+  if constexpr (
+#else
----------------
I'm very open to reverting this, perhaps it's overkill. By the way, do we have any existing precedent for cases where we would want to use `if constexpr` but can't due to pre-C++17 language modes? I didn't find an obvious macro to use in `__config`, but perhaps I missed something.


================
Comment at: libcxx/include/string:2656
     {
-        if (__cap < __n)
+        if (__addr_in_range(*__first))
         {
----------------
Note: this is an inversion of the original "positive" condition ("if condition, proceed" now inverted to "if not condition, return early"):
```
if (__cap >= __n || !__addr_in_range(*__first)))
```
Unless I made a mistake and the rewrite is not equivalent, I don't really see how this case could happen. If `__first` is `__addr_in_range`, i.e., an iterator pointing at a character within the string itself, it seems that the input range cannot be longer than the size of the string, which cannot be less than the capacity. I don't think it's possible for the end iterator to point to some characters past the end of the string without UB. Perhaps I'm missing something? FWIW, no tests fail if I remove this early return.

(If we could remove this condition, it would simplify the function so that it no longer has to return a boolean)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149832