Skip to content

BTreeMap: fix pointer provenance rules in underfullness#78631

Merged
bors merged 1 commit into
rust-lang:masterfrom
ssomers:btree-alias_for_underfull
Nov 16, 2020
Merged

BTreeMap: fix pointer provenance rules in underfullness#78631
bors merged 1 commit into
rust-lang:masterfrom
ssomers:btree-alias_for_underfull

Conversation

@ssomers

@ssomers ssomers commented Nov 1, 2020

Copy link
Copy Markdown
Contributor

Continuing on #78480, and for readability, and possibly for performance: avoid aliasing when handling underfull nodes, and consolidate the code doing that. In particular:

  • Avoid the rather explicit aliasing for internal nodes in remove_kv_tracking.
  • Climb down to the root to handle underfull nodes using a reborrowed handle, rather than one copied with ptr::read, before resuming on the leaf level.
  • Integrate the code tracking leaf edge position into the functions performing changes, rather than bolting it on.

r? @Mark-Simulacrum

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 1, 2020
@Mark-Simulacrum

Copy link
Copy Markdown
Member

r=me on the last commit (I did not review the first two, but I believe they have been previously reviewed, so that shouldn't be a concern). Ping me when those are rebased away after the previous PRs land, please.

@Mark-Simulacrum Mark-Simulacrum added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 8, 2020
@bors

bors commented Nov 9, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #78889) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@ssomers

ssomers commented Nov 9, 2020

Copy link
Copy Markdown
Contributor Author

I tweaked the name and comments of choose_balancing_contextchoose_parent_kv

@ssomers

ssomers commented Nov 10, 2020

Copy link
Copy Markdown
Contributor Author

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

Also, github doesn't seem to process commits.

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 10, 2020
@Mark-Simulacrum

Copy link
Copy Markdown
Member

First commit indeed seems to be on master, not sure why GitHub is showing it. Thanks!

@bors r+

@bors

bors commented Nov 10, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit b6c26c4 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 Nov 10, 2020
@ssomers

ssomers commented Nov 11, 2020

Copy link
Copy Markdown
Contributor Author

The commit you approved is the one GitHub showed me too, but I had (tried to) push a rebase on the newest master. Now GitHub sees that rebase commit, but somehow it's still targeting the old master. While the branch of this PR looks perfectly normal in GitHub itself: one commit on a recent master.

I doubt bors will attempt to take off in this state, so trying to wake up GitHub by pushing an even fresher rebase.

@ssomers

ssomers commented Nov 11, 2020

Copy link
Copy Markdown
Contributor Author

Aha, just one commit instead of 35 now.

@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors r+

@bors

bors commented Nov 11, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 5a129ac35cd7bdb64f762db4eeade48223ebf331 has been approved by Mark-Simulacrum

@bors

bors commented Nov 12, 2020

Copy link
Copy Markdown
Collaborator

☔ The latest upstream changes (presumably #78956) made this pull request unmergeable. Please resolve the merge conflicts.

Note that reviewers usually do not review pull requests until merge conflicts are resolved! Once you resolve the conflicts, you should change the labels applied by bors to indicate that your PR is ready for review. Post this as a comment to change the labels:

@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 12, 2020
@ssomers

ssomers commented Nov 12, 2020

Copy link
Copy Markdown
Contributor Author

Oh right, fix_right_edge moved to its own file.
@rustbot modify labels: +S-waiting-on-review -S-waiting-on-author

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 12, 2020
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors r+ rollup=never

@bors

bors commented Nov 14, 2020

Copy link
Copy Markdown
Collaborator

📌 Commit 4cfa5bd 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 Nov 14, 2020
@bors

bors commented Nov 15, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4cfa5bd with merge 3e5a681469d8559e2bb5f9f384487c960db92d72...

@bors

bors commented Nov 15, 2020

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 15, 2020
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors retry

@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 Nov 15, 2020
@bors

bors commented Nov 15, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4cfa5bd with merge 1cf9258ce60a03dbbb4589d2df424dc1e11db52c...

@bors

bors commented Nov 15, 2020

Copy link
Copy Markdown
Collaborator

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 15, 2020
@Mark-Simulacrum

Copy link
Copy Markdown
Member

@bors retry

@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 Nov 15, 2020
@bors

bors commented Nov 16, 2020

Copy link
Copy Markdown
Collaborator

⌛ Testing commit 4cfa5bd with merge f5230fb...

@bors

bors commented Nov 16, 2020

Copy link
Copy Markdown
Collaborator

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

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Nov 16, 2020
@bors bors merged commit f5230fb into rust-lang:master Nov 16, 2020
@rustbot rustbot added this to the 1.50.0 milestone Nov 16, 2020
@ssomers ssomers deleted the btree-alias_for_underfull branch November 16, 2020 08:09
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants