diff --git a/src/libcore/macros/mod.rs b/src/libcore/macros/mod.rs index 9a52823a45474..0eb9e19423617 100644 --- a/src/libcore/macros/mod.rs +++ b/src/libcore/macros/mod.rs @@ -1,19 +1,13 @@ #[doc(include = "panic.md")] #[macro_export] -#[allow_internal_unstable(core_panic, - // FIXME(anp, eddyb) `core_intrinsics` is used here to allow calling - // the `caller_location` intrinsic, but once `#[track_caller]` is implemented, - // `panicking::{panic, panic_fmt}` can use that instead of a `Location` argument. - core_intrinsics, - const_caller_location, -)] +#[allow_internal_unstable(core_panic, track_caller)] #[stable(feature = "core", since = "1.6.0")] macro_rules! panic { () => ( $crate::panic!("explicit panic") ); ($msg:expr) => ( - $crate::panicking::panic($msg, $crate::intrinsics::caller_location()) + $crate::panicking::panic($msg) ); ($msg:expr,) => ( $crate::panic!($msg) @@ -21,7 +15,7 @@ macro_rules! panic { ($fmt:expr, $($arg:tt)+) => ( $crate::panicking::panic_fmt( $crate::format_args!($fmt, $($arg)+), - $crate::intrinsics::caller_location(), + $crate::panic::Location::caller(), ) ); } diff --git a/src/libcore/panicking.rs b/src/libcore/panicking.rs index 7ebb72e3ce7ba..61b764f2d6206 100644 --- a/src/libcore/panicking.rs +++ b/src/libcore/panicking.rs @@ -36,8 +36,9 @@ use crate::panic::{Location, PanicInfo}; // 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))] +#[track_caller] #[lang = "panic"] // needed by codegen for panic on overflow and other `Assert` MIR terminators -pub fn panic(expr: &str, location: &Location<'_>) -> ! { +pub fn panic(expr: &str) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { super::intrinsics::abort() } } @@ -48,7 +49,7 @@ pub fn panic(expr: &str, location: &Location<'_>) -> ! { // truncation and padding (even though none is used here). Using // Arguments::new_v1 may allow the compiler to omit Formatter::pad from the // output binary, saving up to a few kilobytes. - panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), location) + panic_fmt(fmt::Arguments::new_v1(&[expr], &[]), Location::caller()) } #[cold] diff --git a/src/librustc_expand/build.rs b/src/librustc_expand/build.rs index 3375efe41f133..868f620300133 100644 --- a/src/librustc_expand/build.rs +++ b/src/librustc_expand/build.rs @@ -6,7 +6,7 @@ use syntax::ptr::P; use syntax::source_map::{respan, Spanned}; use syntax::symbol::{kw, sym, Symbol}; -use rustc_span::{Pos, Span}; +use rustc_span::Span; impl<'a> ExtCtxt<'a> { pub fn path(&self, span: Span, strs: Vec) -> ast::Path { @@ -350,16 +350,10 @@ impl<'a> ExtCtxt<'a> { } pub fn expr_fail(&self, span: Span, msg: Symbol) -> P { - let loc = self.source_map().lookup_char_pos(span.lo()); - let expr_file = self.expr_str(span, Symbol::intern(&loc.file.name.to_string())); - let expr_line = self.expr_u32(span, loc.line as u32); - let expr_col = self.expr_u32(span, loc.col.to_usize() as u32 + 1); - let expr_loc_tuple = self.expr_tuple(span, vec![expr_file, expr_line, expr_col]); - let expr_loc_ptr = self.expr_addr_of(span, expr_loc_tuple); self.expr_call_global( span, [sym::std, sym::rt, sym::begin_panic].iter().map(|s| Ident::new(*s, span)).collect(), - vec![self.expr_str(span, msg), expr_loc_ptr], + vec![self.expr_str(span, msg)], ) } diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs index a76153c19ec67..aa9c718c3eb68 100644 --- a/src/librustc_mir/const_eval/machine.rs +++ b/src/librustc_mir/const_eval/machine.rs @@ -177,6 +177,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx>], ret: Option<(PlaceTy<'tcx>, mir::BasicBlock)>, @@ -199,7 +200,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, // Some functions we support even if they are non-const -- but avoid testing // that for const fn! We certainly do *not* want to actually call the fn // though, so be sure we return here. - return if ecx.hook_panic_fn(instance, args, ret)? { + return if ecx.hook_panic_fn(span, instance, args)? { Ok(None) } else { throw_unsup_format!("calling non-const function `{}`", instance) diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index cc38cbbac8d65..b075e8ea383e1 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -366,47 +366,21 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Returns `true` if an intercept happened. pub fn hook_panic_fn( &mut self, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, M::PointerTag>], - _ret: Option<(PlaceTy<'tcx, M::PointerTag>, mir::BasicBlock)>, ) -> InterpResult<'tcx, bool> { let def_id = instance.def_id(); - if Some(def_id) == self.tcx.lang_items().panic_fn() { - // &'static str, &core::panic::Location { &'static str, u32, u32 } - assert!(args.len() == 2); + if Some(def_id) == self.tcx.lang_items().panic_fn() + || Some(def_id) == self.tcx.lang_items().begin_panic_fn() + { + // &'static str + assert!(args.len() == 1); let msg_place = self.deref_operand(args[0])?; let msg = Symbol::intern(self.read_str(msg_place)?); - - let location = self.deref_operand(args[1])?; - let (file, line, col) = ( - self.mplace_field(location, 0)?, - self.mplace_field(location, 1)?, - self.mplace_field(location, 2)?, - ); - - let file_place = self.deref_operand(file.into())?; - let file = Symbol::intern(self.read_str(file_place)?); - let line = self.read_scalar(line.into())?.to_u32()?; - let col = self.read_scalar(col.into())?.to_u32()?; - throw_panic!(Panic { msg, file, line, col }) - } else if Some(def_id) == self.tcx.lang_items().begin_panic_fn() { - assert!(args.len() == 2); - // &'static str, &(&'static str, u32, u32) - let msg = args[0]; - let place = self.deref_operand(args[1])?; - let (file, line, col) = ( - self.mplace_field(place, 0)?, - self.mplace_field(place, 1)?, - self.mplace_field(place, 2)?, - ); - - let msg_place = self.deref_operand(msg.into())?; - let msg = Symbol::intern(self.read_str(msg_place)?); - let file_place = self.deref_operand(file.into())?; - let file = Symbol::intern(self.read_str(file_place)?); - let line = self.read_scalar(line.into())?.to_u32()?; - let col = self.read_scalar(col.into())?.to_u32()?; + let span = self.find_closest_untracked_caller_location().unwrap_or(span); + let (file, line, col) = self.location_triple_for_span(span); throw_panic!(Panic { msg, file, line, col }) } else { return Ok(false); diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs index da0a9fb4f1be5..0525108d2d129 100644 --- a/src/librustc_mir/interpret/intrinsics/caller_location.rs +++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs @@ -9,8 +9,9 @@ use crate::interpret::{ }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { - /// Walks up the callstack from the intrinsic's callsite, searching for the first frame which is - /// not `#[track_caller]`. + /// Walks up the callstack from the intrinsic's callsite, searching for the first callsite in a + /// frame which is not `#[track_caller]`. If the first frame found lacks `#[track_caller]`, then + /// `None` is returned and the callsite of the function invocation itself should be used. crate fn find_closest_untracked_caller_location(&self) -> Option { let mut caller_span = None; for next_caller in self.stack.iter().rev() { @@ -54,9 +55,14 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } pub fn alloc_caller_location_for_span(&mut self, span: Span) -> MPlaceTy<'tcx, M::PointerTag> { + let (file, line, column) = self.location_triple_for_span(span); + self.alloc_caller_location(file, line, column) + } + + pub(super) fn location_triple_for_span(&self, span: Span) -> (Symbol, u32, u32) { let topmost = span.ctxt().outer_expn().expansion_cause().unwrap_or(span); let caller = self.tcx.sess.source_map().lookup_char_pos(topmost.lo()); - self.alloc_caller_location( + ( Symbol::intern(&caller.file.name.to_string()), caller.line as u32, caller.col_display as u32 + 1, diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 3b444ac46ef26..3dc572d256d8e 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -139,6 +139,7 @@ pub trait Machine<'mir, 'tcx>: Sized { /// was used. fn find_mir_or_eval_fn( ecx: &mut InterpCx<'mir, 'tcx, Self>, + span: Span, instance: ty::Instance<'tcx>, args: &[OpTy<'tcx, Self::PointerTag>], ret: Option<(PlaceTy<'tcx, Self::PointerTag>, mir::BasicBlock)>, diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 1cc22c03a05f9..7b0efd2177882 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -238,7 +238,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { | ty::InstanceDef::CloneShim(..) | ty::InstanceDef::Item(_) => { // We need MIR for this fn - let body = match M::find_mir_or_eval_fn(self, instance, args, ret, unwind)? { + let body = match M::find_mir_or_eval_fn(self, span, instance, args, ret, unwind)? { Some(body) => body, None => return Ok(()), }; diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index e1c2b494337e7..4580ae631c2f4 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -125,6 +125,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine { fn find_mir_or_eval_fn( _ecx: &mut InterpCx<'mir, 'tcx, Self>, + _span: Span, _instance: ty::Instance<'tcx>, _args: &[OpTy<'tcx>], _ret: Option<(PlaceTy<'tcx>, BasicBlock)>, diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs index 930bf397bc45b..23f82d7c2119d 100644 --- a/src/libstd/lib.rs +++ b/src/libstd/lib.rs @@ -306,6 +306,7 @@ #![feature(thread_local)] #![feature(toowned_clone_into)] #![feature(trace_macros)] +#![feature(track_caller)] #![feature(try_reserve)] #![feature(unboxed_closures)] #![feature(untagged_unions)] diff --git a/src/libstd/macros.rs b/src/libstd/macros.rs index 11850a1b5fc38..18fb0f87688de 100644 --- a/src/libstd/macros.rs +++ b/src/libstd/macros.rs @@ -4,6 +4,7 @@ //! library. Each macro is available for use when linking against the standard //! library. +#[cfg(bootstrap)] #[doc(include = "../libcore/macros/panic.md")] #[macro_export] #[stable(feature = "rust1", since = "1.0.0")] @@ -19,8 +20,21 @@ macro_rules! panic { $crate::panic!($msg) }); ($fmt:expr, $($arg:tt)+) => ({ - $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+), - &($crate::file!(), $crate::line!(), $crate::column!())) + $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+)) + }); +} + +#[cfg(not(bootstrap))] +#[doc(include = "../libcore/macros/panic.md")] +#[macro_export] +#[stable(feature = "rust1", since = "1.0.0")] +#[allow_internal_unstable(libstd_sys_internals)] +macro_rules! panic { + () => ({ $crate::panic!("explicit panic") }); + ($msg:expr) => ({ $crate::rt::begin_panic($msg) }); + ($msg:expr,) => ({ $crate::panic!($msg) }); + ($fmt:expr, $($arg:tt)+) => ({ + $crate::rt::begin_panic_fmt(&$crate::format_args!($fmt, $($arg)+)) }); } diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs index 43230d7a2c7af..599ccc809be1f 100644 --- a/src/libstd/panicking.rs +++ b/src/libstd/panicking.rs @@ -313,17 +313,15 @@ pub fn panicking() -> bool { #[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)] -pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>, file_line_col: &(&'static str, u32, u32)) -> ! { +pub fn begin_panic_fmt(msg: &fmt::Arguments<'_>) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { intrinsics::abort() } } - // Just package everything into a `PanicInfo` and continue like libcore panics. - let (file, line, col) = *file_line_col; - let location = Location::internal_constructor(file, line, col); - let info = PanicInfo::internal_constructor(Some(msg), &location); + let info = PanicInfo::internal_constructor(Some(msg), Location::caller()); begin_panic_handler(&info) } @@ -356,6 +354,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { unsafe impl<'a> BoxMeUp for PanicPayload<'a> { fn take_box(&mut self) -> *mut (dyn Any + Send) { + // We do two allocations here, unfortunately. But (a) they're required with the current + // scheme, and (b) we don't handle panic + OOM properly anyway (see comment in + // begin_panic below). let contents = mem::take(self.fill()); Box::into_raw(Box::new(contents)) } @@ -365,15 +366,9 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { } } - // We do two allocations here, unfortunately. But (a) they're - // required with the current scheme, and (b) we don't handle - // panic + OOM properly anyway (see comment in begin_panic - // below). - let loc = info.location().unwrap(); // The current implementation always returns Some let msg = info.message().unwrap(); // The current implementation always returns Some - let file_line_col = (loc.file(), loc.line(), loc.column()); - rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), &file_line_col); + rust_panic_with_hook(&mut PanicPayload::new(msg), info.message(), loc); } /// This is the entry point of panicking for the non-format-string variants of @@ -386,19 +381,13 @@ pub fn begin_panic_handler(info: &PanicInfo<'_>) -> ! { // bloat at the call sites as much as possible #[cfg_attr(not(feature = "panic_immediate_abort"), inline(never))] #[cold] -pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u32)) -> ! { +#[track_caller] +pub fn begin_panic(msg: M, #[cfg(bootstrap)] _: &(&str, u32, u32)) -> ! { if cfg!(feature = "panic_immediate_abort") { unsafe { intrinsics::abort() } } - // Note that this should be the only allocation performed in this code path. - // Currently this means that panic!() on OOM will invoke this code path, - // but then again we're not really ready for panic on OOM anyway. If - // we do start doing this, then we should propagate this allocation to - // be performed in the parent of this thread instead of the thread that's - // panicking. - - rust_panic_with_hook(&mut PanicPayload::new(msg), None, file_line_col); + rust_panic_with_hook(&mut PanicPayload::new(msg), None, Location::caller()); struct PanicPayload { inner: Option, @@ -412,6 +401,11 @@ pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u3 unsafe impl BoxMeUp for PanicPayload { fn take_box(&mut self) -> *mut (dyn Any + Send) { + // Note that this should be the only allocation performed in this code path. Currently + // this means that panic!() on OOM will invoke this code path, but then again we're not + // really ready for panic on OOM anyway. If we do start doing this, then we should + // propagate this allocation to be performed in the parent of this thread instead of the + // thread that's panicking. let data = match self.inner.take() { Some(a) => Box::new(a) as Box, None => process::abort(), @@ -436,10 +430,8 @@ pub fn begin_panic(msg: M, file_line_col: &(&'static str, u32, u3 fn rust_panic_with_hook( payload: &mut dyn BoxMeUp, message: Option<&fmt::Arguments<'_>>, - file_line_col: &(&str, u32, u32), + location: &Location<'_>, ) -> ! { - let (file, line, col) = *file_line_col; - let panics = update_panic_count(1); // If this is the third nested call (e.g., panics == 2, this is 0-indexed), @@ -456,8 +448,7 @@ fn rust_panic_with_hook( } unsafe { - let location = Location::internal_constructor(file, line, col); - let mut info = PanicInfo::internal_constructor(message, &location); + let mut info = PanicInfo::internal_constructor(message, location); HOOK_LOCK.read(); match HOOK { // Some platforms (like wasm) know that printing to stderr won't ever actually diff --git a/src/test/mir-opt/retain-never-const.rs b/src/test/mir-opt/retain-never-const.rs index 04394dcdf1334..8e9bae8569f19 100644 --- a/src/test/mir-opt/retain-never-const.rs +++ b/src/test/mir-opt/retain-never-const.rs @@ -6,6 +6,7 @@ #![feature(const_panic)] #![feature(never_type)] +#![warn(const_err)] struct PrintName(T); diff --git a/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs b/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs index 8727c9d1ca655..74c6e501c9136 100644 --- a/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs +++ b/src/test/ui/extern/issue-64655-allow-unwind-when-calling-panic-directly.rs @@ -22,7 +22,7 @@ //[thin]compile-flags: -C lto=thin //[fat]compile-flags: -C lto=fat -#![feature(core_panic, panic_internals)] +#![feature(core_panic)] // (For some reason, reproducing the LTO issue requires pulling in std // explicitly this way.) @@ -50,9 +50,7 @@ fn main() { } let _guard = Droppable; - let s = "issue-64655-allow-unwind-when-calling-panic-directly.rs"; - let location = core::panic::Location::internal_constructor(s, 17, 4); - core::panicking::panic("???", &location); + core::panicking::panic("???"); }); let wait = handle.join(); diff --git a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs index 0a79aea376fbc..090e912c1d0ba 100644 --- a/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs +++ b/src/test/ui/rfc-2091-track-caller/caller-location-intrinsic.rs @@ -4,16 +4,16 @@ #[inline(never)] #[track_caller] -fn defeat_const_prop() -> &'static core::panic::Location<'static> { +fn codegen_caller_loc() -> &'static core::panic::Location<'static> { core::panic::Location::caller() } macro_rules! caller_location_from_macro { - () => (defeat_const_prop()); + () => (codegen_caller_loc()); } fn main() { - let loc = defeat_const_prop(); + let loc = codegen_caller_loc(); assert_eq!(loc.file(), file!()); assert_eq!(loc.line(), 16); assert_eq!(loc.column(), 15); diff --git a/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs new file mode 100644 index 0000000000000..1fb75ef35ffc1 --- /dev/null +++ b/src/test/ui/rfc-2091-track-caller/diverging-caller-location.rs @@ -0,0 +1,19 @@ +// run-fail + +//! This test ensures that `#[track_caller]` can be applied directly to diverging functions, as +//! the tracking issue says: https://github.com/rust-lang/rust/issues/47809#issue-292138490. +//! Because the annotated function must diverge and a panic keeps that faster than an infinite loop, +//! we don't inspect the location returned -- it would be difficult to distinguish between the +//! explicit panic and a failed assertion. That it compiles and runs is enough for this one. + +#![feature(track_caller)] + +#[track_caller] +fn doesnt_return() -> ! { + let _location = core::panic::Location::caller(); + panic!("huzzah"); +} + +fn main() { + doesnt_return(); +} diff --git a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs index 8436ee510a5bc..76a380ed3e30d 100644 --- a/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs +++ b/src/test/ui/rfc-2091-track-caller/track-caller-attribute.rs @@ -1,6 +1,6 @@ // run-pass -#![feature(const_fn, track_caller)] +#![feature(track_caller)] use std::panic::Location;