Auto merge of #104134 - dtolnay:panictemporaries, r=joshtriplett

Shorten lifetime of panic temporaries in panic_fmt case

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:**

```rust
#![allow(unreachable_code)]

async fn f(_: u8) {}

async fn g() {
    f(todo!("...")).await;
}

fn require_send(_: impl Send) {}

fn main() {
    require_send(g());
}
```

**Before:**

```console
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.

5 files changed, 42 insertions, 25 deletions

diff --git a/library/core/src/panic.rs b/library/core/src/panic.rs
index 8338a5d7e5a..ebcce79b0f8 100644
--- a/library/core/src/panic.rs
+++ b/library/core/src/panic.rs
@@ -35,9 +35,11 @@ pub macro panic_2015 {
     ("{}", $arg:expr $(,)?) => (
         $crate::panicking::panic_display(&$arg)
     ),
-    ($fmt:expr, $($arg:tt)+) => (
-        $crate::panicking::panic_fmt($crate::const_format_args!($fmt, $($arg)+))
-    ),
+    ($fmt:expr, $($arg:tt)+) => ({
+        // Semicolon to prevent temporaries inside the formatting machinery from
+        // being considered alive in the caller after the panic_fmt call.
+        $crate::panicking::panic_fmt($crate::const_format_args!($fmt, $($arg)+));
+    }),
 }
 
 #[doc(hidden)]
@@ -53,9 +55,11 @@ pub macro panic_2021 {
     ("{}", $arg:expr $(,)?) => (
         $crate::panicking::panic_display(&$arg)
     ),
-    ($($t:tt)+) => (
-        $crate::panicking::panic_fmt($crate::const_format_args!($($t)+))
-    ),
+    ($($t:tt)+) => ({
+        // Semicolon to prevent temporaries inside the formatting machinery from
+        // being considered alive in the caller after the panic_fmt call.
+        $crate::panicking::panic_fmt($crate::const_format_args!($($t)+));
+    }),
 }
 
 #[doc(hidden)]
diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs
index 345d72ef867..a2ffd8b1e7e 100644
--- a/library/std/src/panic.rs
+++ b/library/std/src/panic.rs
@@ -26,7 +26,9 @@ pub macro panic_2015 {
         $crate::rt::panic_display(&$arg)
     }),
     ($fmt:expr, $($arg:tt)+) => ({
-        $crate::rt::panic_fmt($crate::const_format_args!($fmt, $($arg)+))
+        // Semicolon to prevent temporaries inside the formatting machinery from
+        // being considered alive in the caller after the panic_fmt call.
+        $crate::rt::panic_fmt($crate::const_format_args!($fmt, $($arg)+));
     }),
 }
 
diff --git a/src/tools/clippy/tests/ui/diverging_sub_expression.stderr b/src/tools/clippy/tests/ui/diverging_sub_expression.stderr
index 9c91d935716..51a3b0d972e 100644
--- a/src/tools/clippy/tests/ui/diverging_sub_expression.stderr
+++ b/src/tools/clippy/tests/ui/diverging_sub_expression.stderr
@@ -31,18 +31,10 @@ LL |             3 => true || diverge(),
    |                          ^^^^^^^^^
 
 error: sub-expression diverges
-  --> $DIR/diverging_sub_expression.rs:36:30
-   |
-LL |                 _ => true || panic!("boo"),
-   |                              ^^^^^^^^^^^^^
-   |
-   = note: this error originates in the macro `$crate::panic::panic_2021` which comes from the expansion of the macro `panic` (in Nightly builds, run with -Z macro-backtrace for more info)
-
-error: sub-expression diverges
   --> $DIR/diverging_sub_expression.rs:38:26
    |
 LL |             _ => true || break,
    |                          ^^^^^
 
-error: aborting due to 7 previous errors
+error: aborting due to 6 previous errors
 
diff --git a/tests/ui/macros/panic-temporaries.rs b/tests/ui/macros/panic-temporaries.rs
new file mode 100644
index 00000000000..5b5b8b7c2d9
--- /dev/null
+++ b/tests/ui/macros/panic-temporaries.rs
@@ -0,0 +1,19 @@
+// check-pass
+// edition:2021
+
+#![allow(unreachable_code)]
+
+async fn f(_: u8) {}
+
+async fn g() {
+    // Todo returns `!`, so the await is never reached, and in particular the
+    // temporaries inside the formatting machinery are not still alive at the
+    // await point.
+    f(todo!("...")).await;
+}
+
+fn require_send(_: impl Send) {}
+
+fn main() {
+    require_send(g());
+}
diff --git a/tests/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout b/tests/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout
index ad97f7a4a75..b69b5bc3b53 100644
--- a/tests/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout
+++ b/tests/ui/macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout
@@ -26,7 +26,7 @@ fn arbitrary_consuming_method_for_demonstration_purposes() {
 
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem as usize\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -42,7 +42,7 @@ fn addr_of() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: &elem\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -58,7 +58,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem == 1\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -71,7 +71,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem >= 1\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -84,7 +84,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem > 0\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -97,7 +97,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem < 3\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -110,7 +110,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem <= 3\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -123,7 +123,7 @@ fn binary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: elem != 3\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };
@@ -139,7 +139,7 @@ fn unary() {
                 (&::core::asserting::Wrapper(__local_bind0)).try_capture(&mut __capture0);
                 {
                     ::std::rt::panic_fmt(format_args!("Assertion failed: *elem\nWith captures:\n  elem = {0:?}\n",
-                            __capture0))
+                            __capture0));
                 }
             }
     };