Skip to content

Commit

Permalink
Rollup merge of rust-lang#123050 - RalfJung:panic_str, r=m-ou-se
Browse files Browse the repository at this point in the history
panic_str only exists for the migration to 2021 panic macros

The only caller is `expect_failed`, which is already a cold inline(never) function, so inlining into that function should be fine. (And indeed `panic_str` was `#[inline]` anyway.)

The existence of panic_str risks someone calling it when they should call `panic` instead, and I can't see a reason why this footgun should exist.

I also extended the comment in `panic` to explain why it needs a `'static` string -- I know I've wondered about this in the past and it took me quite a while to understand.
  • Loading branch information
matthiaskrgr authored Apr 23, 2024
2 parents c672773 + 132921e commit 72e8fb4
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 18 deletions.
2 changes: 1 addition & 1 deletion compiler/rustc_lint/src/non_fmt_panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'tcx> LateLintPass<'tcx> for NonPanicFmt {

if Some(def_id) == cx.tcx.lang_items().begin_panic_fn()
|| Some(def_id) == cx.tcx.lang_items().panic_fn()
|| f_diagnostic_name == Some(sym::panic_str)
|| f_diagnostic_name == Some(sym::panic_str_2015)
{
if let Some(id) = f.span.ctxt().outer_expn_data().macro_def_id {
if matches!(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_span/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1348,7 +1348,7 @@ symbols! {
panic_misaligned_pointer_dereference,
panic_nounwind,
panic_runtime,
panic_str,
panic_str_2015,
panic_unwind,
panicking,
param_attrs,
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/option.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@
#![stable(feature = "rust1", since = "1.0.0")]

use crate::iter::{self, FusedIterator, TrustedLen};
use crate::panicking::{panic, panic_str};
use crate::panicking::{panic, panic_display};
use crate::pin::Pin;
use crate::{
cmp, convert, hint, mem,
Expand Down Expand Up @@ -1991,7 +1991,7 @@ const fn unwrap_failed() -> ! {
#[track_caller]
#[rustc_const_unstable(feature = "const_option", issue = "67441")]
const fn expect_failed(msg: &str) -> ! {
panic_str(msg)
panic_display(&msg)
}

/////////////////////////////////////////////////////////////////////////////
Expand Down
4 changes: 2 additions & 2 deletions library/core/src/panic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@ pub macro panic_2015 {
($msg:literal $(,)?) => (
$crate::panicking::panic($msg)
),
// Use `panic_str` instead of `panic_display::<&str>` for non_fmt_panic lint.
// Use `panic_str_2015` instead of `panic_display::<&str>` for non_fmt_panic lint.
($msg:expr $(,)?) => ({
$crate::panicking::panic_str($msg);
$crate::panicking::panic_str_2015($msg);
}),
// Special-case the single-argument case for const_panic.
("{}", $arg:expr $(,)?) => ({
Expand Down
27 changes: 17 additions & 10 deletions library/core/src/panicking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,8 @@ pub const fn panic_nounwind_fmt(fmt: fmt::Arguments<'_>, force_no_backtrace: boo
// above.

/// The underlying implementation of core's `panic!` macro when no formatting is used.
// never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible
// Never inline unless panic_immediate_abort to avoid code
// bloat at the call sites as much as possible.
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
#[track_caller]
Expand All @@ -138,6 +138,11 @@ pub const fn panic(expr: &'static str) -> ! {
// truncation and padding (even though none is used here). Using
// Arguments::new_const may allow the compiler to omit Formatter::pad from the
// output binary, saving up to a few kilobytes.
// However, this optimization only works for `'static` strings: `new_const` also makes this
// message return `Some` from `Arguments::as_str`, which means it can become part of the panic
// payload without any allocation or copying. Shorter-lived strings would become invalid as
// stack frames get popped during unwinding, and couldn't be directly referenced from the
// payload.
panic_fmt(fmt::Arguments::new_const(&[expr]));
}

Expand Down Expand Up @@ -223,14 +228,6 @@ pub fn panic_nounwind_nobacktrace(expr: &'static str) -> ! {
panic_nounwind_fmt(fmt::Arguments::new_const(&[expr]), /* force_no_backtrace */ true);
}

#[inline]
#[track_caller]
#[rustc_diagnostic_item = "panic_str"]
#[rustc_const_unstable(feature = "panic_internals", issue = "none")]
pub const fn panic_str(expr: &str) -> ! {
panic_display(&expr);
}

#[track_caller]
#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never), cold)]
#[cfg_attr(feature = "panic_immediate_abort", inline)]
Expand All @@ -246,6 +243,16 @@ pub fn unreachable_display<T: fmt::Display>(x: &T) -> ! {
panic_fmt(format_args!("internal error: entered unreachable code: {}", *x));
}

/// This exists solely for the 2015 edition `panic!` macro to trigger
/// a lint on `panic!(my_str_variable);`.
#[inline]
#[track_caller]
#[rustc_diagnostic_item = "panic_str_2015"]
#[rustc_const_unstable(feature = "panic_internals", issue = "none")]
pub const fn panic_str_2015(expr: &str) -> ! {
panic_display(&expr);
}

#[inline]
#[track_caller]
#[rustc_do_not_const_check] // hooked by const-eval
Expand Down
4 changes: 2 additions & 2 deletions tests/ui-fulldeps/stable-mir/check_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,9 +137,9 @@ fn generate_input(path: &str) -> std::io::Result<()> {
write!(
file,
r#"
#![feature(panic_internals)]
fn panic_str(msg: &str) {{ panic!("{{}}", msg); }}
pub fn dummy() {{
core::panicking::panic_str("oops");
panic_str("oops");
}}
"#
)?;
Expand Down

0 comments on commit 72e8fb4

Please sign in to comment.