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.

Sign up for GitHub

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

Use valtrees as the type-system representation for constant values #96591

Merged
merged 11 commits into from
Jun 14, 2022
Merged

Use valtrees as the type-system representation for constant values #96591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
705d818
implement valtrees as the type-system representation for constant values
b-naber Feb 16, 2022
5c95a3d
fix clippy test failures
b-naber Jun 2, 2022
dbef6e4
address review
b-naber Jun 2, 2022
17323e0
manually bless 32-bit mir-opt tests
b-naber Jun 3, 2022
0a6815a
bless 32-bit ui tests
b-naber Jun 3, 2022
773d8b2
address review
b-naber Jun 3, 2022
90c4b94
fix wrong evaluation in clippy
b-naber Jun 3, 2022
8093db6
correctly create Scalar for meta info
b-naber Jun 4, 2022
060acc9
rebase
b-naber Jun 7, 2022
e14b34c
account for endianness in debuginfo for const args
b-naber Jun 14, 2022
15c1c06
rebase
b-naber Jun 14, 2022
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
9 changes: 5 additions & 4 deletions compiler/rustc_codegen_cranelift/src/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ pub(crate) fn codegen_constant<'tcx>(
ConstantKind::Val(val, ty) => return codegen_const_value(fx, val, ty),
};
let const_val = match const_.kind() {
ConstKind::Value(const_val) => const_val,
ConstKind::Value(valtree) => fx.tcx.valtree_to_const_val((const_.ty(), valtree)),
ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted })
if fx.tcx.is_static(def.did) =>
{
Expand Down Expand Up @@ -468,9 +468,10 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
) -> Option<ConstValue<'tcx>> {
match operand {
Operand::Constant(const_) => match const_.literal {
ConstantKind::Ty(const_) => {
fx.monomorphize(const_).eval(fx.tcx, ParamEnv::reveal_all()).kind().try_to_value()
}
ConstantKind::Ty(const_) => fx
.monomorphize(const_)
.eval_for_mir(fx.tcx, ParamEnv::reveal_all())
.try_to_value(fx.tcx),
ConstantKind::Val(val, _) => Some(val),
},
// FIXME(rust-lang/rust#85105): Casts like `IMM8 as u32` result in the const being stored
Expand Down
12 changes: 8 additions & 4 deletions compiler/rustc_codegen_ssa/src/debuginfo/type_names.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -703,15 +703,19 @@ fn push_const_param<'tcx>(tcx: TyCtxt<'tcx>, ct: ty::Const<'tcx>, output: &mut S
// but we get a deterministic, virtually unique value for the constant.
let hcx = &mut tcx.create_stable_hashing_context();
let mut hasher = StableHasher::new();
hcx.while_hashing_spans(false, |hcx| ct.kind().hash_stable(hcx, &mut hasher));
let ct = ct.eval(tcx, ty::ParamEnv::reveal_all());
hcx.while_hashing_spans(false, |hcx| ct.to_valtree().hash_stable(hcx, &mut hasher));
// Let's only emit 64 bits of the hash value. That should be plenty for
// avoiding collisions and will make the emitted type names shorter.
let hash: u64 = hasher.finish();
// Note: Don't use `StableHashResult` impl of `u64` here directly, since that
// would lead to endianness problems.
let hash: u128 = hasher.finish();
let hash_short = (hash.to_le() as u64).to_le();
Comment on lines +710 to +713
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me like we should really fix the StableHashResult impl for u64 instead 😅 can you do that in a separate PR or here, depending on what's easier?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's do that in a separate PR.


if cpp_like_debuginfo(tcx) {
write!(output, "CONST${:x}", hash)
write!(output, "CONST${:x}", hash_short)
} else {
write!(output, "{{CONST#{:x}}}", hash)
write!(output, "{{CONST#{:x}}}", hash_short)
}
}
},
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.cx.tcx().sess.span_err(constant.span, "erroneous constant encountered");
err
}),
ty::ConstKind::Value(value) => Ok(value),
ty::ConstKind::Value(val) => Ok(self.cx.tcx().valtree_to_const_val((ct.ty(), val))),
err => span_bug!(
constant.span,
"encountered bad ConstKind after monomorphizing: {:?}",
Expand All @@ -58,14 +58,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
constant
.map(|val| {
let field_ty = ty.builtin_index().unwrap();
let c = ty::Const::from_value(bx.tcx(), val, ty);
let c = mir::ConstantKind::from_value(val, ty);
let values: Vec<_> = bx
.tcx()
.destructure_const(ty::ParamEnv::reveal_all().and(c))
.destructure_mir_constant(ty::ParamEnv::reveal_all(), c)
.fields
.iter()
.map(|field| {
if let Some(prim) = field.kind().try_to_scalar() {
if let Some(prim) = field.try_to_scalar() {
let layout = bx.layout_of(field_ty);
let Abi::Scalar(scalar) = layout.abi else {
bug!("from_const: invalid ByVal layout: {:#?}", layout);
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_const_eval/src/const_eval/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,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, lint_root), level = "debug")]
fn struct_generic(
&self,
tcx: TyCtxtAt<'tcx>,
Expand Down Expand Up @@ -190,6 +191,7 @@ impl<'tcx> ConstEvalErr<'tcx> {
decorate(err);
};

debug!("self.error: {:?}", self.error);
// Special handling for certain errors
match &self.error {
// Don't emit a new diagnostic for these errors
Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/const_eval/eval_queries.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ pub(super) fn op_to_const<'tcx>(
}

#[instrument(skip(tcx), level = "debug")]
fn turn_into_const_value<'tcx>(
pub(crate) fn turn_into_const_value<'tcx>(
tcx: TyCtxt<'tcx>,
constant: ConstAlloc<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
Expand All @@ -222,6 +222,7 @@ fn turn_into_const_value<'tcx>(
const_val
}

#[instrument(skip(tcx), level = "debug")]
pub fn eval_to_const_value_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
Expand Down Expand Up @@ -256,6 +257,7 @@ pub fn eval_to_const_value_raw_provider<'tcx>(
tcx.eval_to_allocation_raw(key).map(|val| turn_into_const_value(tcx, val, key))
}

#[instrument(skip(tcx), level = "debug")]
pub fn eval_to_allocation_raw_provider<'tcx>(
tcx: TyCtxt<'tcx>,
key: ty::ParamEnvAnd<'tcx, GlobalId<'tcx>>,
Expand Down
157 changes: 81 additions & 76 deletions compiler/rustc_const_eval/src/const_eval/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
// Not in interpret to make sure we do not use private implementation details

use std::convert::TryFrom;

use rustc_hir::Mutability;
use rustc_middle::mir;
use rustc_middle::mir::interpret::{EvalToValTreeResult, GlobalId};
use rustc_middle::ty::{self, TyCtxt};
use rustc_span::{source_map::DUMMY_SP, symbol::Symbol};
use rustc_target::abi::VariantIdx;

use crate::interpret::{
intern_const_alloc_recursive, ConstValue, InternKind, InterpCx, InterpResult, MemPlaceMeta,
Expand Down Expand Up @@ -40,7 +39,7 @@ pub(crate) fn const_caller_location(
}

// We forbid type-level constants that contain more than `VALTREE_MAX_NODES` nodes.
const VALTREE_MAX_NODES: usize = 1000;
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you removing that? It still seems desirable to deal with [(); usize::MAX] and stuff.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm not sure about this. It's kind of trial and error to find a good max value for this, if I remember correctly I was encountering constants with >100k nodes after the normalization changes. Using some arbitrary threshold might also break crates.

On tests that use constant arrays with usize::MAX elements we currently get an ICE that fails with a 'capacity overflow' error on Vec:

#![feature(adt_const_params, generic_const_exprs)]

const ARR: [(); usize::MAX] = [(); usize::MAX];

fn test<const ARR: [(); usize::MAX]>() {
  let _ = ARR;
}

fn main() {
  test::<ARR>();
}

Really not sure what the best solution here is.

Copy link
Contributor

Choose a reason for hiding this comment

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

Using some arbitrary threshold might also break crates.

not on stable, on stable all valtrees have a size of 1 because only scalars are allowed. Getting a weird error is preferable to getting an ICE in these cases

const VALTREE_MAX_NODES: usize = 100000;

pub(crate) enum ValTreeCreationError {
NodesOverflow,
Expand All @@ -56,6 +55,8 @@ pub(crate) fn eval_to_valtree<'tcx>(
cid: GlobalId<'tcx>,
) -> EvalToValTreeResult<'tcx> {
let const_alloc = tcx.eval_to_allocation_raw(param_env.and(cid))?;

// FIXME Need to provide a span to `eval_to_valtree`
Copy link
Contributor

Choose a reason for hiding this comment

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

would that span ever be used somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the rationale was to point to the use of a constant that had a Node overflow.

let ecx = mk_eval_cx(
tcx, DUMMY_SP, param_env,
// It is absolutely crucial for soundness that
Expand Down Expand Up @@ -90,40 +91,81 @@ pub(crate) fn eval_to_valtree<'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.
/// Tries to destructure constants of type Array or Adt into the constants
/// of its fields.
pub(crate) fn try_destructure_const<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
val: ty::Const<'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)?;
// 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),
// Checks if we have any variants, to avoid downcasting to a non-existing variant (when
// there are no variants `read_discriminant` successfully returns a non-existing variant
// index).
ty::Adt(def, _) if def.variants().is_empty() => throw_ub!(Unreachable),
ty::Adt(def, _) => {
let variant = ecx.read_discriminant(&op)?.1;
let down = ecx.operand_downcast(&op, variant)?;
(def.variant(variant).fields.len(), Some(variant), down)
}
ty::Tuple(substs) => (substs.len(), None, op),
_ => bug!("cannot destructure constant {:?}", val),
};
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);
Ok(mir::DestructuredConst { variant, fields })
Copy link
Member

Choose a reason for hiding this comment

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

There's a bunch of interpreter functions that are no longer being called here, nice. :) I wonder if this means that we can make some of them private to the interpreter?

const_: ty::Const<'tcx>,
) -> Option<ty::DestructuredConst<'tcx>> {
if let ty::ConstKind::Value(valtree) = const_.kind() {
let branches = match valtree {
ty::ValTree::Branch(b) => b,
_ => return None,
};

let (fields, variant) = match const_.ty().kind() {
ty::Array(inner_ty, _) | ty::Slice(inner_ty) => {
// construct the consts for the elements of the array/slice
let field_consts = branches
.iter()
.map(|b| {
tcx.mk_const(ty::ConstS { kind: ty::ConstKind::Value(*b), ty: *inner_ty })
})
.collect::<Vec<_>>();
debug!(?field_consts);

(field_consts, None)
}
ty::Adt(def, _) if def.variants().is_empty() => bug!("unreachable"),
ty::Adt(def, substs) => {
let variant_idx = if def.is_enum() {
VariantIdx::from_u32(branches[0].unwrap_leaf().try_to_u32().ok()?)
} else {
VariantIdx::from_u32(0)
};
let fields = &def.variant(variant_idx).fields;
let mut field_consts = Vec::with_capacity(fields.len());

// Note: First element inValTree corresponds to variant of enum
let mut valtree_idx = if def.is_enum() { 1 } else { 0 };
for field in fields {
let field_ty = field.ty(tcx, substs);
let field_valtree = branches[valtree_idx]; // first element of branches is variant
let field_const = tcx.mk_const(ty::ConstS {
kind: ty::ConstKind::Value(field_valtree),
ty: field_ty,
});
field_consts.push(field_const);
valtree_idx += 1;
}
debug!(?field_consts);

(field_consts, Some(variant_idx))
}
ty::Tuple(elem_tys) => {
let fields = elem_tys
.iter()
.enumerate()
.map(|(i, elem_ty)| {
let elem_valtree = branches[i];
tcx.mk_const(ty::ConstS {
kind: ty::ConstKind::Value(elem_valtree),
ty: elem_ty,
})
})
.collect::<Vec<_>>();

(fields, None)
}
_ => bug!("cannot destructure constant {:?}", const_),
};

let fields = tcx.arena.alloc_from_iter(fields.into_iter());

Some(ty::DestructuredConst { variant, fields })
} else {
None
}
}

#[instrument(skip(tcx), level = "debug")]
Expand All @@ -143,8 +185,8 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
throw_ub!(Unreachable)
}
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),
Expand All @@ -163,43 +205,6 @@ pub(crate) fn try_destructure_mir_constant<'tcx>(
Ok(mir::DestructuredMirConstant { variant, fields })
}

#[instrument(skip(tcx), level = "debug")]
pub(crate) fn deref_const<'tcx>(
tcx: TyCtxt<'tcx>,
param_env: ty::ParamEnv<'tcx>,
val: ty::Const<'tcx>,
) -> ty::Const<'tcx> {
trace!("deref_const: {:?}", val);
let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env, false);
let op = ecx.const_to_op(val, None).unwrap();
let mplace = ecx.deref_operand(&op).unwrap();
if let Some(alloc_id) = mplace.ptr.provenance {
assert_eq!(
tcx.get_global_alloc(alloc_id).unwrap().unwrap_memory().inner().mutability,
Mutability::Not,
"deref_const cannot be used with mutable allocations as \
that could allow pattern matching to observe mutable statics",
);
}

let ty = match mplace.meta {
MemPlaceMeta::None => mplace.layout.ty,
MemPlaceMeta::Poison => bug!("poison metadata in `deref_const`: {:#?}", mplace),
// In case of unsized types, figure out the real type behind.
MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() {
ty::Str => bug!("there's no sized equivalent of a `str`"),
ty::Slice(elem_ty) => tcx.mk_array(*elem_ty, scalar.to_machine_usize(&tcx).unwrap()),
_ => bug!(
"type {} should not have metadata, but had {:?}",
mplace.layout.ty,
mplace.meta
),
},
};

tcx.mk_const(ty::ConstS { kind: ty::ConstKind::Value(op_to_const(&ecx, &mplace.into())), ty })
}

#[instrument(skip(tcx), level = "debug")]
pub(crate) fn deref_mir_constant<'tcx>(
tcx: TyCtxt<'tcx>,
Expand All @@ -213,14 +218,14 @@ pub(crate) fn deref_mir_constant<'tcx>(
assert_eq!(
tcx.get_global_alloc(alloc_id).unwrap().unwrap_memory().0.0.mutability,
Mutability::Not,
"deref_const cannot be used with mutable allocations as \
"deref_mir_constant cannot be used with mutable allocations as \
that could allow pattern matching to observe mutable statics",
);
}

let ty = match mplace.meta {
MemPlaceMeta::None => mplace.layout.ty,
MemPlaceMeta::Poison => bug!("poison metadata in `deref_const`: {:#?}", mplace),
MemPlaceMeta::Poison => bug!("poison metadata in `deref_mir_constant`: {:#?}", mplace),
// In case of unsized types, figure out the real type behind.
MemPlaceMeta::Meta(scalar) => match mplace.layout.ty.kind() {
ty::Str => bug!("there's no sized equivalent of a `str`"),
Expand Down
Loading