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

Add RingBuf::as_slices as per collections reform v2. #19903

Merged
merged 1 commit into from
Dec 20, 2014

Conversation

cgaebel
Copy link
Contributor

@cgaebel cgaebel commented Dec 16, 2014

See: rust-lang/rfcs#509

Not sure if this is allowed to land before the RFC. Either way,
it's here for review.

r? @gankro
cc: @bfops

@@ -76,7 +76,13 @@ impl<T> Default for RingBuf<T> {
impl<T> RingBuf<T> {
/// Turn ptr into a slice
#[inline]
unsafe fn buffer_as_slice(&self) -> &[T] {
unsafe fn buffer_as_slice<'a>(&'a self) -> &'a [T] {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why add the lifetimes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's implemented with transmute with no lifetime annotations. It seems safer this way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Mmm... sweet, sweet elision paranoia. :)

@alexcrichton
Copy link
Member

Let's please hold off merging this until rust-lang/rfcs#509 is merged as well (but review is fine!)

@cgaebel
Copy link
Contributor Author

cgaebel commented Dec 18, 2014

RFC 509 has landed. Let's merge this sucker!

let expected_right: Vec<_> = range(cap+1, cap+2).collect();
assert_eq!(left, expected_left);
assert_eq!(right, expected_right);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually, it occurs to me: can't you make this test capacity agnostic by just doing push_back()x2, push_front()x2? Or are you testing some particular edge cases?

@Gankra
Copy link
Contributor

Gankra commented Dec 19, 2014

This LGTM now. r=me whether you change the test or not; your call on which way you prefer.

See: rust-lang/rfcs#509

Not sure if this is allowed to land before the RFC. Either way,
it's here for review.

r? @gankro
cc: @bfops
@cgaebel
Copy link
Contributor Author

cgaebel commented Dec 19, 2014

bors added a commit that referenced this pull request Dec 20, 2014
See: rust-lang/rfcs#509

Not sure if this is allowed to land before the RFC. Either way,
it's here for review.

r? @gankro
cc: @bfops
@bors bors merged commit 525f65e into rust-lang:master Dec 20, 2014
@bfops bfops mentioned this pull request Dec 24, 2014
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.

4 participants