Rollup merge of #104317 - RalfJung:ctfe-error-reporting, r=oli-obk

cleanup and dedupe CTFE and Miri error reporting

It looks like most of the time, this error raised from const_prop_lint is just redundant -- it duplicates the error reported when evaluating the const-eval query. This lets us make `ConstEvalErr` private to the const_eval module which I think is a good step.

The Miri change mostly replaces a `match` by `if let`, and dedupes the "this error is impossible in Miri" checks.

r? ``@oli-obk``
Fixes #75461

Author: matthiaskrgr

compiler/rustc_codegen_cranelift/src/constant.rs

@@ -47,7 +47,7 @@ pub(crate) fn check_constants(fx: &mut FunctionCx<'_, '_, '_>) -> bool {
         if let Err(err) = fx.tcx.const_eval_resolve(ParamEnv::reveal_all(), unevaluated, None) {
             all_constants_ok = false;
             match err {
-                ErrorHandled::Reported(_) | ErrorHandled::Linted => {
+                ErrorHandled::Reported(_) => {
                     fx.tcx.sess.span_err(constant.span, "erroneous constant encountered");
                 }
                 ErrorHandled::TooGeneric => {

compiler/rustc_codegen_ssa/src/mir/mod.rs

@@ -189,7 +189,7 @@ pub fn codegen_mir<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(
             all_consts_ok = false;
             match err {
                 // errored or at least linted
-                ErrorHandled::Reported(_) | ErrorHandled::Linted => {}
+                ErrorHandled::Reported(_) => {}
                 ErrorHandled::TooGeneric => {
                     span_bug!(const_.span, "codegen encountered polymorphic constant: {:?}", err)
                 }

compiler/rustc_const_eval/src/const_eval/error.rs

@@ -55,7 +55,7 @@ impl Error for ConstEvalErrKind {}
 /// When const-evaluation errors, this type is constructed with the resulting information,
 /// and then used to emit the error as a lint or hard error.
 #[derive(Debug)]
-pub struct ConstEvalErr<'tcx> {
+pub(super) struct ConstEvalErr<'tcx> {
     pub span: Span,
     pub error: InterpError<'tcx>,
     pub stacktrace: Vec<FrameInfo<'tcx>>,
@@ -82,8 +82,8 @@ impl<'tcx> ConstEvalErr<'tcx> {
         ConstEvalErr { error: error.into_kind(), stacktrace, span }
     }
 
-    pub fn report_as_error(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled {
-        self.struct_error(tcx, message, |_| {})
+    pub(super) fn report(&self, tcx: TyCtxtAt<'tcx>, message: &str) -> ErrorHandled {
+        self.report_decorated(tcx, message, |_| {})
     }
 
     /// Create a diagnostic for this const eval error.
@@ -95,7 +95,7 @@ impl<'tcx> ConstEvalErr<'tcx> {
     /// If `lint_root.is_some()` report it as a lint, else report it as a hard error.
     /// (Except that for some errors, we ignore all that -- see `must_error` below.)
     #[instrument(skip(self, tcx, decorate), level = "debug")]
-    pub fn struct_error(
+    pub(super) fn report_decorated(
         &self,
         tcx: TyCtxtAt<'tcx>,
         message: &str,

compiler/rustc_const_eval/src/const_eval/eval_queries.rs

@@ -255,7 +255,7 @@ pub fn eval_to_const_value_raw_provider<'tcx>(
         return eval_nullary_intrinsic(tcx, key.param_env, def_id, substs).map_err(|error| {
             let span = tcx.def_span(def_id);
             let error = ConstEvalErr { error: error.into_kind(), stacktrace: vec![], span };
-            error.report_as_error(tcx.at(span), "could not evaluate nullary intrinsic")
+            error.report(tcx.at(span), "could not evaluate nullary intrinsic")
         });
     }
 
@@ -333,7 +333,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
                 }
             };
 
-            Err(err.report_as_error(ecx.tcx.at(err.span), &msg))
+            Err(err.report(ecx.tcx.at(err.span), &msg))
         }
         Ok(mplace) => {
             // Since evaluation had no errors, validate the resulting constant.
@@ -358,7 +358,7 @@ pub fn eval_to_allocation_raw_provider<'tcx>(
             if let Err(error) = validation {
                 // Validation failed, report an error. This is always a hard error.
                 let err = ConstEvalErr::new(&ecx, error, None);
-                Err(err.struct_error(
+                Err(err.report_decorated(
                     ecx.tcx,
                     "it is undefined behavior to use this value",
                     |diag| {

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -103,7 +103,7 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
 ) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> {
     trace!("destructure_mir_constant: {:?}", val);
     let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false);
-    let op = ecx.const_to_op(&val, None)?;
+    let op = ecx.eval_mir_constant(&val, None, None)?;
 
     // We go to `usize` as we cannot allocate anything bigger anyway.
     let (field_count, variant, down) = match val.ty().kind() {
@@ -139,7 +139,7 @@ pub(crate) fn deref_mir_constant<'tcx>(
     val: mir::ConstantKind<'tcx>,
 ) -> mir::ConstantKind<'tcx> {
     let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false);
-    let op = ecx.const_to_op(&val, None).unwrap();
+    let op = ecx.eval_mir_constant(&val, None, None).unwrap();
     let mplace = ecx.deref_operand(&op).unwrap();
     if let Some(alloc_id) = mplace.ptr.provenance {
         assert_eq!(

compiler/rustc_const_eval/src/interpret/eval_context.rs

@@ -5,7 +5,7 @@ use std::mem;
 use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData};
 use rustc_index::vec::IndexVec;
 use rustc_middle::mir;
-use rustc_middle::mir::interpret::{InterpError, InvalidProgramInfo};
+use rustc_middle::mir::interpret::{ErrorHandled, InterpError, InvalidProgramInfo};
 use rustc_middle::ty::layout::{
     self, FnAbiError, FnAbiOfHelpers, FnAbiRequest, LayoutError, LayoutOf, LayoutOfHelpers,
     TyAndLayout,
@@ -696,12 +696,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         for ct in &body.required_consts {
             let span = ct.span;
             let ct = self.subst_from_current_frame_and_normalize_erasing_regions(ct.literal)?;
-            self.const_to_op(&ct, 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
-            })?;
+            self.eval_mir_constant(&ct, Some(span), None)?;
         }
 
         // Most locals are initially dead.
@@ -912,9 +907,32 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         Ok(())
     }
 
-    pub fn eval_to_allocation(
+    /// Call a query that can return `ErrorHandled`. If `span` is `Some`, point to that span when an error occurs.
+    pub fn ctfe_query<T>(
+        &self,
+        span: Option<Span>,
+        query: impl FnOnce(TyCtxtAt<'tcx>) -> Result<T, ErrorHandled>,
+    ) -> InterpResult<'tcx, T> {
+        // Use a precise span for better cycle errors.
+        query(self.tcx.at(span.unwrap_or_else(|| self.cur_span()))).map_err(|err| {
+            match err {
+                ErrorHandled::Reported(err) => {
+                    if let Some(span) = span {
+                        // To make it easier to figure out where this error comes from, also add a note at the current location.
+                        self.tcx.sess.span_note_without_error(span, "erroneous constant used");
+                    }
+                    err_inval!(AlreadyReported(err))
+                }
+                ErrorHandled::TooGeneric => err_inval!(TooGeneric),
+            }
+            .into()
+        })
+    }
+
+    pub fn eval_global(
         &self,
         gid: GlobalId<'tcx>,
+        span: Option<Span>,
     ) -> InterpResult<'tcx, MPlaceTy<'tcx, M::Provenance>> {
         // For statics we pick `ParamEnv::reveal_all`, because statics don't have generics
         // and thus don't care about the parameter environment. While we could just use
@@ -927,8 +945,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             self.param_env
         };
         let param_env = param_env.with_const();
-        // Use a precise span for better cycle errors.
-        let val = self.tcx.at(self.cur_span()).eval_to_allocation_raw(param_env.and(gid))?;
+        let val = self.ctfe_query(span, |tcx| tcx.eval_to_allocation_raw(param_env.and(gid)))?;
         self.raw_const_to_mplace(val)
     }
 

compiler/rustc_const_eval/src/interpret/intrinsics.rs

@@ -177,8 +177,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     sym::type_name => self.tcx.mk_static_str(),
                     _ => bug!(),
                 };
-                let val =
-                    self.tcx.const_eval_global_id(self.param_env, gid, Some(self.tcx.span))?;
+                let val = self.ctfe_query(None, |tcx| {
+                    tcx.const_eval_global_id(self.param_env, gid, Some(tcx.span))
+                })?;
                 let val = self.const_val_to_op(val, ty, Some(dest.layout))?;
                 self.copy_op(&val, dest, /*allow_transmute*/ false)?;
             }

compiler/rustc_const_eval/src/interpret/memory.rs

@@ -501,8 +501,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
                     throw_unsup!(ReadExternStatic(def_id));
                 }
 
-                // Use a precise span for better cycle errors.
-                (self.tcx.at(self.cur_span()).eval_static_initializer(def_id)?, Some(def_id))
+                // We don't give a span -- statics don't need that, they cannot be generic or associated.
+                let val = self.ctfe_query(None, |tcx| tcx.eval_static_initializer(def_id))?;
+                (val, Some(def_id))
             }
         };
         M::before_access_global(*self.tcx, &self.machine, id, alloc, def_id, is_write)?;

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -4,8 +4,9 @@
 use rustc_hir::def::Namespace;
 use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout};
 use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter};
-use rustc_middle::ty::{ConstInt, Ty};
+use rustc_middle::ty::{ConstInt, Ty, ValTree};
 use rustc_middle::{mir, ty};
+use rustc_span::Span;
 use rustc_target::abi::{self, Abi, Align, HasDataLayout, Size, TagEncoding};
 use rustc_target::abi::{VariantIdx, Variants};
 
@@ -527,14 +528,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
             Copy(place) | Move(place) => self.eval_place_to_op(place, layout)?,
 
             Constant(ref constant) => {
-                let val =
+                let c =
                     self.subst_from_current_frame_and_normalize_erasing_regions(constant.literal)?;
 
                 // This can still fail:
                 // * During ConstProp, with `TooGeneric` or since the `required_consts` were not all
                 //   checked yet.
                 // * During CTFE, since promoteds in `const`/`static` initializer bodies can fail.
-                self.const_to_op(&val, layout)?
+                self.eval_mir_constant(&c, Some(constant.span), layout)?
             }
         };
         trace!("{:?}: {:?}", mir_op, *op);
@@ -549,9 +550,35 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         ops.iter().map(|op| self.eval_operand(op, None)).collect()
     }
 
-    pub fn const_to_op(
+    fn eval_ty_constant(
+        &self,
+        val: ty::Const<'tcx>,
+        span: Option<Span>,
+    ) -> InterpResult<'tcx, ValTree<'tcx>> {
+        Ok(match val.kind() {
+            ty::ConstKind::Param(_) | ty::ConstKind::Placeholder(..) => {
+                throw_inval!(TooGeneric)
+            }
+            ty::ConstKind::Error(reported) => {
+                throw_inval!(AlreadyReported(reported))
+            }
+            ty::ConstKind::Unevaluated(uv) => {
+                let instance = self.resolve(uv.def, uv.substs)?;
+                let cid = GlobalId { instance, promoted: None };
+                self.ctfe_query(span, |tcx| tcx.eval_to_valtree(self.param_env.and(cid)))?
+                    .unwrap_or_else(|| bug!("unable to create ValTree for {uv:?}"))
+            }
+            ty::ConstKind::Bound(..) | ty::ConstKind::Infer(..) => {
+                span_bug!(self.cur_span(), "unexpected ConstKind in ctfe: {val:?}")
+            }
+            ty::ConstKind::Value(valtree) => valtree,
+        })
+    }
+
+    pub fn eval_mir_constant(
         &self,
         val: &mir::ConstantKind<'tcx>,
+        span: Option<Span>,
         layout: Option<TyAndLayout<'tcx>>,
     ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> {
         // FIXME(const_prop): normalization needed b/c const prop lint in
@@ -563,44 +590,20 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         let val = self.tcx.normalize_erasing_regions(self.param_env, *val);
         match val {
             mir::ConstantKind::Ty(ct) => {
-                match ct.kind() {
-                    ty::ConstKind::Param(_) | ty::ConstKind::Placeholder(..) => {
-                        throw_inval!(TooGeneric)
-                    }
-                    ty::ConstKind::Error(reported) => {
-                        throw_inval!(AlreadyReported(reported))
-                    }
-                    ty::ConstKind::Unevaluated(uv) => {
-                        // NOTE: We evaluate to a `ValTree` here as a check to ensure
-                        // we're working with valid constants, even though we never need it.
-                        let instance = self.resolve(uv.def, uv.substs)?;
-                        let cid = GlobalId { instance, promoted: None };
-                        let _valtree = self
-                            .tcx
-                            .eval_to_valtree(self.param_env.and(cid))?
-                            .unwrap_or_else(|| bug!("unable to create ValTree for {uv:?}"));
-
-                        Ok(self.eval_to_allocation(cid)?.into())
-                    }
-                    ty::ConstKind::Bound(..) | ty::ConstKind::Infer(..) => {
-                        span_bug!(self.cur_span(), "unexpected ConstKind in ctfe: {ct:?}")
-                    }
-                    ty::ConstKind::Value(valtree) => {
-                        let ty = ct.ty();
-                        let const_val = self.tcx.valtree_to_const_val((ty, valtree));
-                        self.const_val_to_op(const_val, ty, layout)
-                    }
-                }
+                let ty = ct.ty();
+                let valtree = self.eval_ty_constant(ct, span)?;
+                let const_val = self.tcx.valtree_to_const_val((ty, valtree));
+                self.const_val_to_op(const_val, ty, layout)
             }
             mir::ConstantKind::Val(val, ty) => self.const_val_to_op(val, ty, layout),
             mir::ConstantKind::Unevaluated(uv, _) => {
                 let instance = self.resolve(uv.def, uv.substs)?;
-                Ok(self.eval_to_allocation(GlobalId { instance, promoted: uv.promoted })?.into())
+                Ok(self.eval_global(GlobalId { instance, promoted: uv.promoted }, span)?.into())
             }
         }
     }
 
-    pub(crate) fn const_val_to_op(
+    pub(super) fn const_val_to_op(
         &self,
         val_val: ConstValue<'tcx>,
         ty: Ty<'tcx>,

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

@@ -16,8 +16,6 @@ pub enum ErrorHandled {
     /// Already reported an error for this evaluation, and the compilation is
     /// *guaranteed* to fail. Warnings/lints *must not* produce `Reported`.
     Reported(ErrorGuaranteed),
-    /// Already emitted a lint for this evaluation.
-    Linted,
     /// Don't emit an error, the evaluation failed because the MIR was generic
     /// and the substs didn't fully monomorphize it.
     TooGeneric,
@@ -89,18 +87,6 @@ fn print_backtrace(backtrace: &Backtrace) {
     eprintln!("\n\nAn error occurred in miri:\n{}", backtrace);
 }
 
-impl From<ErrorHandled> for InterpErrorInfo<'_> {
-    fn from(err: ErrorHandled) -> Self {
-        match err {
-            ErrorHandled::Reported(ErrorGuaranteed { .. }) | ErrorHandled::Linted => {
-                err_inval!(ReferencedConstant)
-            }
-            ErrorHandled::TooGeneric => err_inval!(TooGeneric),
-        }
-        .into()
-    }
-}
-
 impl From<ErrorGuaranteed> for InterpErrorInfo<'_> {
     fn from(err: ErrorGuaranteed) -> Self {
         InterpError::InvalidProgram(InvalidProgramInfo::AlreadyReported(err)).into()
@@ -138,9 +124,6 @@ impl<'tcx> From<InterpError<'tcx>> for InterpErrorInfo<'tcx> {
 pub enum InvalidProgramInfo<'tcx> {
     /// Resolution can fail if we are in a too generic context.
     TooGeneric,
-    /// Cannot compute this constant because it depends on another one
-    /// which already produced an error.
-    ReferencedConstant,
     /// Abort in case errors are already reported.
     AlreadyReported(ErrorGuaranteed),
     /// An error occurred during layout computation.
@@ -158,9 +141,11 @@ impl fmt::Display for InvalidProgramInfo<'_> {
         use InvalidProgramInfo::*;
         match self {
             TooGeneric => write!(f, "encountered overly generic constant"),
-            ReferencedConstant => write!(f, "referenced constant has errors"),
             AlreadyReported(ErrorGuaranteed { .. }) => {
-                write!(f, "encountered constants with type errors, stopping evaluation")
+                write!(
+                    f,
+                    "an error has already been reported elsewhere (this sould not usually be printed)"
+                )
             }
             Layout(ref err) => write!(f, "{err}"),
             FnAbiAdjustForForeignAbi(ref err) => write!(f, "{err}"),

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

@@ -175,6 +175,8 @@ impl<'tcx> TyCtxt<'tcx> {
 
 impl<'tcx> TyCtxtAt<'tcx> {
     /// Evaluate a static's initializer, returning the allocation of the initializer's memory.
+    ///
+    /// The span is entirely ignored here, but still helpful for better query cycle errors.
     pub fn eval_static_initializer(
         self,
         def_id: DefId,
@@ -187,6 +189,8 @@ impl<'tcx> TyCtxtAt<'tcx> {
     }
 
     /// Evaluate anything constant-like, returning the allocation of the final memory.
+    ///
+    /// The span is entirely ignored here, but still helpful for better query cycle errors.
     fn eval_to_allocation(
         self,
         gid: GlobalId<'tcx>,

compiler/rustc_middle/src/mir/mod.rs

@@ -2304,7 +2304,7 @@ impl<'tcx> ConstantKind<'tcx> {
                 // FIXME: We might want to have a `try_eval`-like function on `Unevaluated`
                 match tcx.const_eval_resolve(param_env, uneval, None) {
                     Ok(val) => Self::Val(val, ty),
-                    Err(ErrorHandled::TooGeneric | ErrorHandled::Linted) => self,
+                    Err(ErrorHandled::TooGeneric) => self,
                     Err(ErrorHandled::Reported(guar)) => {
                         Self::Ty(tcx.const_error_with_guaranteed(ty, guar))
                     }

compiler/rustc_middle/src/ty/adt.rs

@@ -463,9 +463,7 @@ impl<'tcx> AdtDef<'tcx> {
             }
             Err(err) => {
                 let msg = match err {
-                    ErrorHandled::Reported(_) | ErrorHandled::Linted => {
-                        "enum discriminant evaluation failed"
-                    }
+                    ErrorHandled::Reported(_) => "enum discriminant evaluation failed",
                     ErrorHandled::TooGeneric => "enum discriminant depends on generics",
                 };
                 tcx.sess.delay_span_bug(tcx.def_span(expr_did), msg);