about summary refs log tree commit diff
diff options
context:
space:
mode:
authorbors <bors@rust-lang.org>2023-05-19 07:15:38 +0000
committerbors <bors@rust-lang.org>2023-05-19 07:15:38 +0000
commit2d17294d18040f872e5c33e38cf9ce8da860f609 (patch)
treefae1ac6520ccc6635fe87f20615c373a2543f721
parent19ca5692f69d20643656bf501fd171f1907ef875 (diff)
parent2f5d99394511b0b6f90182e34ce38001aa02f5aa (diff)
downloadrust-2d17294d18040f872e5c33e38cf9ce8da860f609.tar.gz
rust-2d17294d18040f872e5c33e38cf9ce8da860f609.zip
Auto merge of #111590 - dtolnay:panictemporaries, r=bjorn3
Shorten even more panic temporary lifetimes

Followup to #104134. As pointed out by `@bjorn3` in https://github.com/rust-lang/rust/pull/104134#pullrequestreview-1425585948, there are other cases in the panic macros which would also benefit from dropping their non-Send temporaries as soon as possible, avoiding pointlessly holding them across an await point.

For the tests added in this PR, here are the failures you get today on master without the macro changes in this PR:

<details>
<summary>tests/ui/macros/panic-temporaries-2018.rs</summary>

```console
error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries-2018.rs:52:18
   |
LL |     require_send(panic_display());
   |                  ^^^^^^^^^^^^^^^ future returned by `panic_display` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries-2018.rs:35:31
   |
LL |     f(panic!("{}", NOT_SEND)).await;
   |                    --------   ^^^^^- `NOT_SEND` is later dropped here
   |                    |          |
   |                    |          await occurs here, with `NOT_SEND` maybe used later
   |                    has type `NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries-2018.rs:48:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries-2018.rs:52:18
   |
LL |     require_send(panic_display());
   |                  ^^^^^^^^^^^^^^^ future returned by `panic_display` is not `Send`
   |
   = help: within `NotSend`, the trait `Sync` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries-2018.rs:35:31
   |
LL |     f(panic!("{}", NOT_SEND)).await;
   |       ----------------------  ^^^^^- the value is later dropped here
   |       |                       |
   |       |                       await occurs here, with the value maybe used later
   |       has type `&NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries-2018.rs:48:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries-2018.rs:53:18
   |
LL |     require_send(panic_str());
   |                  ^^^^^^^^^^^ future returned by `panic_str` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries-2018.rs:40:36
   |
LL |     f(panic!((NOT_SEND, "...").1)).await;
   |               --------             ^^^^^- `NOT_SEND` is later dropped here
   |               |                    |
   |               |                    await occurs here, with `NOT_SEND` maybe used later
   |               has type `NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries-2018.rs:48:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries-2018.rs:54:18
   |
LL |     require_send(unreachable_display());
   |                  ^^^^^^^^^^^^^^^^^^^^^ future returned by `unreachable_display` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries-2018.rs:45:31
   |
LL |     f(unreachable!(NOT_SEND)).await;
   |                    --------   ^^^^^- `NOT_SEND` is later dropped here
   |                    |          |
   |                    |          await occurs here, with `NOT_SEND` maybe used later
   |                    has type `NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries-2018.rs:48:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries-2018.rs:54:18
   |
LL |     require_send(unreachable_display());
   |                  ^^^^^^^^^^^^^^^^^^^^^ future returned by `unreachable_display` is not `Send`
   |
   = help: within `NotSend`, the trait `Sync` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries-2018.rs:45:31
   |
LL |     f(unreachable!(NOT_SEND)).await;
   |       ----------------------  ^^^^^- the value is later dropped here
   |       |                       |
   |       |                       await occurs here, with the value maybe used later
   |       has type `&NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries-2018.rs:48:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: aborting due to 5 previous errors
```
</details>

<details>
<summary>tests/ui/macros/panic-temporaries.rs</summary>

```console
error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries.rs:42:18
   |
LL |     require_send(panic_display());
   |                  ^^^^^^^^^^^^^^^ future returned by `panic_display` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries.rs:35:31
   |
LL |     f(panic!("{}", NOT_SEND)).await;
   |                    --------   ^^^^^- `NOT_SEND` is later dropped here
   |                    |          |
   |                    |          await occurs here, with `NOT_SEND` maybe used later
   |                    has type `NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries.rs:38:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: future cannot be sent between threads safely
  --> tests/ui/macros/panic-temporaries.rs:42:18
   |
LL |     require_send(panic_display());
   |                  ^^^^^^^^^^^^^^^ future returned by `panic_display` is not `Send`
   |
   = help: within `NotSend`, the trait `Sync` is not implemented for `*const u8`
note: future is not `Send` as this value is used across an await
  --> tests/ui/macros/panic-temporaries.rs:35:31
   |
LL |     f(panic!("{}", NOT_SEND)).await;
   |       ----------------------  ^^^^^- the value is later dropped here
   |       |                       |
   |       |                       await occurs here, with the value maybe used later
   |       has type `&NotSend` which is not `Send`
note: required by a bound in `require_send`
  --> tests/ui/macros/panic-temporaries.rs:38:25
   |
LL | fn require_send(_: impl Send) {}
   |                         ^^^^ required by this bound in `require_send`

error: aborting due to 2 previous errors
```
</details>

r? bjorn3
-rw-r--r--library/core/src/panic.rs24
-rw-r--r--library/std/src/panic.rs4
-rw-r--r--tests/ui/macros/panic-temporaries-2018.rs55
-rw-r--r--tests/ui/macros/panic-temporaries.rs32
4 files changed, 97 insertions, 18 deletions
diff --git a/library/core/src/panic.rs b/library/core/src/panic.rs
index ebcce79b0f8..20be60d3535 100644
--- a/library/core/src/panic.rs
+++ b/library/core/src/panic.rs
@@ -28,13 +28,13 @@ pub macro panic_2015 {
         $crate::panicking::panic($msg)
     ),
     // Use `panic_str` instead of `panic_display::<&str>` for non_fmt_panic lint.
-    ($msg:expr $(,)?) => (
-        $crate::panicking::panic_str($msg)
-    ),
+    ($msg:expr $(,)?) => ({
+        $crate::panicking::panic_str($msg);
+    }),
     // Special-case the single-argument case for const_panic.
-    ("{}", $arg:expr $(,)?) => (
-        $crate::panicking::panic_display(&$arg)
-    ),
+    ("{}", $arg:expr $(,)?) => ({
+        $crate::panicking::panic_display(&$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.
@@ -52,9 +52,9 @@ pub macro panic_2021 {
         $crate::panicking::panic("explicit panic")
     ),
     // Special-case the single-argument case for const_panic.
-    ("{}", $arg:expr $(,)?) => (
-        $crate::panicking::panic_display(&$arg)
-    ),
+    ("{}", $arg:expr $(,)?) => ({
+        $crate::panicking::panic_display(&$arg);
+    }),
     ($($t:tt)+) => ({
         // Semicolon to prevent temporaries inside the formatting machinery from
         // being considered alive in the caller after the panic_fmt call.
@@ -73,9 +73,9 @@ pub macro unreachable_2015 {
     ),
     // Use of `unreachable_display` for non_fmt_panic lint.
     // NOTE: the message ("internal error ...") is embedded directly in unreachable_display
-    ($msg:expr $(,)?) => (
-        $crate::panicking::unreachable_display(&$msg)
-    ),
+    ($msg:expr $(,)?) => ({
+        $crate::panicking::unreachable_display(&$msg);
+    }),
     ($fmt:expr, $($arg:tt)*) => (
         $crate::panic!($crate::concat!("internal error: entered unreachable code: ", $fmt), $($arg)*)
     ),
diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs
index a2ffd8b1e7e..69a6f3e6d5a 100644
--- a/library/std/src/panic.rs
+++ b/library/std/src/panic.rs
@@ -19,11 +19,11 @@ pub macro panic_2015 {
         $crate::rt::begin_panic("explicit panic")
     }),
     ($msg:expr $(,)?) => ({
-        $crate::rt::begin_panic($msg)
+        $crate::rt::begin_panic($msg);
     }),
     // Special-case the single-argument case for const_panic.
     ("{}", $arg:expr $(,)?) => ({
-        $crate::rt::panic_display(&$arg)
+        $crate::rt::panic_display(&$arg);
     }),
     ($fmt:expr, $($arg:tt)+) => ({
         // Semicolon to prevent temporaries inside the formatting machinery from
diff --git a/tests/ui/macros/panic-temporaries-2018.rs b/tests/ui/macros/panic-temporaries-2018.rs
new file mode 100644
index 00000000000..d914df38062
--- /dev/null
+++ b/tests/ui/macros/panic-temporaries-2018.rs
@@ -0,0 +1,55 @@
+// check-pass
+// edition:2018
+
+#![allow(non_fmt_panics, unreachable_code)]
+
+use std::fmt::{self, Display};
+use std::marker::PhantomData;
+
+struct NotSend {
+    marker: PhantomData<*const u8>,
+}
+
+const NOT_SEND: NotSend = NotSend { marker: PhantomData };
+
+impl Display for NotSend {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("this value does not implement Send")
+    }
+}
+
+async fn f(_: u8) {}
+
+// Exercises this matcher in panic_2015:
+// ($fmt:expr, $($arg:tt)+) => $crate::panicking::panic_fmt(...)
+async fn panic_fmt() {
+    // Panic returns `!`, so the await is never reached, and in particular the
+    // temporaries inside the formatting machinery are not still alive at the
+    // await point.
+    let todo = "...";
+    f(panic!("not yet implemented: {}", todo)).await;
+}
+
+// Exercises ("{}", $arg:expr) => $crate::panicking::panic_display(&$arg)
+async fn panic_display() {
+    f(panic!("{}", NOT_SEND)).await;
+}
+
+// Exercises ($msg:expr) => $crate::panicking::panic_str($msg)
+async fn panic_str() {
+    f(panic!((NOT_SEND, "...").1)).await;
+}
+
+// Exercises ($msg:expr) => $crate::panicking::unreachable_display(&$msg)
+async fn unreachable_display() {
+    f(unreachable!(NOT_SEND)).await;
+}
+
+fn require_send(_: impl Send) {}
+
+fn main() {
+    require_send(panic_fmt());
+    require_send(panic_display());
+    require_send(panic_str());
+    require_send(unreachable_display());
+}
diff --git a/tests/ui/macros/panic-temporaries.rs b/tests/ui/macros/panic-temporaries.rs
index 5b5b8b7c2d9..db65601fb73 100644
--- a/tests/ui/macros/panic-temporaries.rs
+++ b/tests/ui/macros/panic-temporaries.rs
@@ -3,17 +3,41 @@
 
 #![allow(unreachable_code)]
 
+use std::fmt::{self, Display};
+use std::marker::PhantomData;
+
+struct NotSend {
+    marker: PhantomData<*const u8>,
+}
+
+const NOT_SEND: NotSend = NotSend { marker: PhantomData };
+
+impl Display for NotSend {
+    fn fmt(&self, formatter: &mut fmt::Formatter) -> fmt::Result {
+        formatter.write_str("this value does not implement Send")
+    }
+}
+
 async fn f(_: u8) {}
 
-async fn g() {
-    // Todo returns `!`, so the await is never reached, and in particular the
+// Exercises this matcher in panic_2021:
+// ($($t:tt)+) => $crate::panicking::panic_fmt(...)
+async fn panic_fmt() {
+    // Panic 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;
+    let todo = "...";
+    f(panic!("not yet implemented: {}", todo)).await;
+}
+
+// Exercises ("{}", $arg:expr) => $crate::panicking::panic_display(&$arg)
+async fn panic_display() {
+    f(panic!("{}", NOT_SEND)).await;
 }
 
 fn require_send(_: impl Send) {}
 
 fn main() {
-    require_send(g());
+    require_send(panic_fmt());
+    require_send(panic_display());
 }