Skip to content

Shorten lifetime of panic temporaries in panic_fmt case #104134

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 14, 2023
Merged

Shorten lifetime of panic temporaries in panic_fmt case #104134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
cbee2a1
Add ui test to reproduce non-Send panic temporary
dtolnay Nov 7, 2022
cb109a6
Shorten lifetime of panic temporaries in panic_fmt case
dtolnay Nov 7, 2022
4be97e0
Exposes false negative in clippy's diverging_sub_expression
dtolnay May 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 10 additions & 6 deletions library/core/src/panic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand All @@ -53,9 +55,11 @@ pub macro panic_2021 {
("{}", $arg:expr $(,)?) => (
$crate::panicking::panic_display(&$arg)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the same be done here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followup: #111590.

),
($($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)]
Expand Down
4 changes: 3 additions & 1 deletion library/std/src/panic.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)+));
}),
}

Expand Down
10 changes: 1 addition & 9 deletions src/tools/clippy/tests/ui/diverging_sub_expression.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,11 @@ error: sub-expression diverges
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

19 changes: 19 additions & 0 deletions tests/ui/macros/panic-temporaries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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());
}
18 changes: 9 additions & 9 deletions ...macros/rfc-2011-nicer-assert-messages/non-consuming-methods-have-optimized-codegen.stdout
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand All @@ -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));
}
}
};
Expand Down