Skip to content

Emit either mono item of Drop::drop or Drop::pin_drop depending on which was implemented#156495

Open
frank-king wants to merge 1 commit into
rust-lang:mainfrom
frank-king:feature/pin-drop
Open

Emit either mono item of Drop::drop or Drop::pin_drop depending on which was implemented#156495
frank-king wants to merge 1 commit into
rust-lang:mainfrom
frank-king:feature/pin-drop

Conversation

@frank-king
Copy link
Copy Markdown
Contributor

@frank-king frank-king commented May 12, 2026

This PR makes drop glues of types that implement Drop::pin_drop call Drop::pin_drop directly instead of Drop::drop, and thus avoids both mono items of Drop::drop and Drop::pin_drop being emitted.

Part of #130494 (Pin Ergonomics).

r? @bjorn3

@rustbot rustbot added 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. labels May 12, 2026
if let Some(trait_id) = tcx.trait_of_assoc(def_id)
&& tcx.is_lang_item(trait_id, LangItem::Drop)
{
return false;
Copy link
Copy Markdown
Contributor Author

@frank-king frank-king May 12, 2026

Choose a reason for hiding this comment

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

Like the #[rustc_force_inline] case above, I expect Drop::drop and Drop::pin_drop should never be codegened, so I tried emitting a delayed bug, but all the codegen-units tests related to drop glue failed.

I've also tried to add #[rustc_force_inline] to Drop::drop (with trait method attribute target allowed), but core could not even be built where the debug_assert_matches failed at ForceInliner::check_codegen_attributes_extra.

View changes since the review

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Either Drop::drop or Drop::pin_drop would need to be codegened for the drop glue to call, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's true, but the root item Drop::drop or Drop::pin_drop has been collected by collect_items_root, which is not guarded by should_codegen_locally, right?

@frank-king frank-king marked this pull request as ready for review May 12, 2026 09:47
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 12, 2026

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

@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 May 12, 2026
Copy link
Copy Markdown
Member

@bjorn3 bjorn3 May 12, 2026

Choose a reason for hiding this comment

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

The change in this file looks correct to me.

View changes since the review

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 16, 2026

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

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

Labels

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants