use precise spans for recursive const evaluation

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -126,7 +126,9 @@ pub struct Frame<'mir, 'tcx, Tag: Provenance = AllocId, Extra = ()> {
     /// 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>,
+    ///
+    /// Needs to be public because ConstProp does unspeakable things to it.
+    pub loc: Result<mir::Location, Span>,
 }
 
 /// What we store about a frame in an interpreter backtrace.
@@ -320,6 +322,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> LayoutOfHelpers<'tcx> for InterpC
 
     #[inline]
     fn layout_tcx_at_span(&self) -> Span {
+        // Using the cheap root span for performance.
         self.tcx.span
     }
 
@@ -923,7 +926,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             self.param_env
         };
         let param_env = param_env.with_const();
-        let val = self.tcx.eval_to_allocation_raw(param_env.and(gid))?;
+        // Use a precise span for better cycle errors.
+        let val = self.tcx.at(self.cur_span()).eval_to_allocation_raw(param_env.and(gid))?;
         self.raw_const_to_mplace(val)
     }
 

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -169,7 +169,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::needs_drop => self.tcx.types.bool,
                     sym::type_id => self.tcx.types.u64,
                     sym::type_name => self.tcx.mk_static_str(),
-                    _ => bug!("already checked for nullary intrinsics"),
+                    _ => bug!(),
                 };
                 let val =
                     self.tcx.const_eval_global_id(self.param_env, gid, Some(self.tcx.span))?;
@@ -215,7 +215,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::add_with_overflow => BinOp::Add,
                     sym::sub_with_overflow => BinOp::Sub,
                     sym::mul_with_overflow => BinOp::Mul,
-                    _ => bug!("Already checked for int ops"),
+                    _ => bug!(),
                 };
                 self.binop_with_overflow(bin_op, &lhs, &rhs, dest)?;
             }
@@ -251,7 +251,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::unchecked_mul => BinOp::Mul,
                     sym::unchecked_div => BinOp::Div,
                     sym::unchecked_rem => BinOp::Rem,
-                    _ => bug!("Already checked for int ops"),
+                    _ => bug!(),
                 };
                 let (val, overflowed, _ty) = self.overflowing_binary_op(bin_op, &l, &r)?;
                 if overflowed {

compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs

@@ -70,7 +70,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             }
         }
 
-        bug!("no non-`#[track_caller]` frame found")
+        span_bug!(self.cur_span(), "no non-`#[track_caller]` frame found")
     }
 
     /// Allocate a `const core::panic::Location` with the provided filename and line/column numbers.

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -504,7 +504,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     throw_unsup!(ReadExternStatic(def_id));
                 }
 
-                (self.tcx.eval_static_initializer(def_id)?, Some(def_id))
+                // Use a precise span for better cycle errors.
+                (self.tcx.at(self.cur_span()).eval_static_initializer(def_id)?, Some(def_id))
             }
         };
         M::before_access_global(*self.tcx, &self.machine, id, alloc, def_id, is_write)?;

compiler/rustc_const_eval/src/interpret/operator.rs

@@ -154,14 +154,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                 let result = match bin_op {
                     Shl => l.checked_shl(r).unwrap(),
                     Shr => l.checked_shr(r).unwrap(),
-                    _ => bug!("it has already been checked that this is a shift op"),
+                    _ => bug!(),
                 };
                 result as u128
             } else {
                 match bin_op {
                     Shl => l.checked_shl(r).unwrap(),
                     Shr => l.checked_shr(r).unwrap(),
-                    _ => bug!("it has already been checked that this is a shift op"),
+                    _ => bug!(),
                 }
             };
             let truncated = self.truncate(result, left_layout);

compiler/rustc_const_eval/src/interpret/step.rs

@@ -55,33 +55,32 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         };
         let basic_block = &self.body().basic_blocks()[loc.block];
 
-        let old_frames = self.frame_idx();
-
         if let Some(stmt) = basic_block.statements.get(loc.statement_index) {
-            assert_eq!(old_frames, self.frame_idx());
+            let old_frames = self.frame_idx();
             self.statement(stmt)?;
+            // Make sure we are not updating `statement_index` of the wrong frame.
+            assert_eq!(old_frames, self.frame_idx());
+            // Advance the program counter.
+            self.frame_mut().loc.as_mut().unwrap().statement_index += 1;
             return Ok(true);
         }
 
         M::before_terminator(self)?;
 
         let terminator = basic_block.terminator();
-        assert_eq!(old_frames, self.frame_idx());
         self.terminator(terminator)?;
         Ok(true)
     }
 
     /// Runs the interpretation logic for the given `mir::Statement` at the current frame and
-    /// statement counter. This also moves the statement counter forward.
+    /// statement counter.
+    ///
+    /// This does NOT move the statement counter forward, the caller has to do that!
     pub fn statement(&mut self, stmt: &mir::Statement<'tcx>) -> InterpResult<'tcx> {
         info!("{:?}", stmt);
 
         use rustc_middle::mir::StatementKind::*;
 
-        // Some statements (e.g., box) push new stack frames.
-        // We have to record the stack frame number *before* executing the statement.
-        let frame_idx = self.frame_idx();
-
         match &stmt.kind {
             Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?,
 
@@ -144,7 +143,6 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Nop => {}
         }
 
-        self.stack_mut()[frame_idx].loc.as_mut().unwrap().statement_index += 1;
         Ok(())
     }
 
@@ -300,6 +298,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         Ok(())
     }
 
+    /// Evaluate the given terminator. Will also adjust the stack frame and statement position accordingly.
     fn terminator(&mut self, terminator: &mir::Terminator<'tcx>) -> InterpResult<'tcx> {
         info!("{:?}", terminator.kind);
 

compiler/rustc_middle/src/mir/interpret/queries.rs

@@ -3,9 +3,9 @@ use super::{ErrorHandled, EvalToConstValueResult, GlobalId};
 use crate::mir;
 use crate::ty::fold::TypeFoldable;
 use crate::ty::subst::InternalSubsts;
-use crate::ty::{self, TyCtxt};
+use crate::ty::{self, query::TyCtxtAt, TyCtxt};
 use rustc_hir::def_id::DefId;
-use rustc_span::Span;
+use rustc_span::{Span, DUMMY_SP};
 
 impl<'tcx> TyCtxt<'tcx> {
     /// Evaluates a constant without providing any substitutions. This is useful to evaluate consts
@@ -86,14 +86,25 @@ impl<'tcx> TyCtxt<'tcx> {
         }
     }
 
+    /// Evaluate a static's initializer, returning the allocation of the initializer's memory.
+    #[inline(always)]
+    pub fn eval_static_initializer(
+        self,
+        def_id: DefId,
+    ) -> Result<mir::ConstAllocation<'tcx>, ErrorHandled> {
+        self.at(DUMMY_SP).eval_static_initializer(def_id)
+    }
+}
+
+impl<'tcx> TyCtxtAt<'tcx> {
     /// Evaluate a static's initializer, returning the allocation of the initializer's memory.
     pub fn eval_static_initializer(
         self,
         def_id: DefId,
     ) -> Result<mir::ConstAllocation<'tcx>, ErrorHandled> {
         trace!("eval_static_initializer: Need to compute {:?}", def_id);
         assert!(self.is_static(def_id));
-        let instance = ty::Instance::mono(self, def_id);
+        let instance = ty::Instance::mono(*self, def_id);
         let gid = GlobalId { instance, promoted: None };
         self.eval_to_allocation(gid, ty::ParamEnv::reveal_all())
     }
@@ -109,7 +120,9 @@ impl<'tcx> TyCtxt<'tcx> {
         let raw_const = self.eval_to_allocation_raw(param_env.and(gid))?;
         Ok(self.global_alloc(raw_const.alloc_id).unwrap_memory())
     }
+}
 
+impl<'tcx> TyCtxt<'tcx> {
     /// Destructure a type-level constant ADT or array into its variant index and its field values.
     /// Panics if the destructuring fails, use `try_destructure_const` for fallible version.
     pub fn destructure_const(

compiler/rustc_mir_transform/src/const_prop_lint.rs

@@ -437,10 +437,12 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         source_info.scope.lint_root(self.source_scopes)
     }
 
-    fn use_ecx<F, T>(&mut self, f: F) -> Option<T>
+    fn use_ecx<F, T>(&mut self, source_info: SourceInfo, f: F) -> Option<T>
     where
         F: FnOnce(&mut Self) -> InterpResult<'tcx, T>,
     {
+        // Overwrite the PC -- whatever the interpreter does to it does not make any sense anyway.
+        self.ecx.frame_mut().loc = Err(source_info.span);
         match f(self) {
             Ok(val) => Some(val),
             Err(error) => {
@@ -501,17 +503,17 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
     }
 
     /// Returns the value, if any, of evaluating `place`.
-    fn eval_place(&mut self, place: Place<'tcx>) -> Option<OpTy<'tcx>> {
+    fn eval_place(&mut self, place: Place<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
         trace!("eval_place(place={:?})", place);
-        self.use_ecx(|this| this.ecx.eval_place_to_op(place, None))
+        self.use_ecx(source_info, |this| this.ecx.eval_place_to_op(place, None))
     }
 
     /// Returns the value, if any, of evaluating `op`. Calls upon `eval_constant`
     /// or `eval_place`, depending on the variant of `Operand` used.
     fn eval_operand(&mut self, op: &Operand<'tcx>, source_info: SourceInfo) -> Option<OpTy<'tcx>> {
         match *op {
             Operand::Constant(ref c) => self.eval_constant(c, source_info),
-            Operand::Move(place) | Operand::Copy(place) => self.eval_place(place),
+            Operand::Move(place) | Operand::Copy(place) => self.eval_place(place, source_info),
         }
     }
 
@@ -537,7 +539,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         arg: &Operand<'tcx>,
         source_info: SourceInfo,
     ) -> Option<()> {
-        if let (val, true) = self.use_ecx(|this| {
+        if let (val, true) = self.use_ecx(source_info, |this| {
             let val = this.ecx.read_immediate(&this.ecx.eval_operand(arg, None)?)?;
             let (_res, overflow, _ty) = this.ecx.overflowing_unary_op(op, &val)?;
             Ok((val, overflow))
@@ -564,8 +566,12 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
         right: &Operand<'tcx>,
         source_info: SourceInfo,
     ) -> Option<()> {
-        let r = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(right, None)?));
-        let l = self.use_ecx(|this| this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?));
+        let r = self.use_ecx(source_info, |this| {
+            this.ecx.read_immediate(&this.ecx.eval_operand(right, None)?)
+        });
+        let l = self.use_ecx(source_info, |this| {
+            this.ecx.read_immediate(&this.ecx.eval_operand(left, None)?)
+        });
         // Check for exceeding shifts *even if* we cannot evaluate the LHS.
         if op == BinOp::Shr || op == BinOp::Shl {
             let r = r?;
@@ -602,7 +608,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
 
         if let (Some(l), Some(r)) = (&l, &r) {
             // The remaining operators are handled through `overflowing_binary_op`.
-            if self.use_ecx(|this| {
+            if self.use_ecx(source_info, |this| {
                 let (_res, overflow, _ty) = this.ecx.overflowing_binary_op(op, l, r)?;
                 Ok(overflow)
             })? {
@@ -690,7 +696,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> {
             return None;
         }
 
-        self.use_ecx(|this| this.ecx.eval_rvalue_into_place(rvalue, place))
+        self.use_ecx(source_info, |this| this.ecx.eval_rvalue_into_place(rvalue, place))
     }
 }
 
@@ -890,7 +896,10 @@ impl<'tcx> Visitor<'tcx> for ConstPropagator<'_, 'tcx> {
                 StatementKind::SetDiscriminant { ref place, .. } => {
                     match self.ecx.machine.can_const_prop[place.local] {
                         ConstPropMode::FullConstProp | ConstPropMode::OnlyInsideOwnBlock => {
-                            if self.use_ecx(|this| this.ecx.statement(statement)).is_some() {
+                            if self
+                                .use_ecx(source_info, |this| this.ecx.statement(statement))
+                                .is_some()
+                            {
                                 trace!("propped discriminant into {:?}", place);
                             } else {
                                 Self::remove_const(&mut self.ecx, place.local);

src/test/ui/consts/recursive-zst-static.default.stderr

@@ -5,10 +5,10 @@ LL | static FOO: () = FOO;
    | ^^^^^^^^^^^^^^^^^^^^^
    |
 note: ...which requires const-evaluating + checking `FOO`...
-  --> $DIR/recursive-zst-static.rs:10:1
+  --> $DIR/recursive-zst-static.rs:10:18
    |
 LL | static FOO: () = FOO;
-   | ^^^^^^^^^^^^^^^^^^^^^
+   |                  ^^^
    = note: ...which again requires const-evaluating + checking `FOO`, completing the cycle
    = note: cycle used when running analysis passes on this crate
 

src/test/ui/consts/recursive-zst-static.unleash.stderr

@@ -5,10 +5,10 @@ LL | static FOO: () = FOO;
    | ^^^^^^^^^^^^^^^^^^^^^
    |
 note: ...which requires const-evaluating + checking `FOO`...
-  --> $DIR/recursive-zst-static.rs:10:1
+  --> $DIR/recursive-zst-static.rs:10:18
    |
 LL | static FOO: () = FOO;
-   | ^^^^^^^^^^^^^^^^^^^^^
+   |                  ^^^
    = note: ...which again requires const-evaluating + checking `FOO`, completing the cycle
    = note: cycle used when running analysis passes on this crate
 

src/test/ui/consts/write-to-static-mut-in-static.stderr

@@ -11,10 +11,10 @@ LL | pub static mut C: u32 = unsafe { C = 1; 0 };
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: ...which requires const-evaluating + checking `C`...
-  --> $DIR/write-to-static-mut-in-static.rs:5:1
+  --> $DIR/write-to-static-mut-in-static.rs:5:34
    |
 LL | pub static mut C: u32 = unsafe { C = 1; 0 };
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                                  ^^^^^
    = note: ...which again requires const-evaluating + checking `C`, completing the cycle
    = note: cycle used when running analysis passes on this crate
 

src/test/ui/recursion/recursive-static-definition.stderr

@@ -5,10 +5,10 @@ LL | pub static FOO: u32 = FOO;
    | ^^^^^^^^^^^^^^^^^^^^^^^^^^
    |
 note: ...which requires const-evaluating + checking `FOO`...
-  --> $DIR/recursive-static-definition.rs:1:1
+  --> $DIR/recursive-static-definition.rs:1:23
    |
 LL | pub static FOO: u32 = FOO;
-   | ^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |                       ^^^
    = note: ...which again requires const-evaluating + checking `FOO`, completing the cycle
    = note: cycle used when running analysis passes on this crate