Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement a few methods for RingBuf #20197

Merged
merged 1 commit into from
Jan 6, 2015

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Dec 24, 2014

Part of collections reform part 1 and 2, #18424 and #19986

  • shrink_to_fit
  • swap_back_remove
  • swap_front_remove
  • truncate
  • resize

@rust-highfive
Copy link
Collaborator

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@pczarn
Copy link
Contributor Author

pczarn commented Dec 24, 2014

r? @gankro
@csherratt

@rust-highfive rust-highfive assigned Gankra and unassigned huonw Dec 24, 2014
@pczarn pczarn force-pushed the ring_buf-collections-reform branch from 0646db8 to 13be65b Compare December 24, 2014 11:12
/// It will drop down as close as possible to the length but the allocator may still inform the
/// vector that there is space for a few more elements.
///
/// Returns `None` if `index` is out of bounds.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function does not seem to return anything judging by its type notation.

@pczarn pczarn force-pushed the ring_buf-collections-reform branch from 13be65b to 8c3436e Compare December 24, 2014 19:46
/// # Examples
///
/// ```
/// let mut vec = vec![1i, 2, 3, 4];
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This example needs to be updated.

@Gankra
Copy link
Contributor

Gankra commented Dec 30, 2014

Done review.

@pczarn pczarn force-pushed the ring_buf-collections-reform branch from c6c6946 to e0c06b7 Compare December 31, 2014 12:55
@Gankra
Copy link
Contributor

Gankra commented Jan 2, 2015

@pczarn I see you've added some commits; is that everything or are you still working on it (basically: should I re-review now?)

@pczarn
Copy link
Contributor Author

pczarn commented Jan 2, 2015

@gankro that's everything, except for swap_.._remove tests; hope to add them now.

self.cap);
ptr::copy_nonoverlapping_memory(
self.ptr.offset(dst as int),
self.ptr.offset(src as int) as *const T,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the as *const T should just coerce for free? Still trying to grok when these coercions do or don't kick in, though.

@Gankra
Copy link
Contributor

Gankra commented Jan 2, 2015

Okay lgtm modulo nits/tests.

@pczarn pczarn force-pushed the ring_buf-collections-reform branch 3 times, most recently from 603fadd to f47dd53 Compare January 3, 2015 00:29
@pczarn pczarn force-pushed the ring_buf-collections-reform branch from f47dd53 to 00f8cdf Compare January 3, 2015 12:29
@Gankra
Copy link
Contributor

Gankra commented Jan 4, 2015

r=me with squash

@pczarn pczarn force-pushed the ring_buf-collections-reform branch 2 times, most recently from b216a09 to 841b458 Compare January 4, 2015 23:10
@pczarn
Copy link
Contributor Author

pczarn commented Jan 4, 2015

@gankro, done.

* shrink_to_fit
* swap_back_remove
* swap_front_remove
* truncate
* resize
@pczarn pczarn force-pushed the ring_buf-collections-reform branch from 841b458 to 156a1c3 Compare January 5, 2015 14:49
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Jan 6, 2015
Part of collections reform part 1 and 2, rust-lang#18424 and rust-lang#19986

* shrink_to_fit
* swap_back_remove
* swap_front_remove
* truncate
* resize
@bors bors merged commit 156a1c3 into rust-lang:master Jan 6, 2015
@pczarn pczarn deleted the ring_buf-collections-reform branch January 14, 2015 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants