Skip to content

Replace LinkedList's use of Box with Shared #34608

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Merged
merged 1 commit into from
Jul 13, 2016
Merged

Replace LinkedList's use of Box with Shared #34608

merged 1 commit into from
Jul 13, 2016

Conversation

apasel422
Copy link
Contributor

Closes #34417

@rust-highfive
Copy link
Contributor

r? @alexcrichton

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

@alexcrichton
Copy link
Member

Awesome work @apasel422, looks great to me!

Just to make sure we've got at least another pair of eyes on this though, r? @bluss

@rust-highfive rust-highfive assigned bluss and unassigned alexcrichton Jul 2, 2016
@apasel422
Copy link
Contributor Author

Ping.

bluss
bluss reviewed Jul 13, 2016
View reviewed changes
src/libcollections/linked_list.rs
head: Option<Shared<Node<T>>>,
tail: Option<Shared<Node<T>>>,
len: usize,
marker: PhantomData<Box<Node<T>>>,
Copy link
Member

Choose a reason for hiding this comment

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

Not strictly needed since Shared<X> contains PhantomData<X> and Box<X> and X have the same implications.

@bluss
Copy link
Member

bluss commented Jul 13, 2016

This looks good to me too. LinkedList gets into better shape with this change.

@bluss
Copy link
Member

bluss commented Jul 13, 2016

@bors r+

@bors
Copy link
Collaborator

bors commented Jul 13, 2016

📌 Commit 6d3bf6e has been approved by bluss

@bors
Copy link
Collaborator

bors commented Jul 13, 2016

⌛ Testing commit 6d3bf6e with merge 362b665...

bors added a commit that referenced this pull request Jul 13, 2016
@bors
Auto merge of #34608 - apasel422:ll, r=bluss
362b665 
Replace `LinkedList`'s use of `Box` with `Shared`

Closes #34417
@bors
Copy link
Collaborator

bors commented Jul 13, 2016

☀️ Test successful - auto-linux-32-nopt-t, auto-linux-32-opt, auto-linux-64-cargotest, auto-linux-64-cross-freebsd, auto-linux-64-debug-opt, auto-linux-64-nopt-t, auto-linux-64-opt, auto-linux-64-opt-mir, auto-linux-64-opt-rustbuild, auto-linux-64-x-android-t, auto-linux-cross-opt, auto-linux-musl-64-opt, auto-mac-32-opt, auto-mac-64-opt, auto-mac-64-opt-rustbuild, auto-mac-cross-ios-opt, auto-win-gnu-32-opt, auto-win-gnu-32-opt-rustbuild, auto-win-gnu-64-opt, auto-win-msvc-32-opt, auto-win-msvc-64-cargotest, auto-win-msvc-64-opt, auto-win-msvc-64-opt-mir, auto-win-msvc-64-opt-rustbuild
Approved by: bluss
Pushing 362b665 to master...

@bors bors merged commit 6d3bf6e into rust-lang:master Jul 13, 2016
@apasel422 apasel422 deleted the ll branch July 13, 2016 14:28
@apasel422 apasel422 mentioned this pull request Aug 16, 2016
Closed
@jrmuizel jrmuizel mentioned this pull request Apr 20, 2019
Open
@F001 F001 mentioned this pull request Apr 29, 2019
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers
No reviews
Assignees

@bluss bluss

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@apasel422 @rust-highfive @alexcrichton @bluss @bors