Auto merge of rust-lang#113291 - oli-obk:pretty_print_mir_const, r=RalfJung

Specialize `try_destructure_mir_constant` for its sole user (pretty printing)

We can't remove the query, as we need to invoke it from rustc_middle, but can only implement it in mir interpretation/const eval.

r? `@RalfJung` for a first round.

While we could move all the logic into pretty printing, that would end up duplicating a bit of code with const eval, which doesn't seem great either.

Author: bors

compiler/rustc_const_eval/src/const_eval/mod.rs

@@ -1,12 +1,10 @@
 // Not in interpret to make sure we do not use private implementation details
 
 use crate::errors::MaxNumNodesInConstErr;
-use crate::interpret::{
-    intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, InterpResult, Scalar,
-};
+use crate::interpret::{intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, Scalar};
 use rustc_middle::mir;
 use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId};
-use rustc_middle::ty::{self, TyCtxt};
+use rustc_middle::ty::{self, Ty, TyCtxt};
 use rustc_span::{source_map::DUMMY_SP, symbol::Symbol};
 
 mod error;
@@ -87,23 +85,24 @@ pub(crate) fn eval_to_valtree<'tcx>(
 }
 
 #[instrument(skip(tcx), level = "debug")]
-pub(crate) fn try_destructure_mir_constant<'tcx>(
+pub(crate) fn try_destructure_mir_constant_for_diagnostics<'tcx>(
     tcx: TyCtxt<'tcx>,
-    param_env: ty::ParamEnv<'tcx>,
-    val: mir::ConstantKind<'tcx>,
-) -> InterpResult<'tcx, mir::DestructuredConstant<'tcx>> {
+    val: ConstValue<'tcx>,
+    ty: Ty<'tcx>,
+) -> Option<mir::DestructuredConstant<'tcx>> {
+    let param_env = ty::ParamEnv::reveal_all();
     let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, CanAccessStatics::No);
-    let op = ecx.eval_mir_constant(&val, None, None)?;
+    let op = ecx.const_val_to_op(val, ty, None).ok()?;
 
     // We go to `usize` as we cannot allocate anything bigger anyway.
-    let (field_count, variant, down) = match val.ty().kind() {
+    let (field_count, variant, down) = match ty.kind() {
         ty::Array(_, len) => (len.eval_target_usize(tcx, param_env) as usize, None, op),
         ty::Adt(def, _) if def.variants().is_empty() => {
-            throw_ub!(Unreachable)
+            return None;
         }
         ty::Adt(def, _) => {
-            let variant = ecx.read_discriminant(&op)?.1;
-            let down = ecx.operand_downcast(&op, variant)?;
+            let variant = ecx.read_discriminant(&op).ok()?.1;
+            let down = ecx.operand_downcast(&op, variant).ok()?;
             (def.variants()[variant].fields.len(), Some(variant), down)
         }
         ty::Tuple(substs) => (substs.len(), None, op),
@@ -112,12 +111,12 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
 
     let fields_iter = (0..field_count)
         .map(|i| {
-            let field_op = ecx.operand_field(&down, i)?;
+            let field_op = ecx.operand_field(&down, i).ok()?;
             let val = op_to_const(&ecx, &field_op);
-            Ok(mir::ConstantKind::Val(val, field_op.layout.ty))
+            Some((val, field_op.layout.ty))
         })
-        .collect::<InterpResult<'tcx, Vec<_>>>()?;
+        .collect::<Option<Vec<_>>>()?;
     let fields = tcx.arena.alloc_from_iter(fields_iter);
 
-    Ok(mir::DestructuredConstant { variant, fields })
+    Some(mir::DestructuredConstant { variant, fields })
 }

compiler/rustc_const_eval/src/interpret/operand.rs

@@ -106,7 +106,7 @@ impl<Prov: Provenance> std::fmt::Display for ImmTy<'_, Prov> {
                     // Just print the ptr value. `pretty_print_const_scalar_ptr` would also try to
                     // print what is points to, which would fail since it has no access to the local
                     // memory.
-                    cx.pretty_print_const_pointer(ptr, ty, true)
+                    cx.pretty_print_const_pointer(ptr, ty)
                 }
             }
         }
@@ -633,7 +633,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
         }
     }
 
-    pub(super) fn const_val_to_op(
+    pub(crate) fn const_val_to_op(
         &self,
         val_val: ConstValue<'tcx>,
         ty: Ty<'tcx>,

compiler/rustc_const_eval/src/lib.rs

@@ -52,10 +52,8 @@ pub fn provide(providers: &mut Providers) {
         let (param_env, raw) = param_env_and_value.into_parts();
         const_eval::eval_to_valtree(tcx, param_env, raw)
     };
-    providers.try_destructure_mir_constant = |tcx, param_env_and_value| {
-        let (param_env, value) = param_env_and_value.into_parts();
-        const_eval::try_destructure_mir_constant(tcx, param_env, value).ok()
-    };
+    providers.try_destructure_mir_constant_for_diagnostics =
+        |tcx, (cv, ty)| const_eval::try_destructure_mir_constant_for_diagnostics(tcx, cv, ty);
     providers.valtree_to_const_val = |tcx, (ty, valtree)| {
         const_eval::valtree_to_const_value(tcx, ty::ParamEnv::empty().and(ty), valtree)
     };

compiler/rustc_middle/src/mir/mod.rs

@@ -2776,7 +2776,7 @@ impl<'tcx> Display for ConstantKind<'tcx> {
     fn fmt(&self, fmt: &mut Formatter<'_>) -> fmt::Result {
         match *self {
             ConstantKind::Ty(c) => pretty_print_const(c, fmt, true),
-            ConstantKind::Val(val, ty) => pretty_print_const_value(val, ty, fmt, true),
+            ConstantKind::Val(val, ty) => pretty_print_const_value(val, ty, fmt),
             // FIXME(valtrees): Correctly print mir constants.
             ConstantKind::Unevaluated(..) => {
                 fmt.write_str("_")?;
@@ -2806,13 +2806,16 @@ fn pretty_print_byte_str(fmt: &mut Formatter<'_>, byte_str: &[u8]) -> fmt::Resul
     write!(fmt, "b\"{}\"", byte_str.escape_ascii())
 }
 
-fn comma_sep<'tcx>(fmt: &mut Formatter<'_>, elems: Vec<ConstantKind<'tcx>>) -> fmt::Result {
+fn comma_sep<'tcx>(
+    fmt: &mut Formatter<'_>,
+    elems: Vec<(ConstValue<'tcx>, Ty<'tcx>)>,
+) -> fmt::Result {
     let mut first = true;
-    for elem in elems {
+    for (ct, ty) in elems {
         if !first {
             fmt.write_str(", ")?;
         }
-        fmt.write_str(&format!("{}", elem))?;
+        pretty_print_const_value(ct, ty, fmt)?;
         first = false;
     }
     Ok(())
@@ -2823,7 +2826,6 @@ fn pretty_print_const_value<'tcx>(
     ct: ConstValue<'tcx>,
     ty: Ty<'tcx>,
     fmt: &mut Formatter<'_>,
-    print_ty: bool,
 ) -> fmt::Result {
     use crate::ty::print::PrettyPrinter;
 
@@ -2882,16 +2884,11 @@ fn pretty_print_const_value<'tcx>(
             // introducing ICEs (e.g. via `layout_of`) from missing bounds.
             // E.g. `transmute([0usize; 2]): (u8, *mut T)` needs to know `T: Sized`
             // to be able to destructure the tuple into `(0u8, *mut T)`
-            //
-            // FIXME(eddyb) for `--emit=mir`/`-Z dump-mir`, we should provide the
-            // correct `ty::ParamEnv` to allow printing *all* constant values.
             (_, ty::Array(..) | ty::Tuple(..) | ty::Adt(..)) if !ty.has_non_region_param() => {
                 let ct = tcx.lift(ct).unwrap();
                 let ty = tcx.lift(ty).unwrap();
-                if let Some(contents) = tcx.try_destructure_mir_constant(
-                    ty::ParamEnv::reveal_all().and(ConstantKind::Val(ct, ty)),
-                ) {
-                    let fields = contents.fields.to_vec();
+                if let Some(contents) = tcx.try_destructure_mir_constant_for_diagnostics((ct, ty)) {
+                    let fields: Vec<(ConstValue<'_>, Ty<'_>)> = contents.fields.to_vec();
                     match *ty.kind() {
                         ty::Array(..) => {
                             fmt.write_str("[")?;
@@ -2930,12 +2927,14 @@ fn pretty_print_const_value<'tcx>(
                                 None => {
                                     fmt.write_str(" {{ ")?;
                                     let mut first = true;
-                                    for (field_def, field) in iter::zip(&variant_def.fields, fields)
+                                    for (field_def, (ct, ty)) in
+                                        iter::zip(&variant_def.fields, fields)
                                     {
                                         if !first {
                                             fmt.write_str(", ")?;
                                         }
-                                        fmt.write_str(&format!("{}: {}", field_def.name, field))?;
+                                        write!(fmt, "{}: ", field_def.name)?;
+                                        pretty_print_const_value(ct, ty, fmt)?;
                                         first = false;
                                     }
                                     fmt.write_str(" }}")?;
@@ -2945,20 +2944,13 @@ fn pretty_print_const_value<'tcx>(
                         _ => unreachable!(),
                     }
                     return Ok(());
-                } else {
-                    // Fall back to debug pretty printing for invalid constants.
-                    fmt.write_str(&format!("{:?}", ct))?;
-                    if print_ty {
-                        fmt.write_str(&format!(": {}", ty))?;
-                    }
-                    return Ok(());
-                };
+                }
             }
             (ConstValue::Scalar(scalar), _) => {
                 let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
                 cx.print_alloc_ids = true;
                 let ty = tcx.lift(ty).unwrap();
-                cx = cx.pretty_print_const_scalar(scalar, ty, print_ty)?;
+                cx = cx.pretty_print_const_scalar(scalar, ty)?;
                 fmt.write_str(&cx.into_buffer())?;
                 return Ok(());
             }
@@ -2973,12 +2965,8 @@ fn pretty_print_const_value<'tcx>(
             // their fields instead of just dumping the memory.
             _ => {}
         }
-        // fallback
-        fmt.write_str(&format!("{:?}", ct))?;
-        if print_ty {
-            fmt.write_str(&format!(": {}", ty))?;
-        }
-        Ok(())
+        // Fall back to debug pretty printing for invalid constants.
+        write!(fmt, "{ct:?}: {ty}")
     })
 }
 

compiler/rustc_middle/src/mir/query.rs

@@ -1,6 +1,6 @@
 //! Values computed by queries that use MIR.
 
-use crate::mir::ConstantKind;
+use crate::mir::interpret::ConstValue;
 use crate::ty::{self, OpaqueHiddenType, Ty, TyCtxt};
 use rustc_data_structures::fx::FxIndexMap;
 use rustc_data_structures::unord::UnordSet;
@@ -444,7 +444,7 @@ impl<'tcx> ClosureOutlivesSubjectTy<'tcx> {
 #[derive(Copy, Clone, Debug, HashStable)]
 pub struct DestructuredConstant<'tcx> {
     pub variant: Option<VariantIdx>,
-    pub fields: &'tcx [ConstantKind<'tcx>],
+    pub fields: &'tcx [(ConstValue<'tcx>, Ty<'tcx>)],
 }
 
 /// Coverage information summarized from a MIR if instrumented for source code coverage (see

compiler/rustc_middle/src/query/keys.rs

@@ -2,6 +2,7 @@
 
 use crate::infer::canonical::Canonical;
 use crate::mir;
+use crate::mir::interpret::ConstValue;
 use crate::traits;
 use crate::ty::fast_reject::SimplifiedType;
 use crate::ty::layout::{TyAndLayout, ValidityRequirement};
@@ -333,6 +334,14 @@ impl<'tcx> Key for (ty::Const<'tcx>, FieldIdx) {
     }
 }
 
+impl<'tcx> Key for (ConstValue<'tcx>, Ty<'tcx>) {
+    type CacheSelector = DefaultCacheSelector<Self>;
+
+    fn default_span(&self, _: TyCtxt<'_>) -> Span {
+        DUMMY_SP
+    }
+}
+
 impl<'tcx> Key for mir::interpret::ConstAlloc<'tcx> {
     type CacheSelector = DefaultCacheSelector<Self>;
 

compiler/rustc_middle/src/query/mod.rs

@@ -1087,11 +1087,13 @@ rustc_queries! {
     }
 
     /// Tries to destructure an `mir::ConstantKind` ADT or array into its variant index
-    /// and its field values.
-    query try_destructure_mir_constant(
-        key: ty::ParamEnvAnd<'tcx, mir::ConstantKind<'tcx>>
+    /// and its field values. This should only be used for pretty printing.
+    query try_destructure_mir_constant_for_diagnostics(
+        key: (ConstValue<'tcx>, Ty<'tcx>)
     ) -> Option<mir::DestructuredConstant<'tcx>> {
         desc { "destructuring MIR constant"}
+        no_hash
+        eval_always
     }
 
     query const_caller_location(key: (rustc_span::Symbol, u32, u32)) -> ConstValue<'tcx> {

compiler/rustc_middle/src/ty/print/pretty.rs

@@ -1393,19 +1393,19 @@ pub trait PrettyPrinter<'tcx>:
         self,
         scalar: Scalar,
         ty: Ty<'tcx>,
-        print_ty: bool,
     ) -> Result<Self::Const, Self::Error> {
         match scalar {
-            Scalar::Ptr(ptr, _size) => self.pretty_print_const_scalar_ptr(ptr, ty, print_ty),
-            Scalar::Int(int) => self.pretty_print_const_scalar_int(int, ty, print_ty),
+            Scalar::Ptr(ptr, _size) => self.pretty_print_const_scalar_ptr(ptr, ty),
+            Scalar::Int(int) => {
+                self.pretty_print_const_scalar_int(int, ty, /* print_ty */ true)
+            }
         }
     }
 
     fn pretty_print_const_scalar_ptr(
         mut self,
         ptr: Pointer,
         ty: Ty<'tcx>,
-        print_ty: bool,
     ) -> Result<Self::Const, Self::Error> {
         define_scoped_cx!(self);
 
@@ -1459,7 +1459,7 @@ pub trait PrettyPrinter<'tcx>:
             _ => {}
         }
         // Any pointer values not covered by a branch above
-        self = self.pretty_print_const_pointer(ptr, ty, print_ty)?;
+        self = self.pretty_print_const_pointer(ptr, ty)?;
         Ok(self)
     }
 
@@ -1527,24 +1527,18 @@ pub trait PrettyPrinter<'tcx>:
     /// This is overridden for MIR printing because we only want to hide alloc ids from users, not
     /// from MIR where it is actually useful.
     fn pretty_print_const_pointer<Prov: Provenance>(
-        mut self,
+        self,
         _: Pointer<Prov>,
         ty: Ty<'tcx>,
-        print_ty: bool,
     ) -> Result<Self::Const, Self::Error> {
-        if print_ty {
-            self.typed_value(
-                |mut this| {
-                    this.write_str("&_")?;
-                    Ok(this)
-                },
-                |this| this.print_type(ty),
-                ": ",
-            )
-        } else {
-            self.write_str("&_")?;
-            Ok(self)
-        }
+        self.typed_value(
+            |mut this| {
+                this.write_str("&_")?;
+                Ok(this)
+            },
+            |this| this.print_type(ty),
+            ": ",
+        )
     }
 
     fn pretty_print_byte_str(mut self, byte_str: &'tcx [u8]) -> Result<Self::Const, Self::Error> {
@@ -2156,7 +2150,6 @@ impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
         self,
         p: Pointer<Prov>,
         ty: Ty<'tcx>,
-        print_ty: bool,
     ) -> Result<Self::Const, Self::Error> {
         let print = |mut this: Self| {
             define_scoped_cx!(this);
@@ -2167,11 +2160,7 @@ impl<'tcx> PrettyPrinter<'tcx> for FmtPrinter<'_, 'tcx> {
             }
             Ok(this)
         };
-        if print_ty {
-            self.typed_value(print, |this| this.print_type(ty), ": ")
-        } else {
-            print(self)
-        }
+        self.typed_value(print, |this| this.print_type(ty), ": ")
     }
 }
 

src/tools/clippy/clippy_utils/src/consts.rs

@@ -725,13 +725,14 @@ fn field_of_struct<'tcx>(
     result: mir::ConstantKind<'tcx>,
     field: &Ident,
 ) -> Option<mir::ConstantKind<'tcx>> {
-    if let Some(dc) = lcx.tcx.try_destructure_mir_constant(lcx.param_env.and(result))
+    if let mir::ConstantKind::Val(result, ty) = result
+        && let Some(dc) = lcx.tcx.try_destructure_mir_constant_for_diagnostics((result, ty))
         && let Some(dc_variant) = dc.variant
         && let Some(variant) = adt_def.variants().get(dc_variant)
         && let Some(field_idx) = variant.fields.iter().position(|el| el.name == field.name)
-        && let Some(dc_field) = dc.fields.get(field_idx)
+        && let Some(&(val, ty)) = dc.fields.get(field_idx)
     {
-        Some(*dc_field)
+        Some(mir::ConstantKind::Val(val, ty))
     }
     else {
         None