Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support pretty printing of invalid constants #94020

Merged
merged 1 commit into from
Feb 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 18 additions & 20 deletions compiler/rustc_const_eval/src/const_eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ use rustc_middle::{
use rustc_span::{source_map::DUMMY_SP, symbol::Symbol};

use crate::interpret::{
intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, MPlaceTy, MemPlaceMeta, Scalar,
intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, InterpResult, MPlaceTy,
MemPlaceMeta, Scalar,
};

mod error;
Expand Down Expand Up @@ -132,42 +133,39 @@ fn const_to_valtree_inner<'tcx>(
}
}

/// This function uses `unwrap` copiously, because an already validated constant
/// must have valid fields and can thus never fail outside of compiler bugs. However, it is
/// invoked from the pretty printer, where it can receive enums with no variants and e.g.
/// `read_discriminant` needs to be able to handle that.
pub(crate) fn destructure_const<'tcx>(
/// This function should never fail for validated constants. However, it is also invoked from the
/// pretty printer which might attempt to format invalid constants and in that case it might fail.
pub(crate) fn try_destructure_const<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
val: ty::Const<'tcx>,
) -> mir::DestructuredConst<'tcx> {
) -> InterpResult<'tcx, mir::DestructuredConst<'tcx>> {
trace!("destructure_const: {:?}", val);
let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false);
let op = ecx.const_to_op(val, None).unwrap();
let op = ecx.const_to_op(val, None)?;

// We go to `usize` as we cannot allocate anything bigger anyway.
let (field_count, variant, down) = match val.ty().kind() {
ty::Array(_, len) => (usize::try_from(len.eval_usize(tcx, param_env)).unwrap(), None, op),
ty::Adt(def, _) if def.variants.is_empty() => {
return mir::DestructuredConst { variant: None, fields: &[] };
}
ty::Adt(def, _) => {
let variant = ecx.read_discriminant(&op).unwrap().1;
let down = ecx.operand_downcast(&op, variant).unwrap();
let variant = ecx.read_discriminant(&op)?.1;
let down = ecx.operand_downcast(&op, variant)?;
(def.variants[variant].fields.len(), Some(variant), down)
}
ty::Tuple(substs) => (substs.len(), None, op),
_ => bug!("cannot destructure constant {:?}", val),
};

let fields_iter = (0..field_count).map(|i| {
let field_op = ecx.operand_field(&down, i).unwrap();
let val = op_to_const(&ecx, &field_op);
ty::Const::from_value(tcx, val, field_op.layout.ty)
});
let fields = tcx.arena.alloc_from_iter(fields_iter);
let fields = (0..field_count)
.map(|i| {
let field_op = ecx.operand_field(&down, i)?;
let val = op_to_const(&ecx, &field_op);
Ok(ty::Const::from_value(tcx, val, field_op.layout.ty))
})
.collect::<InterpResult<'tcx, Vec<_>>>()?;
let fields = tcx.arena.alloc_from_iter(fields);

mir::DestructuredConst { variant, fields }
Ok(mir::DestructuredConst { variant, fields })
}

pub(crate) fn deref_const<'tcx>(
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ pub fn provide(providers: &mut Providers) {
providers.eval_to_const_value_raw = const_eval::eval_to_const_value_raw_provider;
providers.eval_to_allocation_raw = const_eval::eval_to_allocation_raw_provider;
providers.const_caller_location = const_eval::const_caller_location;
providers.destructure_const = |tcx, param_env_and_value| {
providers.try_destructure_const = |tcx, param_env_and_value| {
let (param_env, value) = param_env_and_value.into_parts();
const_eval::destructure_const(tcx, param_env, value)
const_eval::try_destructure_const(tcx, param_env, value).ok()
};
providers.const_to_valtree = |tcx, param_env_and_value| {
let (param_env, raw) = param_env_and_value.into_parts();
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/mir/interpret/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,4 +98,12 @@ 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())
}

/// Destructure a constant ADT or array into its variant index and its field values.
pub fn destructure_const(
self,
param_env_and_val: ty::ParamEnvAnd<'tcx, ty::Const<'tcx>>,
) -> mir::DestructuredConst<'tcx> {
self.try_destructure_const(param_env_and_val).unwrap()
}
}
8 changes: 5 additions & 3 deletions compiler/rustc_middle/src/query/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -924,10 +924,12 @@ rustc_queries! {
}

/// Destructure a constant ADT or array into its variant index and its
/// field values.
query destructure_const(
/// field values or return `None` if constant is invalid.
///
/// Use infallible `TyCtxt::destructure_const` when you know that constant is valid.
query try_destructure_const(
key: ty::ParamEnvAnd<'tcx, ty::Const<'tcx>>
) -> mir::DestructuredConst<'tcx> {
) -> Option<mir::DestructuredConst<'tcx>> {
desc { "destructure constant" }
remap_env_constness
}
Expand Down
16 changes: 12 additions & 4 deletions compiler/rustc_middle/src/ty/print/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1459,10 +1459,18 @@ pub trait PrettyPrinter<'tcx>:
// 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_param_types_or_consts() => {
let contents =
self.tcx().destructure_const(ty::ParamEnv::reveal_all().and(
self.tcx().mk_const(ty::ConstS { val: ty::ConstKind::Value(ct), ty }),
));
let Some(contents) = self.tcx().try_destructure_const(
ty::ParamEnv::reveal_all()
.and(self.tcx().mk_const(ty::ConstS { val: ty::ConstKind::Value(ct), ty })),
) else {
// Fall back to debug pretty printing for invalid constants.
p!(write("{:?}", ct));
if print_ty {
p!(": ", print(ty));
}
return Ok(self);
};

let fields = contents.fields.iter().copied();

match *ty.kind() {
Expand Down
55 changes: 55 additions & 0 deletions src/test/mir-opt/const_prop/invalid_constant.main.ConstProp.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
- // MIR for `main` before ConstProp
+ // MIR for `main` after ConstProp

fn main() -> () {
let mut _0: (); // return place in scope 0 at $DIR/invalid_constant.rs:15:11: 15:11
let _1: std::option::Option<()>; // in scope 0 at $DIR/invalid_constant.rs:16:5: 16:12
let mut _2: std::option::Option<std::option::Option<()>>; // in scope 0 at $DIR/invalid_constant.rs:16:7: 16:11
scope 1 (inlined f) { // at $DIR/invalid_constant.rs:16:5: 16:12
debug x => _2; // in scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
let mut _3: isize; // in scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
let _4: std::option::Option<()>; // in scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
scope 2 {
debug y => _4; // in scope 2 at $DIR/invalid_constant.rs:16:5: 16:12
}
}

bb0: {
discriminant(_2) = 0; // scope 0 at $DIR/invalid_constant.rs:16:7: 16:11
- _3 = discriminant(_2); // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
- switchInt(move _3) -> [0_isize: bb3, otherwise: bb2]; // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
+ _3 = const 0_isize; // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this change?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's just the optimization changing the mir. This is a diff of a diff

+ switchInt(const 0_isize) -> [0_isize: bb3, otherwise: bb2]; // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
}

bb1: {
nop; // scope 0 at $DIR/invalid_constant.rs:15:11: 17:2
return; // scope 0 at $DIR/invalid_constant.rs:17:2: 17:2
}

bb2: {
- _4 = ((_2 as Some).0: std::option::Option<()>); // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
- _1 = _4; // scope 2 at $DIR/invalid_constant.rs:16:5: 16:12
+ _4 = const Scalar(0x02): Option::<()>; // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
+ // ty::Const
+ // + ty: std::option::Option<()>
+ // + val: Value(Scalar(0x02))
+ // mir::Constant
+ // + span: $DIR/invalid_constant.rs:16:5: 16:12
+ // + literal: Const { ty: std::option::Option<()>, val: Value(Scalar(0x02)) }
+ _1 = const Scalar(0x02): Option::<()>; // scope 2 at $DIR/invalid_constant.rs:16:5: 16:12
+ // ty::Const
+ // + ty: std::option::Option<()>
+ // + val: Value(Scalar(0x02))
+ // mir::Constant
+ // + span: $DIR/invalid_constant.rs:16:5: 16:12
+ // + literal: Const { ty: std::option::Option<()>, val: Value(Scalar(0x02)) }
goto -> bb1; // scope 0 at $DIR/invalid_constant.rs:10:20: 10:21
}

bb3: {
discriminant(_1) = 0; // scope 1 at $DIR/invalid_constant.rs:16:5: 16:12
goto -> bb1; // scope 0 at $DIR/invalid_constant.rs:9:17: 9:21
}
}

17 changes: 17 additions & 0 deletions src/test/mir-opt/const_prop/invalid_constant.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
// Verify that we can pretty print invalid constant introduced
// by constant propagation. Regression test for issue #93688.
//
// compile-flags: -Copt-level=0 -Zinline-mir

#[inline(always)]
pub fn f(x: Option<Option<()>>) -> Option<()> {
match x {
None => None,
Some(y) => y,
}
}

// EMIT_MIR invalid_constant.main.ConstProp.diff
fn main() {
f(None);
}