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

BTreeMap: wrap node's raw parent pointer in NonNull #76929

Merged
merged 1 commit into from
Sep 19, 2020

Conversation

ssomers
Copy link
Contributor

@ssomers ssomers commented Sep 19, 2020

Now that the other *const (root) is gone, seemed like a small step forward.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 19, 2020
let parent_as_leaf = unsafe { (*self.as_leaf_ptr()).parent as *const LeafNode<K, V> };
if let Some(non_zero) = NonNull::new(parent_as_leaf as *mut _) {
let leaf_ptr = self.as_leaf_ptr();
if let &Some(non_zero) = &unsafe { (*leaf_ptr).parent } {
Copy link
Member

Choose a reason for hiding this comment

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

Hm, this looks a bit odd -- maybe we can drop the reference on both sides (in the pattern and before unsafe)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But then it's moved out of the option. Before this I had:
if let Some(non_zero) = unsafe { (*leaf_ptr).parent.as_ref() } {

Copy link
Member

Choose a reason for hiding this comment

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

I think I prefer the latter form, it looks clearer to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about this fancy one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also straigthened out the conversions to usize.

@jyn514 jyn514 added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label Sep 19, 2020
@Mark-Simulacrum
Copy link
Member

@bors r+ rollup=never

@bors
Copy link
Contributor

bors commented Sep 19, 2020

📌 Commit 0661b0a has been approved by Mark-Simulacrum

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 19, 2020
@bors
Copy link
Contributor

bors commented Sep 19, 2020

⌛ Testing commit 0661b0a with merge f68e089...

@bors
Copy link
Contributor

bors commented Sep 19, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: Mark-Simulacrum
Pushing f68e089 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 19, 2020
@bors bors merged commit f68e089 into rust-lang:master Sep 19, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 19, 2020
@ssomers ssomers deleted the btree_cleanup_2 branch September 21, 2020 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants