Shorten lifetime of panic temporaries in panic_fmt case

[dtolnay]

This fixes an issue called out by @fasterthanlime in https://octodon.social/@fasterthanlime/109304454114856561. Macros like todo!("…") and panic!("…", …) drop their format_args temporary at the nearest enclosing semicolon outside the macro invocation, not inside the macro invocation. Due to the formatting internals being type-erased in a way that is not thread-safe, this blocks futures from being Send if there is an await anywhere between the panic call and the nearest enclosing semicolon.

Example:

#![allow(unreachable_code)]
 
async fn f(_: u8) {}
 
async fn g() {
    f(todo!("...")).await;
}
 
fn require_send(_: impl Send) {}
 
fn main() {
    require_send(g());
}

Before:

error: future cannot be sent between threads safely
  --> src/main.rs:15:18
   |
15 |     require_send(g());
   |                  ^^^ future returned by `g` is not `Send`
   |
   = help: the trait `Sync` is not implemented for `core::fmt::Opaque`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:9:20
   |
9  |     f(todo!("...")).await;
   |       ------------ ^^^^^^ await occurs here, with `$crate::format_args!($($arg)+)` maybe used later
   |       |
   |       has type `ArgumentV1<'_>` which is not `Send`
note: `$crate::format_args!($($arg)+)` is later dropped here
  --> src/main.rs:9:26
   |
9  |     f(todo!("...")).await;
   |                          ^
note: required by a bound in `require_send`
  --> src/main.rs:12:25
   |
12 | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

After: works.

Arguably there is a rustc fix that could work here too, instead of a standard library change. Rustc could be taught that the code shown above is fine to compile because the await is unreachable and so temporaries before the await do not need to get put in the anonymous compiler-generated Future struct, regardless of syntactically where they're supposed to be dropped according to the language semantics.

I would be open to that, though my recollection is that in the past we have been very hesitant about introducing any smarts into Drop placement. People want the language rules about where temporaries are dropped to be as simplistic and predictable as possible.

[rustbot]

r? @joshtriplett

(rustbot has picked a reviewer for you, use r? to override)

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[Aaron1011]

This seems very similar to #64856, but I'm not sure if this case has observable drop-order effects.

[dtolnay]

Maybe I should know better after #99689, but I was not able to come up with any program where this PR would cause a change in drop order at runtime.

[joshtriplett]

This seems like a reasonable change to me. I'd want to see it pass a crater run (either directly or as part of the normal beta crater runs) but otherwise 👍.

[dtolnay]

Beta craters are going to be sufficient here I think.

[dtolnay]

• Rebased to resolve conflict with Move src/test to the root #106458

[bors]

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

[dtolnay]

• Rebased to resolve conflict with Move format_args!() into AST (and expand it during AST lowering) #106745

[joshtriplett]

@bors r+

If any beta crater issues point to this it can be reverted.

[bors]

📌 Commit 24a3c6e9f3f3bbdc4056f9a17ec71132305a6ff1 has been approved by joshtriplett

It is now in the queue for this repository.

[bors]

⌛ Testing commit 24a3c6e9f3f3bbdc4056f9a17ec71132305a6ff1 with merge 2843a0374d879599f630d965f729636bd632ab30...

[bors]

💔 Test failed - checks-actions

[rustbot]

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

[dtolnay]

The failure in src/tools/clippy/tests/ui/diverging_sub_expression.stderr is related to rust-lang/rust-clippy#10776. I think this is okay to land without fixing that in clippy first.

[dtolnay]

@bors r=joshtriplett

[bors]

📌 Commit 4be97e0 has been approved by joshtriplett

It is now in the queue for this repository.

[bors]

⌛ Testing commit 4be97e0 with merge eda41ad...

[bjorn3]

Should the same be done here?

[dtolnay]

Followup: #111590.

[bors]

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing eda41ad to master...

[fasterthanlime]

I’m gonna be honest, I completely forgot what the original issue was about but I’m THRILLED an issue this old finally has a resolution. Thanks so much everyone!

[rust-timer]

Finished benchmarking commit (eda41ad): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.6%	[4.3%, 5.1%]	3
Regressions ❌ (secondary)	2.3%	[1.2%, 3.4%]	2
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	4.6%	[4.3%, 5.1%]	3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 641.664s -> 642.273s (0.09%)