Auto merge of #75396 - RalfJung:miri-spans, r=oli-obk

Miri: improve spans of required_const failures

In https://github.com/rust-lang/rust/pull/75339 I added a loop evaluating all consts required by a function body. Unfortunately, if one of their evaluations fails, then the span used for that was that of the first statement in the function body, which happened to work form some existing test but is not sensible in general.

This PR changes it to point to the whole function instead, which is at least not wrong.

r? @oli-obk

11 files changed, 69 insertions, 56 deletions

diff --git a/src/librustc_mir/const_eval/machine.rs b/src/librustc_mir/const_eval/machine.rs
index 3a753a0edb6..2d0e68d5894 100644
--- a/src/librustc_mir/const_eval/machine.rs
+++ b/src/librustc_mir/const_eval/machine.rs
@@ -301,12 +301,16 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir,
         Ok(())
     }
 
-    fn after_stack_push(ecx: &mut InterpCx<'mir, 'tcx, Self>) -> InterpResult<'tcx> {
-        // Enforce stack size limit.
-        if !ecx.tcx.sess.recursion_limit().value_within_limit(ecx.stack().len()) {
+    #[inline(always)]
+    fn init_frame_extra(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx>,
+    ) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
+        // Enforce stack size limit. Add 1 because this is run before the new frame is pushed.
+        if !ecx.tcx.sess.recursion_limit().value_within_limit(ecx.stack().len() + 1) {
             throw_exhaust!(StackFrameLimitReached)
         } else {
-            Ok(())
+            Ok(frame)
         }
     }
 
diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs
index bcf2899b439..525da87463a 100644
--- a/src/librustc_mir/interpret/eval_context.rs
+++ b/src/librustc_mir/interpret/eval_context.rs
@@ -16,7 +16,7 @@ use rustc_middle::ty::layout::{self, TyAndLayout};
 use rustc_middle::ty::{
     self, query::TyCtxtAt, subst::SubstsRef, ParamEnv, Ty, TyCtxt, TypeFoldable,
 };
-use rustc_span::{source_map::DUMMY_SP, Pos, Span};
+use rustc_span::{Pos, Span};
 use rustc_target::abi::{Align, HasDataLayout, LayoutOf, Size, TargetDataLayout};
 
 use super::{
@@ -83,9 +83,11 @@ pub struct Frame<'mir, 'tcx, Tag = (), Extra = ()> {
     ////////////////////////////////////////////////////////////////////////////////
     // Current position within the function
     ////////////////////////////////////////////////////////////////////////////////
-    /// If this is `None`, we are unwinding and this function doesn't need any clean-up.
-    /// Just continue the same as with `Resume`.
-    pub loc: Option<mir::Location>,
+    /// If this is `Err`, we are not currently executing any particular statement in
+    /// this frame (can happen e.g. during frame initialization, and during unwinding on
+    /// frames without cleanup code).
+    /// We basically abuse `Result` as `Either`.
+    pub(super) loc: Result<mir::Location, Span>,
 }
 
 /// What we store about a frame in an interpreter backtrace.
@@ -189,7 +191,14 @@ impl<'mir, 'tcx, Tag> Frame<'mir, 'tcx, Tag> {
 impl<'mir, 'tcx, Tag, Extra> Frame<'mir, 'tcx, Tag, Extra> {
     /// Return the `SourceInfo` of the current instruction.
     pub fn current_source_info(&self) -> Option<&mir::SourceInfo> {
-        self.loc.map(|loc| self.body.source_info(loc))
+        self.loc.ok().map(|loc| self.body.source_info(loc))
+    }
+
+    pub fn current_span(&self) -> Span {
+        match self.loc {
+            Ok(loc) => self.body.source_info(loc).span,
+            Err(span) => span,
+        }
     }
 }
 
@@ -324,11 +333,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
     #[inline(always)]
     pub fn cur_span(&self) -> Span {
-        self.stack()
-            .last()
-            .and_then(|f| f.current_source_info())
-            .map(|si| si.span)
-            .unwrap_or(self.tcx.span)
+        self.stack().last().map(|f| f.current_span()).unwrap_or(self.tcx.span)
     }
 
     #[inline(always)]
@@ -640,7 +645,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         // first push a stack frame so we have access to the local substs
         let pre_frame = Frame {
             body,
-            loc: Some(mir::Location::START),
+            loc: Err(body.span), // Span used for errors caused during preamble.
             return_to_block,
             return_place,
             // empty local array, we fill it in below, after we are inside the stack frame and
@@ -654,9 +659,15 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
         // Make sure all the constants required by this frame evaluate successfully (post-monomorphization check).
         for const_ in &body.required_consts {
+            let span = const_.span;
             let const_ =
                 self.subst_from_current_frame_and_normalize_erasing_regions(const_.literal);
-            self.const_to_op(const_, None)?;
+            self.const_to_op(const_, None).map_err(|err| {
+                // If there was an error, set the span of the current frame to this constant.
+                // Avoiding doing this when evaluation succeeds.
+                self.frame_mut().loc = Err(span);
+                err
+            })?;
         }
 
         // Locals are initially uninitialized.
@@ -683,8 +694,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
         // done
         self.frame_mut().locals = locals;
-
         M::after_stack_push(self)?;
+        self.frame_mut().loc = Ok(mir::Location::START);
         info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance);
 
         Ok(())
@@ -693,7 +704,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// Jump to the given block.
     #[inline]
     pub fn go_to_block(&mut self, target: mir::BasicBlock) {
-        self.frame_mut().loc = Some(mir::Location { block: target, statement_index: 0 });
+        self.frame_mut().loc = Ok(mir::Location { block: target, statement_index: 0 });
     }
 
     /// *Return* to the given `target` basic block.
@@ -715,7 +726,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     /// If `target` is `None`, that indicates the function does not need cleanup during
     /// unwinding, and we will just keep propagating that upwards.
     pub fn unwind_to_block(&mut self, target: Option<mir::BasicBlock>) {
-        self.frame_mut().loc = target.map(|block| mir::Location { block, statement_index: 0 });
+        self.frame_mut().loc = match target {
+            Some(block) => Ok(mir::Location { block, statement_index: 0 }),
+            None => Err(self.frame_mut().body.span),
+        };
     }
 
     /// Pops the current frame from the stack, deallocating the
@@ -743,8 +757,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         assert_eq!(
             unwinding,
             match self.frame().loc {
-                None => true,
-                Some(loc) => self.body().basic_blocks()[loc.block].is_cleanup,
+                Ok(loc) => self.body().basic_blocks()[loc.block].is_cleanup,
+                Err(_) => true,
             }
         );
 
@@ -920,14 +934,13 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
     pub fn generate_stacktrace(&self) -> Vec<FrameInfo<'tcx>> {
         let mut frames = Vec::new();
         for frame in self.stack().iter().rev() {
-            let source_info = frame.current_source_info();
-            let lint_root = source_info.and_then(|source_info| {
+            let lint_root = frame.current_source_info().and_then(|source_info| {
                 match &frame.body.source_scopes[source_info.scope].local_data {
                     mir::ClearCrossCrate::Set(data) => Some(data.lint_root),
                     mir::ClearCrossCrate::Clear => None,
                 }
             });
-            let span = source_info.map_or(DUMMY_SP, |source_info| source_info.span);
+            let span = frame.current_span();
 
             frames.push(FrameInfo { span, instance: frame.instance, lint_root });
         }
diff --git a/src/librustc_mir/interpret/intrinsics/caller_location.rs b/src/librustc_mir/interpret/intrinsics/caller_location.rs
index 9adef8c43c7..fb3a670714b 100644
--- a/src/librustc_mir/interpret/intrinsics/caller_location.rs
+++ b/src/librustc_mir/interpret/intrinsics/caller_location.rs
@@ -30,8 +30,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             // Assert that there is always such a frame.
             .unwrap();
         // Assert that the frame we look at is actually executing code currently
-        // (`current_source_info` is None when we are unwinding and the frame does
-        // not require cleanup).
+        // (`loc` is `Err` when we are unwinding and the frame does not require cleanup).
         let loc = frame.loc.unwrap();
         // If this is a `Call` terminator, use the `fn_span` instead.
         let block = &frame.body.basic_blocks()[loc.block];
diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs
index 634f3c96e6c..5cab4ba37e3 100644
--- a/src/librustc_mir/interpret/machine.rs
+++ b/src/librustc_mir/interpret/machine.rs
@@ -409,12 +409,4 @@ pub macro compile_time_machine(<$mir: lifetime, $tcx: lifetime>) {
     ) -> Self::PointerTag {
         ()
     }
-
-    #[inline(always)]
-    fn init_frame_extra(
-        _ecx: &mut InterpCx<$mir, $tcx, Self>,
-        frame: Frame<$mir, $tcx>,
-    ) -> InterpResult<$tcx, Frame<$mir, $tcx>> {
-        Ok(frame)
-    }
 }
diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs
index 93624c32d30..adecee3f7cb 100644
--- a/src/librustc_mir/interpret/step.rs
+++ b/src/librustc_mir/interpret/step.rs
@@ -47,8 +47,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
 
         let loc = match self.frame().loc {
-            Some(loc) => loc,
-            None => {
+            Ok(loc) => loc,
+            Err(_) => {
                 // We are unwinding and this fn has no cleanup code.
                 // Just go on unwinding.
                 trace!("unwinding: skipping frame");
@@ -283,7 +283,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
 
         self.eval_terminator(terminator)?;
         if !self.stack().is_empty() {
-            if let Some(loc) = self.frame().loc {
+            if let Ok(loc) = self.frame().loc {
                 info!("// executing {:?}", loc.block);
             }
         }
diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs
index 6cf296f2a3f..3b39d5f66b7 100644
--- a/src/librustc_mir/transform/const_prop.rs
+++ b/src/librustc_mir/transform/const_prop.rs
@@ -282,6 +282,14 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for ConstPropMachine<'mir, 'tcx>
     }
 
     #[inline(always)]
+    fn init_frame_extra(
+        _ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        frame: Frame<'mir, 'tcx>,
+    ) -> InterpResult<'tcx, Frame<'mir, 'tcx>> {
+        Ok(frame)
+    }
+
+    #[inline(always)]
     fn stack(
         ecx: &'a InterpCx<'mir, 'tcx, Self>,
     ) -> &'a [Frame<'mir, 'tcx, Self::PointerTag, Self::FrameExtra>] {
diff --git a/src/test/ui/consts/const-err-multi.stderr b/src/test/ui/consts/const-err-multi.stderr
index 4ac4a8754d3..a0c91ff6b54 100644
--- a/src/test/ui/consts/const-err-multi.stderr
+++ b/src/test/ui/consts/const-err-multi.stderr
@@ -24,17 +24,17 @@ error: any use of this value will cause an error
   --> $DIR/const-err-multi.rs:7:19
    |
 LL | pub const C: u8 = A as u8;
-   | ------------------^^^^^^^-
+   | ------------------^-------
    |                   |
    |                   referenced constant has errors
 
 error: any use of this value will cause an error
-  --> $DIR/const-err-multi.rs:9:19
+  --> $DIR/const-err-multi.rs:9:24
    |
 LL | pub const D: i8 = 50 - A;
-   | ------------------^^^^^^-
-   |                   |
-   |                   referenced constant has errors
+   | -----------------------^-
+   |                        |
+   |                        referenced constant has errors
 
 error: aborting due to 4 previous errors
 
diff --git a/src/test/ui/consts/const-eval/erroneous-const.rs b/src/test/ui/consts/const-eval/erroneous-const.rs
index 93c4e9372e8..3df491bf229 100644
--- a/src/test/ui/consts/const-eval/erroneous-const.rs
+++ b/src/test/ui/consts/const-eval/erroneous-const.rs
@@ -8,8 +8,8 @@ impl<T> PrintName<T> {
 }
 
 const fn no_codegen<T>() {
-    if false { //~ERROR evaluation of constant value failed
-        let _ = PrintName::<T>::VOID;
+    if false {
+        let _ = PrintName::<T>::VOID; //~ERROR evaluation of constant value failed
     }
 }
 
diff --git a/src/test/ui/consts/const-eval/erroneous-const.stderr b/src/test/ui/consts/const-eval/erroneous-const.stderr
index da7e7247d50..f06e2c33fd0 100644
--- a/src/test/ui/consts/const-eval/erroneous-const.stderr
+++ b/src/test/ui/consts/const-eval/erroneous-const.stderr
@@ -25,12 +25,10 @@ LL | #![warn(const_err, unconditional_panic)]
    |         ^^^^^^^^^
 
 error[E0080]: evaluation of constant value failed
-  --> $DIR/erroneous-const.rs:11:5
+  --> $DIR/erroneous-const.rs:12:17
    |
-LL | /     if false {
-LL | |         let _ = PrintName::<T>::VOID;
-LL | |     }
-   | |_____^ referenced constant has errors
+LL |         let _ = PrintName::<T>::VOID;
+   |                 ^^^^^^^^^^^^^^^^^^^^ referenced constant has errors
 
 error[E0080]: could not evaluate static initializer
   --> $DIR/erroneous-const.rs:16:22
diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.rs b/src/test/ui/consts/uninhabited-const-issue-61744.rs
index 55f42d84f9c..15436f9c1b2 100644
--- a/src/test/ui/consts/uninhabited-const-issue-61744.rs
+++ b/src/test/ui/consts/uninhabited-const-issue-61744.rs
@@ -1,11 +1,11 @@
 // build-fail
 
 pub const unsafe fn fake_type<T>() -> T {
-    hint_unreachable() //~ ERROR evaluation of constant value failed
+    hint_unreachable()
 }
 
 pub const unsafe fn hint_unreachable() -> ! {
-    fake_type()
+    fake_type() //~ ERROR evaluation of constant value failed
 }
 
 trait Const {
diff --git a/src/test/ui/consts/uninhabited-const-issue-61744.stderr b/src/test/ui/consts/uninhabited-const-issue-61744.stderr
index fc908b2b222..024f9782d4a 100644
--- a/src/test/ui/consts/uninhabited-const-issue-61744.stderr
+++ b/src/test/ui/consts/uninhabited-const-issue-61744.stderr
@@ -1,11 +1,9 @@
 error[E0080]: evaluation of constant value failed
-  --> $DIR/uninhabited-const-issue-61744.rs:4:5
+  --> $DIR/uninhabited-const-issue-61744.rs:8:5
    |
 LL |     hint_unreachable()
-   |     ^^^^^^^^^^^^^^^^^^
+   |     ------------------
    |     |
-   |     reached the configured maximum number of stack frames
-   |     inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
    |     inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
    |     inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
    |     inside `fake_type::<!>` at $DIR/uninhabited-const-issue-61744.rs:4:5
@@ -72,8 +70,9 @@ LL |     hint_unreachable()
    |     inside `fake_type::<i32>` at $DIR/uninhabited-const-issue-61744.rs:4:5
 ...
 LL |     fake_type()
-   |     -----------
+   |     ^^^^^^^^^^^
    |     |
+   |     reached the configured maximum number of stack frames
    |     inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5
    |     inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5
    |     inside `hint_unreachable` at $DIR/uninhabited-const-issue-61744.rs:8:5