Rollup merge of #153611 - RalfJung:interp-error-strings, r=oli-obk

interpret: go back to regular string interpolation for error messages

Using the translatable diagnostic infrastructure adds a whole lot of boilerplate which isn't actually useful for const-eval errors, so let's get rid of it. This effectively reverts #111677. That PR effectively added 1000 lines and this PR only removes around 600 -- the difference is caused by (a) keeping some of the types around for validation, where we can use them to share error strings and to trigger the extra help for pointer byte shenanigans during CTFE, and (b) this not being a full revert of #111677; I am not touching diagnostics outside the interpreter such as all the const-checking code which also got converted to fluent in the same PR.

The last commit does something similar for `LayoutError`, which also helps deduplicate a bunch of error strings. I can make that into a separate PR if you prefer.

r? @oli-obk
Fixes #113117
Fixes #116764
Fixes #112618

Author: JonathanBrouwer

compiler/rustc_abi/src/lib.rs

@@ -1000,20 +1000,6 @@ pub enum AlignFromBytesError {
     TooLarge(u64),
 }
 
-impl AlignFromBytesError {
-    pub fn diag_ident(self) -> &'static str {
-        match self {
-            Self::NotPowerOfTwo(_) => "not_power_of_two",
-            Self::TooLarge(_) => "too_large",
-        }
-    }
-
-    pub fn align(self) -> u64 {
-        let (Self::NotPowerOfTwo(align) | Self::TooLarge(align)) = self;
-        align
-    }
-}
-
 impl fmt::Debug for AlignFromBytesError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         fmt::Display::fmt(self, f)
@@ -1023,8 +1009,8 @@ impl fmt::Debug for AlignFromBytesError {
 impl fmt::Display for AlignFromBytesError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
-            AlignFromBytesError::NotPowerOfTwo(align) => write!(f, "`{align}` is not a power of 2"),
-            AlignFromBytesError::TooLarge(align) => write!(f, "`{align}` is too large"),
+            AlignFromBytesError::NotPowerOfTwo(align) => write!(f, "{align} is not a power of 2"),
+            AlignFromBytesError::TooLarge(align) => write!(f, "{align} is too large"),
         }
     }
 }

compiler/rustc_codegen_gcc/src/context.rs

@@ -538,7 +538,7 @@ impl<'gcc, 'tcx> LayoutOfHelpers<'tcx> for CodegenCx<'gcc, 'tcx> {
         | LayoutError::InvalidSimd { .. }
         | LayoutError::ReferencesError(_) = err
         {
-            self.tcx.dcx().emit_fatal(respan(span, err.into_diagnostic()))
+            self.tcx.dcx().span_fatal(span, err.to_string())
         } else {
             self.tcx.dcx().emit_fatal(ssa_errors::FailedToGetLayout { span, ty, err })
         }

compiler/rustc_codegen_llvm/src/context.rs

@@ -1135,7 +1135,7 @@ impl<'tcx> LayoutOfHelpers<'tcx> for CodegenCx<'_, 'tcx> {
         | LayoutError::ReferencesError(_)
         | LayoutError::InvalidSimd { .. } = err
         {
-            self.tcx.dcx().emit_fatal(Spanned { span, node: err.into_diagnostic() })
+            self.tcx.dcx().span_fatal(span, err.to_string())
         } else {
             self.tcx.dcx().emit_fatal(ssa_errors::FailedToGetLayout { span, ty, err })
         }

compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs

@@ -95,7 +95,7 @@ fn push_debuginfo_type_name<'tcx>(
                         // Computing the layout can still fail here, e.g. if the target architecture
                         // cannot represent the type. See
                         // https://github.com/rust-lang/rust/issues/94961.
-                        tcx.dcx().emit_fatal(e.into_diagnostic());
+                        tcx.dcx().fatal(e.to_string());
                     }
                 }
             } else {

compiler/rustc_const_eval/src/const_eval/dummy_machine.rs

@@ -25,17 +25,8 @@ pub macro throw_machine_stop_str($($tt:tt)*) {{
             write!(f, $($tt)*)
         }
     }
+    impl rustc_middle::mir::interpret::MachineStopType for Zst {}
 
-    impl rustc_middle::mir::interpret::MachineStopType for Zst {
-        fn diagnostic_message(&self) -> rustc_errors::DiagMessage {
-            self.to_string().into()
-        }
-
-        fn add_args(
-            self: Box<Self>,
-            _: &mut dyn FnMut(rustc_errors::DiagArgName, rustc_errors::DiagArgValue),
-        ) {}
-    }
     throw_machine_stop!(Zst)
 }}
 

compiler/rustc_const_eval/src/const_eval/error.rs

@@ -1,15 +1,17 @@
-use std::mem;
+use std::{fmt, mem};
 
-use rustc_errors::{Diag, DiagArgName, DiagArgValue, DiagMessage, IntoDiagArg};
+use rustc_errors::Diag;
 use rustc_middle::mir::AssertKind;
-use rustc_middle::mir::interpret::{AllocId, Provenance, ReportedErrorInfo, UndefinedBehaviorInfo};
+use rustc_middle::mir::interpret::{
+    AllocId, Provenance, ReportedErrorInfo, UndefinedBehaviorInfo, UnsupportedOpInfo,
+};
 use rustc_middle::query::TyCtxtAt;
 use rustc_middle::ty::ConstInt;
 use rustc_middle::ty::layout::LayoutError;
 use rustc_span::{Span, Symbol};
 
 use super::CompileTimeMachine;
-use crate::errors::{self, FrameNote, ReportErrorExt};
+use crate::errors::{self, FrameNote};
 use crate::interpret::{
     CtfeProvenance, ErrorHandled, Frame, InterpCx, InterpErrorInfo, InterpErrorKind,
     MachineStopType, Pointer, err_inval, err_machine_stop,
@@ -40,65 +42,49 @@ pub enum ConstEvalErrKind {
     ConstMakeGlobalWithOffset(Pointer<Option<CtfeProvenance>>),
 }
 
-impl MachineStopType for ConstEvalErrKind {
-    fn diagnostic_message(&self) -> DiagMessage {
+impl fmt::Display for ConstEvalErrKind {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         use ConstEvalErrKind::*;
-        use rustc_errors::msg;
-
         match self {
-            ConstAccessesMutGlobal => "constant accesses mutable global memory".into(),
+            ConstAccessesMutGlobal => write!(f, "constant accesses mutable global memory"),
             ModifiedGlobal => {
-                "modifying a static's initial value from another static's initializer".into()
+                write!(f, "modifying a static's initial value from another static's initializer")
             }
-            Panic { .. } => msg!("evaluation panicked: {$msg}"),
+            Panic { msg, .. } => write!(f, "evaluation panicked: {msg}"),
             RecursiveStatic => {
-                "encountered static that tried to access itself during initialization".into()
+                write!(f, "encountered static that tried to access itself during initialization")
             }
-            AssertFailure(x) => x.diagnostic_message(),
+            AssertFailure(x) => write!(f, "{x}"),
             WriteThroughImmutablePointer => {
-                msg!(
+                write!(
+                    f,
                     "writing through a pointer that was derived from a shared (immutable) reference"
                 )
             }
-            ConstMakeGlobalPtrAlreadyMadeGlobal { .. } => {
-                msg!("attempting to call `const_make_global` twice on the same allocation {$alloc}")
-            }
-            ConstMakeGlobalPtrIsNonHeap(_) => {
-                msg!(
-                    "pointer passed to `const_make_global` does not point to a heap allocation: {$ptr}"
+            ConstMakeGlobalPtrAlreadyMadeGlobal(alloc) => {
+                write!(
+                    f,
+                    "attempting to call `const_make_global` twice on the same allocation {alloc}"
                 )
             }
-            ConstMakeGlobalWithDanglingPtr(_) => {
-                msg!("pointer passed to `const_make_global` is dangling: {$ptr}")
-            }
-            ConstMakeGlobalWithOffset(_) => {
-                msg!("making {$ptr} global which does not point to the beginning of an object")
-            }
-        }
-    }
-    fn add_args(self: Box<Self>, adder: &mut dyn FnMut(DiagArgName, DiagArgValue)) {
-        use ConstEvalErrKind::*;
-        match *self {
-            RecursiveStatic
-            | ConstAccessesMutGlobal
-            | ModifiedGlobal
-            | WriteThroughImmutablePointer => {}
-            AssertFailure(kind) => kind.add_args(adder),
-            Panic { msg, .. } => {
-                adder("msg".into(), msg.into_diag_arg(&mut None));
+            ConstMakeGlobalPtrIsNonHeap(ptr) => {
+                write!(
+                    f,
+                    "pointer passed to `const_make_global` does not point to a heap allocation: {ptr}"
+                )
             }
-            ConstMakeGlobalPtrIsNonHeap(ptr)
-            | ConstMakeGlobalWithOffset(ptr)
-            | ConstMakeGlobalWithDanglingPtr(ptr) => {
-                adder("ptr".into(), format!("{ptr:?}").into_diag_arg(&mut None));
+            ConstMakeGlobalWithDanglingPtr(ptr) => {
+                write!(f, "pointer passed to `const_make_global` is dangling: {ptr}")
             }
-            ConstMakeGlobalPtrAlreadyMadeGlobal(alloc) => {
-                adder("alloc".into(), alloc.into_diag_arg(&mut None));
+            ConstMakeGlobalWithOffset(ptr) => {
+                write!(f, "making {ptr} global which does not point to the beginning of an object")
             }
         }
     }
 }
 
+impl MachineStopType for ConstEvalErrKind {}
+
 /// The errors become [`InterpErrorKind::MachineStop`] when being raised.
 impl<'tcx> Into<InterpErrorInfo<'tcx>> for ConstEvalErrKind {
     fn into(self) -> InterpErrorInfo<'tcx> {
@@ -207,14 +193,20 @@ where
         _ => {
             let (our_span, frames) = get_span_and_frames();
             let span = span.substitute_dummy(our_span);
-            let mut err = tcx.dcx().struct_span_err(our_span, error.diagnostic_message());
-            // We allow invalid programs in infallible promoteds since invalid layouts can occur
-            // anyway (e.g. due to size overflow). And we allow OOM as that can happen any time.
-            let allowed_in_infallible = matches!(
+            let mut err = tcx.dcx().struct_span_err(our_span, error.to_string());
+            if matches!(
                 error,
-                InterpErrorKind::ResourceExhaustion(_) | InterpErrorKind::InvalidProgram(_)
-            );
-
+                InterpErrorKind::UndefinedBehavior(UndefinedBehaviorInfo::ValidationError {
+                    ptr_bytes_warning: true,
+                    ..
+                }) | InterpErrorKind::Unsupported(
+                    UnsupportedOpInfo::ReadPointerAsInt(..)
+                        | UnsupportedOpInfo::ReadPartialPointer(..)
+                )
+            ) {
+                err.help("this code performed an operation that depends on the underlying bytes representing a pointer");
+                err.help("the absolute address of a pointer is not known at compile-time, so such operations are not supported");
+            }
             if let InterpErrorKind::UndefinedBehavior(UndefinedBehaviorInfo::InvalidUninitBytes(
                 Some((alloc_id, _access)),
             )) = error
@@ -229,7 +221,12 @@ where
                 err.subdiagnostic(raw_bytes);
             }
 
-            error.add_args(&mut err);
+            // We allow invalid programs in infallible promoteds since invalid layouts can occur
+            // anyway (e.g. due to size overflow). And we allow OOM as that can happen any time.
+            let allowed_in_infallible = matches!(
+                error,
+                InterpErrorKind::ResourceExhaustion(_) | InterpErrorKind::InvalidProgram(_)
+            );
 
             mk(&mut err, span, frames);
             let g = err.emit();

compiler/rustc_const_eval/src/const_eval/machine.rs

@@ -5,7 +5,6 @@ use std::hash::Hash;
 use rustc_abi::{Align, FIRST_VARIANT, Size};
 use rustc_ast::Mutability;
 use rustc_data_structures::fx::{FxHashMap, FxIndexMap, IndexEntry};
-use rustc_errors::msg;
 use rustc_hir::def_id::{DefId, LocalDefId};
 use rustc_hir::{self as hir, CRATE_HIR_ID, LangItem, find_attr};
 use rustc_middle::mir::AssertMessage;
@@ -24,7 +23,7 @@ use crate::interpret::{
     self, AllocId, AllocInit, AllocRange, ConstAllocation, CtfeProvenance, FnArg, Frame,
     GlobalAlloc, ImmTy, InterpCx, InterpResult, OpTy, PlaceTy, Pointer, RangeSet, Scalar,
     compile_time_machine, ensure_monomorphic_enough, err_inval, interp_ok, throw_exhaust,
-    throw_inval, throw_ub, throw_ub_custom, throw_unsup, throw_unsup_format,
+    throw_inval, throw_ub, throw_ub_format, throw_unsup, throw_unsup_format,
     type_implements_dyn_trait,
 };
 
@@ -489,18 +488,9 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
 
                 let align = match Align::from_bytes(align) {
                     Ok(a) => a,
-                    Err(err) => throw_ub_custom!(
-                        msg!(
-                            "invalid align passed to `{$name}`: {$align} is {$err_kind ->
-                                [not_power_of_two] not a power of 2
-                                [too_large] too large
-                                *[other] {\"\"}
-                            }"
-                        ),
-                        name = "const_allocate",
-                        err_kind = err.diag_ident(),
-                        align = err.align()
-                    ),
+                    Err(err) => {
+                        throw_ub_format!("invalid align passed to `const_allocate`: {err}")
+                    }
                 };
 
                 let ptr = ecx.allocate_ptr(
@@ -519,18 +509,9 @@ impl<'tcx> interpret::Machine<'tcx> for CompileTimeMachine<'tcx> {
                 let size = Size::from_bytes(size);
                 let align = match Align::from_bytes(align) {
                     Ok(a) => a,
-                    Err(err) => throw_ub_custom!(
-                        msg!(
-                            "invalid align passed to `{$name}`: {$align} is {$err_kind ->
-                                [not_power_of_two] not a power of 2
-                                [too_large] too large
-                                *[other] {\"\"}
-                            }"
-                        ),
-                        name = "const_deallocate",
-                        err_kind = err.diag_ident(),
-                        align = err.align()
-                    ),
+                    Err(err) => {
+                        throw_ub_format!("invalid align passed to `const_deallocate`: {err}")
+                    }
                 };
 
                 // If an allocation is created in an another const,