From 93701569573b367aaeaf659af154f5ae0d49af2b Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Sat, 11 Sep 2021 03:44:02 +0100 Subject: [PATCH 1/2] Deduplicate panic_fmt std's begin_panic_fmt and core's panic_fmt are duplicates. Merge them to declutter code and remove a lang item. --- .../src/const_eval/machine.rs | 4 +-- .../src/transform/check_consts/mod.rs | 1 - compiler/rustc_hir/src/lang_items.rs | 1 - compiler/rustc_span/src/symbol.rs | 1 - library/core/src/panic/panic_info.rs | 2 +- library/core/src/panicking.rs | 9 ++++++- library/std/src/panic.rs | 2 +- library/std/src/panicking.rs | 26 ++----------------- library/std/src/rt.rs | 4 +-- 9 files changed, 15 insertions(+), 35 deletions(-) diff --git a/compiler/rustc_const_eval/src/const_eval/machine.rs b/compiler/rustc_const_eval/src/const_eval/machine.rs index ae20f6f97b212..adc574ce9c9df 100644 --- a/compiler/rustc_const_eval/src/const_eval/machine.rs +++ b/compiler/rustc_const_eval/src/const_eval/machine.rs @@ -51,9 +51,7 @@ impl<'mir, 'tcx> InterpCx<'mir, 'tcx, CompileTimeInterpreter<'mir, 'tcx>> { let span = self.find_closest_untracked_caller_location(); let (file, line, col) = self.location_triple_for_span(span); return Err(ConstEvalErrKind::Panic { msg, file, line, col }.into()); - } else if Some(def_id) == self.tcx.lang_items().panic_fmt() - || Some(def_id) == self.tcx.lang_items().begin_panic_fmt() - { + } else if Some(def_id) == self.tcx.lang_items().panic_fmt() { // For panic_fmt, call const_panic_fmt instead. if let Some(const_panic_fmt) = self.tcx.lang_items().const_panic_fmt() { return Ok(Some( diff --git a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs index d1fd3ceaa589a..8fb0d995ec6c2 100644 --- a/compiler/rustc_const_eval/src/transform/check_consts/mod.rs +++ b/compiler/rustc_const_eval/src/transform/check_consts/mod.rs @@ -82,7 +82,6 @@ pub fn is_lang_panic_fn(tcx: TyCtxt<'tcx>, def_id: DefId) -> bool { || Some(def_id) == tcx.lang_items().panic_display() || Some(def_id) == tcx.lang_items().begin_panic_fn() || Some(def_id) == tcx.lang_items().panic_fmt() - || Some(def_id) == tcx.lang_items().begin_panic_fmt() } pub fn rustc_allow_const_fn_unstable( diff --git a/compiler/rustc_hir/src/lang_items.rs b/compiler/rustc_hir/src/lang_items.rs index 814054c551878..790509b691d38 100644 --- a/compiler/rustc_hir/src/lang_items.rs +++ b/compiler/rustc_hir/src/lang_items.rs @@ -292,7 +292,6 @@ language_item_table! { PanicImpl, sym::panic_impl, panic_impl, Target::Fn, GenericRequirement::None; /// libstd panic entry point. Necessary for const eval to be able to catch it BeginPanic, sym::begin_panic, begin_panic_fn, Target::Fn, GenericRequirement::None; - BeginPanicFmt, sym::begin_panic_fmt, begin_panic_fmt, Target::Fn, GenericRequirement::None; ExchangeMalloc, sym::exchange_malloc, exchange_malloc_fn, Target::Fn, GenericRequirement::None; BoxFree, sym::box_free, box_free_fn, Target::Fn, GenericRequirement::Minimum(1); diff --git a/compiler/rustc_span/src/symbol.rs b/compiler/rustc_span/src/symbol.rs index 9551120ca5522..21e6cdb47e195 100644 --- a/compiler/rustc_span/src/symbol.rs +++ b/compiler/rustc_span/src/symbol.rs @@ -355,7 +355,6 @@ symbols! { await_macro, bang, begin_panic, - begin_panic_fmt, bench, bin, bind_by_move_pattern_guards, diff --git a/library/core/src/panic/panic_info.rs b/library/core/src/panic/panic_info.rs index a52a0022e5d2b..649bc3e44ad21 100644 --- a/library/core/src/panic/panic_info.rs +++ b/library/core/src/panic/panic_info.rs @@ -121,7 +121,7 @@ impl<'a> PanicInfo<'a> { #[stable(feature = "panic_hooks", since = "1.10.0")] pub fn location(&self) -> Option<&Location<'_>> { // NOTE: If this is changed to sometimes return None, - // deal with that case in std::panicking::default_hook and std::panicking::begin_panic_fmt. + // deal with that case in std::panicking::default_hook and core::panicking::panic_fmt. Some(&self.location) } } diff --git a/library/core/src/panicking.rs b/library/core/src/panicking.rs index 6d3ec6ae8612a..a12447acf7ec3 100644 --- a/library/core/src/panicking.rs +++ b/library/core/src/panicking.rs @@ -76,8 +76,15 @@ fn panic_bounds_check(index: usize, len: usize) -> ! { panic!("index out of bounds: the len is {} but the index is {}", len, index) } -/// The underlying implementation of libcore's `panic!` macro when formatting is used. +/// The entry point for panicking with a formatted message. +/// +/// This is designed to reduce the amount of code required at the call +/// site as much as possible (so that `panic!()` has as low an impact +/// on (e.g.) the inlining of other functions as possible), by moving +/// the actual formatting into this shared place. #[cold] +// If panic_immediate_abort, inline the abort call, +// otherwise avoid inlining because of it is cold path. #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cfg_attr(feature = "panic_immediate_abort", inline)] #[track_caller] diff --git a/library/std/src/panic.rs b/library/std/src/panic.rs index 21e9669c11079..c0605b2f4121c 100644 --- a/library/std/src/panic.rs +++ b/library/std/src/panic.rs @@ -25,7 +25,7 @@ pub macro panic_2015 { $crate::rt::panic_display(&$arg) }), ($fmt:expr, $($arg:tt)+) => ({ - $crate::rt::begin_panic_fmt(&$crate::const_format_args!($fmt, $($arg)+)) + $crate::rt::panic_fmt($crate::const_format_args!($fmt, $($arg)+)) }), } diff --git a/library/std/src/panicking.rs b/library/std/src/panicking.rs index 231c9fc19c08a..56646b72dd54f 100644 --- a/library/std/src/panicking.rs +++ b/library/std/src/panicking.rs @@ -437,31 +437,9 @@ pub fn panicking() -> bool { !panic_count::count_is_zero() } -/// The entry point for panicking with a formatted message. -/// -/// This is designed to reduce the amount of code required at the call -/// site as much as possible (so that `panic!()` has as low an impact -/// on (e.g.) the inlining of other functions as possible), by moving -/// the actual formatting into this shared place. -#[unstable(feature = "libstd_sys_internals", reason = "used by the panic! macro", issue = "none")] -#[cold] -// If panic_immediate_abort, inline the abort call, -// otherwise avoid inlining because of it is cold path. -#[cfg_attr(not(feature = "panic_immediate_abort"), track_caller)] -#[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] -#[cfg_attr(feature = "panic_immediate_abort", inline)] -#[cfg_attr(not(test), lang = "begin_panic_fmt")] -pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>) -> ! { - if cfg!(feature = "panic_immediate_abort") { - intrinsics::abort() - } - - let info = PanicInfo::internal_constructor(Some(msg), Location::caller()); - begin_panic_handler(&info) -} - /// Entry point of panics from the libcore crate (`panic_impl` lang item). -#[cfg_attr(not(test), panic_handler)] +#[cfg(not(test))] +#[panic_handler] pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { struct PanicPayload<'a> { inner: &'a fmt::Arguments<'a>, diff --git a/library/std/src/rt.rs b/library/std/src/rt.rs index 4d72aff011684..121c214780d2d 100644 --- a/library/std/src/rt.rs +++ b/library/std/src/rt.rs @@ -19,8 +19,8 @@ use crate::ffi::CString; // Re-export some of our utilities which are expected by other crates. -pub use crate::panicking::{begin_panic, begin_panic_fmt, panic_count}; -pub use core::panicking::panic_display; +pub use crate::panicking::{begin_panic, panic_count}; +pub use core::panicking::{panic_display, panic_fmt}; use crate::sync::Once; use crate::sys; From 7bd93dfeefac34a440ff011786d28e7b821add31 Mon Sep 17 00:00:00 2001 From: Gary Guo Date: Tue, 19 Oct 2021 13:58:58 +0100 Subject: [PATCH 2/2] Remove begin_panic_fmt from clippy --- src/tools/clippy/clippy_utils/src/higher.rs | 1 - src/tools/clippy/clippy_utils/src/lib.rs | 1 - src/tools/clippy/clippy_utils/src/paths.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/src/tools/clippy/clippy_utils/src/higher.rs b/src/tools/clippy/clippy_utils/src/higher.rs index ba4d50bf74469..74cf323720cbb 100644 --- a/src/tools/clippy/clippy_utils/src/higher.rs +++ b/src/tools/clippy/clippy_utils/src/higher.rs @@ -619,7 +619,6 @@ impl PanicExpn<'tcx> { if let Some(init) = block.expr; if let ExprKind::Call(_, [format_args]) = init.kind; let expn_data = expr.span.ctxt().outer_expn_data(); - if let ExprKind::AddrOf(_, _, format_args) = format_args.kind; if let Some(format_args) = FormatArgsExpn::parse(format_args); then { Some(PanicExpn { diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index c47aa9170e547..8e94d16a33a0e 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -1646,7 +1646,6 @@ pub fn match_panic_def_id(cx: &LateContext<'_>, did: DefId) -> bool { did, &[ &paths::BEGIN_PANIC, - &paths::BEGIN_PANIC_FMT, &paths::PANIC_ANY, &paths::PANICKING_PANIC, &paths::PANICKING_PANIC_FMT, diff --git a/src/tools/clippy/clippy_utils/src/paths.rs b/src/tools/clippy/clippy_utils/src/paths.rs index e43c575602145..81aff585ded1b 100644 --- a/src/tools/clippy/clippy_utils/src/paths.rs +++ b/src/tools/clippy/clippy_utils/src/paths.rs @@ -20,7 +20,6 @@ pub const ARC_PTR_EQ: [&str; 4] = ["alloc", "sync", "Arc", "ptr_eq"]; pub const ASMUT_TRAIT: [&str; 3] = ["core", "convert", "AsMut"]; pub const ASREF_TRAIT: [&str; 3] = ["core", "convert", "AsRef"]; pub(super) const BEGIN_PANIC: [&str; 3] = ["std", "panicking", "begin_panic"]; -pub(super) const BEGIN_PANIC_FMT: [&str; 3] = ["std", "panicking", "begin_panic_fmt"]; /// Preferably use the diagnostic item `sym::Borrow` where possible pub const BORROW_TRAIT: [&str; 3] = ["core", "borrow", "Borrow"]; pub const BTREEMAP_CONTAINS_KEY: [&str; 6] = ["alloc", "collections", "btree", "map", "BTreeMap", "contains_key"];