Skip to content

Add Drop::pin_drop for pinned drops#144537

Open
frank-king wants to merge 2 commits intorust-lang:mainfrom
frank-king:feature/pin-drop
Open

Add Drop::pin_drop for pinned drops#144537
frank-king wants to merge 2 commits intorust-lang:mainfrom
frank-king:feature/pin-drop

Conversation

@frank-king
Copy link
Copy Markdown
Contributor

@frank-king frank-king commented Jul 27, 2025

View all comments

This PR is part of the pin_ergonomics experiment (the tracking issue is #130494). It allows implementing Drop with a pinned self receiver, which is required for safe pin-projection.

Implementations:

  • At least and at most one of drop and pin_drop should be implemented.
  • No direct call of drop or pin_drop. They should only be called by the drop glue.
  • pin_drop must and must only be used with types that support pin-projection (i.e. types with #[pin_v2]).
  • Allows writing fn drop(&pin mut self) and desugars to fn pin_drop(&pin mut self). (Will be in the next PRs)

@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. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 27, 2025
@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

@frank-king frank-king changed the title Implement Drop::pin_drop for !Unpin types Add Drop::pin_drop for pinned drops Sep 20, 2025
@rust-log-analyzer

This comment has been minimized.

@frank-king frank-king force-pushed the feature/pin-drop branch 2 times, most recently from d6ddfcf to 7b4bb5c Compare September 20, 2025 14:42
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@frank-king
Copy link
Copy Markdown
Contributor Author

The CI failed because Drop::drop becomes a provided method. I'm afraid it might not be a good way to hack into librustdoc. Is there any other good way to fix it?

@rust-log-analyzer

This comment has been minimized.

@bors

This comment was marked as resolved.

@Dylan-DPC Dylan-DPC added S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Oct 7, 2025
@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@frank-king frank-king force-pushed the feature/pin-drop branch 3 times, most recently from 9618d10 to c3e34d5 Compare November 7, 2025 12:45
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 4, 2026

Some changes occurred in compiler/rustc_passes/src/check_attr.rs

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_attr_parsing

cc @jdonszelmann, @JonathanBrouwer

Some changes occurred in compiler/rustc_hir/src/attrs

cc @jdonszelmann, @JonathanBrouwer

@rustbot rustbot added the A-attributes Area: Attributes (`#[…]`, `#![…]`) label May 4, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented May 4, 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.

@P8L1
Copy link
Copy Markdown
Contributor

P8L1 commented May 4, 2026

Hi @frank-king, from what I can see the latest CI is green now, including the previous lint/linkchecker-related issue. Could you please confirm that this part is fully resolved on your side as well?

I also plan to work on the remaining unchecked item in the PR description:

Allows writing fn drop(&pin mut self) and desugars to fn pin_drop(&pin mut self). (Will be in the next PRs)

I’ll prepare that as a follow-up PR so this Drop::pin_drop work can keep moving forward.

@frank-king
Copy link
Copy Markdown
Contributor Author

@rustbot ready

@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 4, 2026
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 4, 2026

📌 Commit fa600c5 has been approved by petrochenkov

It is now in the queue for this repository.

@rust-bors rust-bors Bot 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 May 4, 2026
@rust-bors

This comment has been minimized.

rust-bors Bot pushed a commit that referenced this pull request May 4, 2026
Add `Drop::pin_drop` for pinned drops





This PR is part of the `pin_ergonomics` experiment (the tracking issue is #130494). It allows implementing `Drop` with a pinned `self` receiver, which is required for safe pin-projection.

Implementations:
- [x] At least and at most one of `drop` and `pin_drop` should be implemented.
- [x] No direct call of `drop` or `pin_drop`. They should only be called by the drop glue.
- [x] `pin_drop` must and must only be used with types that support pin-projection (i.e. types with `#[pin_v2]`).
- [ ] Allows writing `fn drop(&pin mut self)` and desugars to `fn pin_drop(&pin mut self)`. (Will be in the next PRs)
@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job x86_64-gnu-llvm-21-1 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
test expr_store::tests::body::async_fn_weird_param_patterns ... ok
test expr_store::scope::tests::test_shadow_variable ... ok
test expr_store::scope::tests::while_let_adds_binding ... ok
test expr_store::tests::body::block::inner_item_smoke ... ok
error: test failed, to rerun pass `-p hir-def --lib`

Caused by:
  process didn't exit successfully: `/checkout/obj/build/x86_64-unknown-linux-gnu/stage2-tools/x86_64-unknown-linux-gnu/release/deps/hir_def-c439368f2b64a049 '--skip=tests::smoke_test_real_sysroot_cargo' --skip=check_code_formatting -Z unstable-options --format json` (signal: 11, SIGSEGV: invalid memory reference)
Bootstrap failed while executing `--stage 2 test --skip tests --skip coverage-map --skip coverage-run --skip library --skip tidyselftest`
Build completed unsuccessfully in 1:11:02
  local time: Mon May  4 18:35:38 UTC 2026
  network time: Mon, 04 May 2026 18:35:39 GMT
##[error]Process completed with exit code 1.

@rust-bors rust-bors Bot 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 May 4, 2026
@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 4, 2026

💔 Test for 13a5a99 failed: CI. Failed job:

@JonathanBrouwer
Copy link
Copy Markdown
Contributor

@bors retry

@rust-bors rust-bors Bot 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 May 4, 2026
@P8L1
Copy link
Copy Markdown
Contributor

P8L1 commented May 4, 2026

@frank-king I opened a stacked follow-up PR for the remaining sugar item:

frank-king#9

It implements fn drop(&pin mut self) as sugar for Drop::pin_drop on top of feature/pin-drop.

The implementation keeps the remap at resolver/HIR, lowering time: the resolver recognises the sugar only for the actual #[lang = "drop"] trait, resolves it as pin_drop, and lowering emits HIR with ident pin_drop. It leaves the existing Drop validation, drop glue, and direct-call checks unchanged.

I also added coverage for hosted Drop, qualified std::ops::Drop / core::ops::Drop, local no-core #[lang = "drop"], non-Drop traits named Drop, non-Drop traits with drop / pin_drop, exact receiver rejection cases, and feature-gate behaviour.

Happy to adjust the approach if you would prefer the sugar to be handled at a different compiler boundary.

@rust-bors

This comment has been minimized.

@rust-log-analyzer
Copy link
Copy Markdown
Collaborator

The job x86_64-gnu-llvm-21-3 failed! Check out the build log: (web) (plain enhanced) (plain)

Click to see the possible cause of the failure (guessed by this bot)
##[endgroup]
[TIMING:end] tool::ToolBuild { build_compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu, tool: "linkchecker", path: "src/tools/linkchecker", mode: ToolBootstrap, source_type: InTree, extra_features: [], allow_features: "", cargo_args: [], artifact_kind: Binary } -- 0.160
[TIMING:end] tool::Linkchecker { compiler: Compiler { stage: 0, host: x86_64-unknown-linux-gnu, forced_compiler: false }, target: x86_64-unknown-linux-gnu } -- 0.000
##[group]Testing stage1 Linkcheck (x86_64-unknown-linux-gnu)
reference/print.html:44444: broken link fragment `#tymethod.drop` pointing to `core/ops/drop/trait.Drop.html`
reference/destructors.html:197: broken link fragment `#tymethod.drop` pointing to `core/ops/drop/trait.Drop.html`
checked links in: 31.6s
number of HTML files scanned: 60134
number of HTML redirects found: 21239
number of links checked: 2876699
number of links ignored due to external: 150637
---

--- BACKTRACE vvv
   0: <bootstrap::utils::exec::DeferredCommand>::finish_process
             at /checkout/src/bootstrap/src/utils/exec.rs:939:17
   1: <bootstrap::utils::exec::DeferredCommand>::wait_for_output::<&bootstrap::utils::exec::ExecutionContext>
             at /checkout/src/bootstrap/src/utils/exec.rs:831:21
   2: <bootstrap::utils::exec::ExecutionContext>::run
             at /checkout/src/bootstrap/src/utils/exec.rs:741:45
   3: <bootstrap::utils::exec::BootstrapCommand>::run::<&bootstrap::core::builder::Builder>
             at /checkout/src/bootstrap/src/utils/exec.rs:339:27
   4: <bootstrap::core::build_steps::test::Linkcheck as bootstrap::core::builder::Step>::run
             at /checkout/src/bootstrap/src/core/build_steps/test.rs:191:77
   5: <bootstrap::core::builder::Builder>::ensure::<bootstrap::core::build_steps::test::Linkcheck>
             at /checkout/src/bootstrap/src/core/builder/mod.rs:1595:36
   6: <bootstrap::core::build_steps::test::Linkcheck as bootstrap::core::builder::Step>::make_run
             at /checkout/src/bootstrap/src/core/build_steps/test.rs:137:21
   7: <bootstrap::core::builder::StepDescription>::maybe_run
             at /checkout/src/bootstrap/src/core/builder/mod.rs:476:13
   8: bootstrap::core::builder::cli_paths::match_paths_to_steps_and_run
             at /checkout/src/bootstrap/src/core/builder/cli_paths.rs:141:22
   9: <bootstrap::core::builder::Builder>::run_step_descriptions
             at /checkout/src/bootstrap/src/core/builder/mod.rs:1138:9
  10: <bootstrap::core::builder::Builder>::execute_cli
             at /checkout/src/bootstrap/src/core/builder/mod.rs:1117:14
  11: <bootstrap::Build>::build
             at /checkout/src/bootstrap/src/lib.rs:803:25
  12: bootstrap::main
             at /checkout/src/bootstrap/src/bin/main.rs:130:11
  13: <fn() as core::ops::function::FnOnce<()>>::call_once
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/core/src/ops/function.rs:250:5
  14: std::sys::backtrace::__rust_begin_short_backtrace::<fn(), ()>
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/sys/backtrace.rs:166:18
  15: std::rt::lang_start::<()>::{closure#0}
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/rt.rs:206:18
  16: <&dyn core::ops::function::Fn<(), Output = i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe as core::ops::function::FnOnce<()>>::call_once
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/core/src/ops/function.rs:287:21
  17: std::panicking::catch_unwind::do_call::<&dyn core::ops::function::Fn<(), Output = i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe, i32>
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panicking.rs:581:40
  18: std::panicking::catch_unwind::<i32, &dyn core::ops::function::Fn<(), Output = i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe>
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panicking.rs:544:19
  19: std::panic::catch_unwind::<&dyn core::ops::function::Fn<(), Output = i32> + core::marker::Sync + core::panic::unwind_safe::RefUnwindSafe, i32>
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panic.rs:359:14
  20: std::rt::lang_start_internal::{closure#0}
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/rt.rs:175:24
  21: std::panicking::catch_unwind::do_call::<std::rt::lang_start_internal::{closure#0}, isize>
             at /rustc/ef0fb8a2563200e322fa4419f09f65a63742038c/library/std/src/panicking.rs:581:40
---
  28: __libc_start_main
  29: _start


Command has failed. Rerun with -v to see more details.
Build completed unsuccessfully in 1:08:37
  local time: Mon May  4 22:34:10 UTC 2026
  network time: Mon, 04 May 2026 22:34:10 GMT
##[error]Process completed with exit code 1.
##[group]Run echo "disk usage:"

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented May 4, 2026

💔 Test for edafd5c failed: CI. Failed job:

@frank-king

This comment has been minimized.

@petrochenkov
Copy link
Copy Markdown
Contributor

Broken links in the reference again.
@rustbot author

@P8L1
Copy link
Copy Markdown
Contributor

P8L1 commented May 5, 2026

@frank-king @petrochenkov I opened a Reference PR for the current linkchecker failure: rust-lang/reference#2261

It avoids the direct core::ops::Drop::drop method-fragment link that resolves to the stale #tymethod.drop anchor after this PR makes Drop::drop a provided/default method.

@ehuss
Copy link
Copy Markdown
Contributor

ehuss commented May 5, 2026

I think for fixing the test failure, I think I would prefer to do something like this:

diff --git a/src/ci/docker/scripts/x86_64-gnu-llvm3.sh b/src/ci/docker/scripts/x86_64-gnu-llvm3.sh
index 17eb2cea59a..d2989b3aae6 100755
--- a/src/ci/docker/scripts/x86_64-gnu-llvm3.sh
+++ b/src/ci/docker/scripts/x86_64-gnu-llvm3.sh
@@ -4,7 +4,9 @@ set -ex

 ##### Test stage 1 #####

-../x.py --stage 1 test --skip src/tools/tidy
+# linkchecker is skipped because mixing old rustc/rustdoc with new standard
+# library causes problems with generating correct links.
+../x.py --stage 1 test --skip src/tools/tidy --skip src/tools/linkchecker

 # Run the `mir-opt` tests again but this time for a 32-bit target.
 # This enforces that tests using `// EMIT_MIR_FOR_EACH_BIT_WIDTH` have

Generally I think it is too problematic to try to get the linkchecks working with --stage 1.

cc @jieyouxu @Kobzol in case you want to weigh in on this kind of change.

This was previously discussed at https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/Skipping.20stage.201.20tests/with/583642338 for a closely related issue. In this case, I don't think we can employ easy tricks to make this work in the general case.

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

Labels

A-attributes Area: Attributes (`#[…]`, `#![…]`) S-experimental Status: Ongoing experiment that does not require reviewing and won't be merged in its current state. 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.