[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:2647-2651
+#if _LIBCPP_STD_VER >= 17
+    static_assert(__string_is_trivial_iterator<_Iterator>::value);
+#else
+    _LIBCPP_ASSERT(__string_is_trivial_iterator<_Iterator>::value);
+#endif
----------------
philnik wrote:
> Why do you have this condition here? We have `static_assert`s all the way back to C++03.
I think if I'm not using `if constexpr` in the caller, this function will always end up being instantiated, whether it's called or not, so I can't use `static_assert`. I reverted the conditional `if constexpr`, so this is no longer applicable.


================
Comment at: libcxx/include/string:2656
     {
-        if (__cap < __n)
+        if (__addr_in_range(*__first))
         {
----------------
philnik wrote:
> ldionne wrote:
> > var-const wrote:
> > > 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)
> > I think I agree with this, @philnik does that make sense to you (you are pretty familiar with string).
> Users are allowed to inspect the object, so this will be the case if a user gives us a pointer to the value representation of this string. The question is just whether we want to bother supporting this, since about two people on this planet will actually do that.
@philnik I don't fully understand the case you're describing, can you please give a code example?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149832