From d21e0118d0eefc8b0073fa47fa16699d37047abf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 11 Aug 2020 14:54:02 +0200 Subject: [PATCH] more precise span for erroneous consts during CTFE/Miri --- src/librustc_mir/interpret/eval_context.rs | 39 ++++++++++++------- src/librustc_mir/interpret/step.rs | 6 +-- src/test/ui/consts/const-err-multi.stderr | 18 ++++++--- .../ui/consts/const-eval/erroneous-const.rs | 4 +- .../consts/const-eval/erroneous-const.stderr | 10 ++--- 5 files changed, 46 insertions(+), 31 deletions(-) diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 32018ddb13f..b12df60d061 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -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, + /// If this is `Err`, we are not currently executing any particular statement in + /// this frame (can happen e.g. during frame initialziation, and during unwinding on + /// frames without cleanup code). + /// We basically abuse `Result` as `Either`. + pub(super) loc: Result, } /// What we store about a frame in an interpreter backtrace. @@ -189,11 +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 { - self.current_source_info().map(|si| si.span).unwrap_or(self.body.span) + match self.loc { + Ok(loc) => self.body.source_info(loc).span, + Err(span) => span, + } } } @@ -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: None, // `None` for errors generated before we start evaluating. + 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 so this constant. + // Avoiding doing this when evaluation succeeds. + self.frame_mut().loc = Err(span); + err + })?; } // Locals are initially uninitialized. @@ -683,9 +694,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } // done self.frame_mut().locals = locals; - self.frame_mut().loc = Some(mir::Location::START); - M::after_stack_push(self)?; + self.frame_mut().loc = Ok(mir::Location::START); info!("ENTERING({}) {}", self.frame_idx(), self.frame().instance); Ok(()) @@ -694,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. @@ -716,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) { - 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 @@ -744,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, } ); 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/test/ui/consts/const-err-multi.stderr b/src/test/ui/consts/const-err-multi.stderr index 171221553e2..a0c91ff6b54 100644 --- a/src/test/ui/consts/const-err-multi.stderr +++ b/src/test/ui/consts/const-err-multi.stderr @@ -13,22 +13,28 @@ LL | #![deny(const_err)] | ^^^^^^^^^ error: any use of this value will cause an error - --> $DIR/const-err-multi.rs:5:1 + --> $DIR/const-err-multi.rs:5:19 | LL | pub const B: i8 = A; - | ^^^^^^^^^^^^^^^^^^^^ referenced constant has errors + | ------------------^- + | | + | referenced constant has errors error: any use of this value will cause an error - --> $DIR/const-err-multi.rs:7:1 + --> $DIR/const-err-multi.rs:7:19 | LL | pub const C: u8 = A as u8; - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ referenced constant has errors + | ------------------^------- + | | + | referenced constant has errors error: any use of this value will cause an error - --> $DIR/const-err-multi.rs:9:1 + --> $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 7ff015d3b8c..3df491bf229 100644 --- a/src/test/ui/consts/const-eval/erroneous-const.rs +++ b/src/test/ui/consts/const-eval/erroneous-const.rs @@ -7,9 +7,9 @@ impl PrintName { //~^ WARN this operation will panic at runtime } -const fn no_codegen() { //~ERROR evaluation of constant value failed +const fn no_codegen() { if false { - let _ = PrintName::::VOID; + let _ = PrintName::::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 5ba113fc6a9..f06e2c33fd0 100644 --- a/src/test/ui/consts/const-eval/erroneous-const.stderr +++ b/src/test/ui/consts/const-eval/erroneous-const.stderr @@ -25,14 +25,10 @@ LL | #![warn(const_err, unconditional_panic)] | ^^^^^^^^^ error[E0080]: evaluation of constant value failed - --> $DIR/erroneous-const.rs:10:1 + --> $DIR/erroneous-const.rs:12:17 | -LL | / const fn no_codegen() { -LL | | if false { -LL | | let _ = PrintName::::VOID; -LL | | } -LL | | } - | |_^ referenced constant has errors +LL | let _ = PrintName::::VOID; + | ^^^^^^^^^^^^^^^^^^^^ referenced constant has errors error[E0080]: could not evaluate static initializer --> $DIR/erroneous-const.rs:16:22