Skip to content

Implement pinned drop sugar#156452

Open
P8L1 wants to merge 1 commit into
rust-lang:mainfrom
P8L1:implement-pin-drop-sugar-upstream
Open

Implement pinned drop sugar#156452
P8L1 wants to merge 1 commit into
rust-lang:mainfrom
P8L1:implement-pin-drop-sugar-upstream

Conversation

@P8L1
Copy link
Copy Markdown
Contributor

@P8L1 P8L1 commented May 11, 2026

Implements fn drop(&pin mut self) as sugar for Drop::pin_drop.

The resolver recognizes the sugar only for the actual #[lang = "drop"] trait, maps accepted impl items to effective pin_drop resolution, and passes marked impl item IDs to lowering. Lowering then emits HIR with ident pin_drop, so existing type checking, Drop validation, drop glue, and direct-call checks remain unchanged.

Drop identity is handled without a hardcoded core::ops::drop::Drop path check. Local #[lang = "drop"] traits are recorded from raw AST attributes, and external Drop identity is checked through external lang-item metadata via defined_lang_items. This avoids calling the all-crates tcx.lang_items() query from resolver.

This intentionally preserves the base pinned-drop behavior where #[pin_v2] types must use pin_drop, while non-#[pin_v2] types may still implement pin_drop.

r? @petrochenkov

Related

#144537
#130494

@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. labels May 11, 2026
Comment thread tests/ui/pin-ergonomics/pinned-drop-sugar-no-core.rs Outdated
Comment thread compiler/rustc_ast/src/ast.rs
Comment thread compiler/rustc_resolve/src/late.rs
Comment thread compiler/rustc_resolve/src/late.rs
Comment thread compiler/rustc_resolve/src/late.rs
Comment thread compiler/rustc_ast_lowering/src/item.rs
Comment thread compiler/rustc_ast_lowering/src/item.rs
Comment thread compiler/rustc_ast_lowering/src/item.rs
@petrochenkov petrochenkov 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 May 12, 2026
@rust-log-analyzer

This comment has been minimized.

@rust-bors

This comment has been minimized.

@P8L1
Copy link
Copy Markdown
Contributor Author

P8L1 commented May 13, 2026

Addressed the review feedback and pushed a follow-up CI fix.

Summary of the review-response changes:

  • Removed the simple Param::is_pinned_mut_self_receiver helper and inlined the check at its single use site.
  • Removed the early local_lang_drop_traits state entirely, so resolution no longer depends on seeing a local #[lang = "drop"] trait before the impl in source order.
  • The resolver now detects fn drop(&pin mut self) syntactically in trait impls and performs the trait-item lookup using pin_drop.
  • The accepted sugar is validated after resolution against the actual lang Drop trait, so custom traits with drop or pin_drop do not accidentally accept the sugar.
  • Kept the HIR ident rewrite only after validation because associated item collection still derives the impl item name from the HIR ident; accepted sugar needs to be collected as pin_drop, not drop.
  • Kept the no-core test custom instead of using add-minicore, because minicore already provides #[lang = "drop"] and does not provide the needed Drop::pin_drop setup for this regression.
  • Added/updated regression coverage for the source-order case and custom-trait rejection.

I also fixed the latest CI failure in tests/ui/hygiene/assoc_item_ctxt.rs. The issue was that my change narrowed the general E0407 missing-trait-item suggestion path too much, which broke the intentional same-name suggestion case used by the macro hygiene test. I restored the normal E0407 candidate lookup path, so ordinary diagnostics keep the existing multipart span/suggestion behavior, while invalid fn drop(&pin mut self) sugar on custom traits is still rejected with the targeted diagnostic.

I also checked the tcx.lang_items() use in AST lowering. It is phase-safe here: lower_to_hir explicitly ensures get_lang_items(()) before stealing resolver_for_lowering, and get_lang_items collects local lang items from AST/resolver state rather than from HIR. So this access does not recursively force hir_crate, hir_owner, or HIR map construction.

@P8L1 P8L1 force-pushed the implement-pin-drop-sugar-upstream branch from 9c61b5e to 9913270 Compare May 13, 2026 11:43
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 13, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job pr-check-2 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
    Checking rustc_public v0.1.0-preview (/checkout/compiler/rustc_public)
error[E0658]: box pattern syntax is experimental
    --> compiler/rustc_resolve/src/late.rs:3571:31
     |
3571 |         let AssocItemKind::Fn(box Fn { sig, .. }) = kind else {
     |                               ^^^^^^^^^^^^^^^^^^
     |
     = note: see issue #29641 <https://github.com/rust-lang/rust/issues/29641> for more information
     = help: add `#![feature(box_patterns)]` to the crate attributes to enable
     = note: this compiler was built on 2026-04-13; consider upgrading it if it is out of date

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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants