[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:
> > 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!
> Note: still need to fix the testing part.
I am moving this comment thread to D149826, feel free to mark as resolved.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142335