[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:1400-1402
+        size_type __n = __string_is_trivial_iterator<ranges::iterator_t<_Range>>::value ?
+            static_cast<size_type>(ranges::distance(__range)) : 0;
+        __assign_with_size(ranges::begin(__range), ranges::end(__range), __n);
----------------
ldionne wrote:
> We should refactor this so that `__assign_with_size` actually assigns with size, so we don't have to compute `__n = 0` when we don't actually need it anywhere. That's too confusing, it seems like we're assigning with size 0 when `!__string_is_trivial_iterator` but in reality `__assign_with_size` simply ignored the size we passed in.
I gave it a shot, but very open to suggestions.

Thinking more about it, it's not really "assign with size" since it only applies to a subset of cases where we know the size:
- first, the input iterators have to be "trivial";
- second, if the input range is a subset of the string itself, then we need to have enough capacity to avoid reallocating (or the input iterators will end up being invalidated).

These conditions are somewhat messy, so to avoid duplicating the check in two different places (`assign` and `assign_range`), I made it a `try` function that returns `false` if it fails to do the job (so that the caller can fall back to `assign_with_sentinel`). This also avoids the question of whether `try_assign_trivial` should assert that its preconditions are met or just trust the caller. Please let me know what you think!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149832