diff options
| author | Matthias Krüger <matthias.krueger@famsik.de> | 2025-01-24 23:25:46 +0100 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2025-01-24 23:25:46 +0100 |
| commit | 99e34a4ea0592d53496900ec9918018e16f67992 (patch) | |
| tree | a1be987475e7a44f532b23e5de09558c17f8de13 | |
| parent | 8824ae6a6c14636a4937f89e9caee80da53a45dc (diff) | |
| parent | 9b82f2026133642f884ff9f019edcf9eb270894b (diff) | |
| download | rust-99e34a4ea0592d53496900ec9918018e16f67992.tar.gz rust-99e34a4ea0592d53496900ec9918018e16f67992.zip | |
Rollup merge of #135976 - WaffleLapkin:tailcall-nodrop, r=oli-obk
Don't drop types with no drop glue when building drops for tailcalls
this is required as otherwise drops of `&mut` refs count as a usage of a
'two-phase temporary' causing an ICE.
fixes #128097
The underlying issue is that the current code generates drops for `&mut` which are later counted as a second use of a two-phase temporary:
`bat t.rs -p`
```rust
#![expect(incomplete_features)]
#![feature(explicit_tail_calls)]
fn f(x: &mut ()) {
let _y = String::new();
become f(x);
}
fn main() {}
```
`rustc t.rs -Zdump_mir=f`
```text
error: internal compiler error: compiler/rustc_borrowck/src/borrow_set.rs:298:17: found two uses for 2-phase borrow temporary _4: bb2[1] and bb3[0]
--> t.rs:6:5
|
6 | become f(x);
| ^^^^^^^^^^^
thread 'rustc' panicked at compiler/rustc_borrowck/src/borrow_set.rs:298:17:
Box<dyn Any>
stack backtrace:
[REDACTED]
error: aborting due to 1 previous error
```
`bat ./mir_dump/t.f.-------.renumber.0.mir -p -lrust`
```rust
// MIR for `f` 0 renumber
fn f(_1: &mut ()) -> () {
debug x => _1;
let mut _0: ();
let mut _2: !;
let _3: std::string::String;
let mut _4: &mut ();
scope 1 {
debug _y => _3;
}
bb0: {
StorageLive(_3);
_3 = String::new() -> [return: bb1, unwind: bb4];
}
bb1: {
FakeRead(ForLet(None), _3);
StorageLive(_4);
_4 = &mut (*_1);
drop(_3) -> [return: bb2, unwind: bb3];
}
bb2: {
StorageDead(_3);
tailcall f(Spanned { node: move _4, span: t.rs:6:14: 6:15 (#0) });
}
bb3 (cleanup): {
drop(_4) -> [return: bb4, unwind terminate(cleanup)];
}
bb4 (cleanup): {
resume;
}
}
```
Note how `_4 is moved into the tail call in `bb2` and dropped in `bb3`.
This PR adds a check that the locals we drop need dropping.
r? `@oli-obk` (feel free to reassign, I'm not sure who would be a good reviewer, but thought you might have an idea)
cc `@beepster4096,` since you wrote the original drop implementation.
13 files changed, 28 insertions, 20 deletions
diff --git a/compiler/rustc_mir_build/src/builder/scope.rs b/compiler/rustc_mir_build/src/builder/scope.rs index 20441530a47..945f02821d9 100644 --- a/compiler/rustc_mir_build/src/builder/scope.rs +++ b/compiler/rustc_mir_build/src/builder/scope.rs @@ -785,6 +785,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let local = place.as_local().unwrap_or_else(|| bug!("projection in tail call args")); + if !self.local_decls[local].ty.needs_drop(self.tcx, self.typing_env()) { + return None; + } + Some(DropData { source_info, local, kind: DropKind::Value }) } Operand::Constant(_) => None, @@ -795,6 +799,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { self.scopes.scopes.iter().rev().nth(1).unwrap().region_scope, DUMMY_SP, ); + let typing_env = self.typing_env(); let unwind_drops = &mut self.scopes.unwind_drops; // the innermost scope contains only the destructors for the tail call arguments @@ -805,6 +810,10 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let source_info = drop_data.source_info; let local = drop_data.local; + if !self.local_decls[local].ty.needs_drop(self.tcx, typing_env) { + continue; + } + match drop_data.kind { DropKind::Value => { // `unwind_to` should drop the value that we're about to diff --git a/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr b/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr index 6acd69ab3f8..33e1e53ea06 100644 --- a/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr +++ b/src/tools/miri/tests/fail/tail_calls/dangling-local-var.stderr @@ -14,8 +14,8 @@ LL | let local = 0; help: ALLOC was deallocated here: --> tests/fail/tail_calls/dangling-local-var.rs:LL:CC | -LL | } - | ^ +LL | become g(ptr) + | ^^^^^^^^^^^^^ = note: BACKTRACE (of the first span): = note: inside `g` at tests/fail/tail_calls/dangling-local-var.rs:LL:CC note: inside `main` diff --git a/tests/crashes/128097.rs b/tests/crashes/128097.rs deleted file mode 100644 index 6ffca640cbd..00000000000 --- a/tests/crashes/128097.rs +++ /dev/null @@ -1,6 +0,0 @@ -//@ known-bug: #128097 -#![feature(explicit_tail_calls)] -fn f(x: &mut ()) { - let _y: String; - become f(x); -} diff --git a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff index 17c64d4baf0..9a4f27a497d 100644 --- a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff +++ b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-abort.diff @@ -66,7 +66,6 @@ bb6: { + _8 = const false; StorageDead(_4); - StorageDead(_3); drop(_2) -> [return: bb7, unwind: bb12]; } diff --git a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff index 58d8a87986d..f13ee78aa36 100644 --- a/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff +++ b/tests/mir-opt/tail_call_drops.f.ElaborateDrops.panic-unwind.diff @@ -66,7 +66,6 @@ bb6: { + _8 = const false; StorageDead(_4); - StorageDead(_3); - drop(_2) -> [return: bb7, unwind continue]; + drop(_2) -> [return: bb7, unwind: bb12]; } diff --git a/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir b/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir index 2c3d62491d7..e017424a4cc 100644 --- a/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir +++ b/tests/mir-opt/tail_call_drops.f.built.after.panic-abort.mir @@ -63,7 +63,6 @@ fn f() -> () { bb6: { StorageDead(_4); - StorageDead(_3); drop(_2) -> [return: bb7, unwind: bb17]; } diff --git a/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir b/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir index 2c3d62491d7..e017424a4cc 100644 --- a/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir +++ b/tests/mir-opt/tail_call_drops.f.built.after.panic-unwind.mir @@ -63,7 +63,6 @@ fn f() -> () { bb6: { StorageDead(_4); - StorageDead(_3); drop(_2) -> [return: bb7, unwind: bb17]; } diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff index 1a51601bc56..a8c57d2cfe0 100644 --- a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff +++ b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-abort.diff @@ -80,7 +80,6 @@ bb8: { + _12 = const false; StorageDead(_6); - StorageDead(_5); drop(_4) -> [return: bb9, unwind: bb16]; } diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff index 1a51601bc56..a8c57d2cfe0 100644 --- a/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff +++ b/tests/mir-opt/tail_call_drops.f_with_arg.ElaborateDrops.panic-unwind.diff @@ -80,7 +80,6 @@ bb8: { + _12 = const false; StorageDead(_6); - StorageDead(_5); drop(_4) -> [return: bb9, unwind: bb16]; } diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir index 744f1989acc..f89b98a3205 100644 --- a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir +++ b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-abort.mir @@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () { bb8: { StorageDead(_6); - StorageDead(_5); drop(_4) -> [return: bb9, unwind: bb23]; } diff --git a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir index 744f1989acc..f89b98a3205 100644 --- a/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir +++ b/tests/mir-opt/tail_call_drops.f_with_arg.built.after.panic-unwind.mir @@ -77,7 +77,6 @@ fn f_with_arg(_1: String, _2: String) -> () { bb8: { StorageDead(_6); - StorageDead(_5); drop(_4) -> [return: bb9, unwind: bb23]; } diff --git a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr index 75fb13c378c..ece581dc626 100644 --- a/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr +++ b/tests/ui/explicit-tail-calls/ctfe-arg-bad-borrow.stderr @@ -4,10 +4,9 @@ error[E0597]: `local` does not live long enough LL | let local = Type; | ----- binding `local` declared here LL | become takes_borrow(&local); - | ^^^^^^ borrowed value does not live long enough -LL | -LL | } - | - `local` dropped here while still borrowed + | ^^^^^^- `local` dropped here while still borrowed + | | + | borrowed value does not live long enough error: aborting due to 1 previous error diff --git a/tests/ui/explicit-tail-calls/two-phase.rs b/tests/ui/explicit-tail-calls/two-phase.rs new file mode 100644 index 00000000000..91365b5e739 --- /dev/null +++ b/tests/ui/explicit-tail-calls/two-phase.rs @@ -0,0 +1,14 @@ +// regression test for <https://github.com/rust-lang/rust/issues/112788>. +// this test used to ICE because we tried to run drop glue of `x` +// if dropping `_y` (happening at the `become` site) panicked and caused an unwind. +// +//@ check-pass +#![expect(incomplete_features)] +#![feature(explicit_tail_calls)] + +fn f(x: &mut ()) { + let _y = String::new(); + become f(x); +} + +fn main() {} |
