[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:2656
     {
-        if (__cap < __n)
+        if (__addr_in_range(*__first))
         {
----------------
var-const wrote:
> 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?
Synced offline, it's basically:
```
  std::string str("abc");
  auto* ptr = reinterpret_cast<char*>(&str);
  str.assign(ptr, ptr + sizeof(str));
```
IIUC, there's no undefined behavior here -- growing capacity will change the byte representation of the string but won't make it invalid. I think we don't need to look for that case (and we already don't in the overload of `assign` that takes a pointer and a size).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D149832