Skip to content

Make dropping an empty BTreeMap free - #161791

Open
jakedrew wants to merge 2 commits into
rust-lang:mainfrom
jakedrew:fix/Making_and_dropping_an_empty_BTreeMap_is_not_free_161375
Open

Make dropping an empty BTreeMap free#161791
jakedrew wants to merge 2 commits into
rust-lang:mainfrom
jakedrew:fix/Making_and_dropping_an_empty_BTreeMap_is_not_free_161375

Conversation

@jakedrew

@jakedrew jakedrew commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

#161375 (code example) shows that drop of an empty BTree map walks the dying_next path and so is not free. I tracked this down to 7b28036 in 1.56.0 using godbolt’s version comparison.

When the Btree map has no root we use the ManuallyDrop to safely drop the allocator early resulting in a free drop and a ret instead of the dying_next path.

Includes codegen test which correctly catches the prior dying_next case, which I based on uninit-aggregate-field.rs.

Fixes #161375

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Aug 26, 2026
@rustbot

rustbot commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

r? @mati865

rustbot has assigned @mati865.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 75 candidates
  • Random selection from 17 candidates

@jakedrew jakedrew changed the title Fix/making and dropping an empty b tree map is not free 161375 Make dropping an empty BTreeMap free Aug 26, 2026
@mati865

mati865 commented Aug 26, 2026

Copy link
Copy Markdown
Member

Nice, but that requires libs review. r? rust-lang/libs

@rustbot rustbot assigned clarfonthey and unassigned mati865 Aug 26, 2026
@ruriww

ruriww commented Aug 26, 2026

Copy link
Copy Markdown

Just a side note, but can't the unsafe code be replaced with take(self)?

Hmm, maybe not https://godbolt.org/z/Wzhqh9jre.

use std::collections::BTreeMap;

// CHECK-LABEL: @drop_btree
// CHECK-NOT: dying_next

@clarfonthey clarfonthey Aug 27, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I personally would rather this check for calls to dealloc rather than dying_next, since I this test could easily become worthless if this method is renamed.

View changes since the review

@clarfonthey

Copy link
Copy Markdown
Contributor

@rustbot author

@rustbot rustbot 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 Aug 27, 2026
@rustbot

rustbot commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. 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.

Making and dropping an empty BTreeMap is not free.

5 participants