[libcxx-commits] [PATCH] D142335: [libc++][ranges] Implement `ranges::to`.

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/include/vector:1478-1479
         {
-            __growing = true;
-            __mid =  __first;
-            std::advance(__mid, size());
+            std::advance(__first, size());
+            __construct_at_end(__first, __last, __new_size - size());
+
----------------
var-const wrote:
> ldionne wrote:
> > This doesn't seem to work, I think we're consuming the first `size()` elements from `__first` and then copying the rest into the vector only. I think this also means there's a gap in the testing.
> Thanks! I think this is the right form:
> ```
> if (__new_size <= capacity()) {
>   if (__new_size > size()) {
>     _ForwardIterator __mid = __first;
>     std::advance(__mid, size());
> 
>     std::copy(__first, __mid, this->__begin_);
>     __construct_at_end(__mid, __last, __new_size - size());
> 
>   } else {
>     pointer __m = std::copy(__first, __last, this->__begin_);
>     this->__destruct_at_end(__m);
>   }
> }
> ```
> I think it's actually more readable without the `__growing` variable.
```
if (__new_size <= capacity()) {
  if (__new_size > size()) {
    _ForwardIterator __mid = std::next(__first, size());

    std::copy(__first, __mid, this->__begin_);
    __construct_at_end(__mid, __last, __new_size - size());

  } else {
    pointer __m = std::copy(__first, __last, this->__begin_);
    this->__destruct_at_end(__m);
  }
}
```

Yeah, I think that works!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335