Skip to content

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

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!

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