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

Use only ty::Unevaluated<'tcx, ()> in type system #98588

Merged
merged 11 commits into from
Sep 17, 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
30 changes: 28 additions & 2 deletions compiler/rustc_borrowck/src/renumber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use rustc_index::vec::IndexVec;
use rustc_infer::infer::{InferCtxt, NllRegionVariableOrigin};
use rustc_middle::mir::visit::{MutVisitor, TyContext};
use rustc_middle::mir::{Body, Location, Promoted};
use rustc_middle::mir::{Constant, ConstantKind};
use rustc_middle::ty::subst::SubstsRef;
use rustc_middle::ty::{self, Ty, TyCtxt, TypeFoldable};

Expand Down Expand Up @@ -37,6 +38,21 @@ where
})
}

// FIXME(valtrees): This function is necessary because `fold_regions`
// panics for mir constants in the visitor.
//
// Once `visit_mir_constant` is removed we can also remove this function
// and just use `renumber_regions`.
fn renumber_regions_in_mir_constant<'tcx>(
b-naber marked this conversation as resolved.
Show resolved Hide resolved
infcx: &InferCtxt<'_, 'tcx>,
value: ConstantKind<'tcx>,
) -> ConstantKind<'tcx> {
infcx.tcx.super_fold_regions(value, |_region, _depth| {
let origin = NllRegionVariableOrigin::Existential { from_forall: false };
infcx.next_nll_region_var(origin)
})
}

struct NllVisitor<'a, 'tcx> {
infcx: &'a InferCtxt<'a, 'tcx>,
}
Expand All @@ -48,6 +64,13 @@ impl<'a, 'tcx> NllVisitor<'a, 'tcx> {
{
renumber_regions(self.infcx, value)
}

fn renumber_regions_in_mir_constant(
&mut self,
value: ConstantKind<'tcx>,
) -> ConstantKind<'tcx> {
renumber_regions_in_mir_constant(self.infcx, value)
}
}

impl<'a, 'tcx> MutVisitor<'tcx> for NllVisitor<'a, 'tcx> {
Expand Down Expand Up @@ -77,7 +100,10 @@ impl<'a, 'tcx> MutVisitor<'tcx> for NllVisitor<'a, 'tcx> {
debug!(?region);
}

fn visit_const(&mut self, constant: &mut ty::Const<'tcx>, _location: Location) {
*constant = self.renumber_regions(*constant);
#[instrument(skip(self), level = "debug")]
fn visit_constant(&mut self, constant: &mut Constant<'tcx>, _location: Location) {
let literal = constant.literal;
lcnr marked this conversation as resolved.
Show resolved Hide resolved
constant.literal = self.renumber_regions_in_mir_constant(literal);
debug!("constant: {:#?}", constant);
}
}
14 changes: 8 additions & 6 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -354,11 +354,15 @@ impl<'a, 'b, 'tcx> Visitor<'tcx> for TypeVerifier<'a, 'b, 'tcx> {
let tcx = self.tcx();
let maybe_uneval = match constant.literal {
ConstantKind::Ty(ct) => match ct.kind() {
ty::ConstKind::Unevaluated(uv) => Some(uv),
ty::ConstKind::Unevaluated(_) => {
bug!("should not encounter unevaluated ConstantKind::Ty here, got {:?}", ct)
}
_ => None,
},
ConstantKind::Unevaluated(uv, _) => Some(uv),
_ => None,
};

if let Some(uv) = maybe_uneval {
if let Some(promoted) = uv.promoted {
let check_err = |verifier: &mut TypeVerifier<'a, 'b, 'tcx>,
Expand Down Expand Up @@ -1812,12 +1816,10 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
fn check_operand(&mut self, op: &Operand<'tcx>, location: Location) {
if let Operand::Constant(constant) = op {
let maybe_uneval = match constant.literal {
ConstantKind::Ty(ct) => match ct.kind() {
ty::ConstKind::Unevaluated(uv) => Some(uv),
_ => None,
},
_ => None,
ConstantKind::Val(..) | ConstantKind::Ty(_) => None,
ConstantKind::Unevaluated(uv, _) => Some(uv),
};

if let Some(uv) = maybe_uneval {
if uv.promoted.is_none() {
let tcx = self.tcx();
Expand Down
73 changes: 31 additions & 42 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,36 +41,30 @@ impl ConstantCx {
pub(crate) fn check_constants(fx: &mut FunctionCx<'_, '_, '_>) -> bool {
let mut all_constants_ok = true;
for constant in &fx.mir.required_consts {
let const_ = match fx.monomorphize(constant.literal) {
ConstantKind::Ty(ct) => ct,
let unevaluated = match fx.monomorphize(constant.literal) {
ConstantKind::Ty(ct) => match ct.kind() {
ConstKind::Unevaluated(uv) => uv.expand(),
ConstKind::Value(_) => continue,
ConstKind::Param(_)
| ConstKind::Infer(_)
| ConstKind::Bound(_, _)
| ConstKind::Placeholder(_)
| ConstKind::Error(_) => unreachable!("{:?}", ct),
},
b-naber marked this conversation as resolved.
Show resolved Hide resolved
ConstantKind::Unevaluated(uv, _) => uv,
ConstantKind::Val(..) => continue,
};
match const_.kind() {
ConstKind::Value(_) => {}
ConstKind::Unevaluated(unevaluated) => {
if let Err(err) =
fx.tcx.const_eval_resolve(ParamEnv::reveal_all(), unevaluated, None)
{
all_constants_ok = false;
match err {
ErrorHandled::Reported(_) | ErrorHandled::Linted => {
fx.tcx.sess.span_err(constant.span, "erroneous constant encountered");
}
ErrorHandled::TooGeneric => {
span_bug!(
constant.span,
"codegen encountered polymorphic constant: {:?}",
err
);
}
}

if let Err(err) = fx.tcx.const_eval_resolve(ParamEnv::reveal_all(), unevaluated, None) {
all_constants_ok = false;
match err {
ErrorHandled::Reported(_) | ErrorHandled::Linted => {
fx.tcx.sess.span_err(constant.span, "erroneous constant encountered");
}
ErrorHandled::TooGeneric => {
span_bug!(constant.span, "codegen encountered polymorphic constant: {:?}", err);
}
}
ConstKind::Param(_)
| ConstKind::Infer(_)
| ConstKind::Bound(_, _)
| ConstKind::Placeholder(_)
| ConstKind::Error(_) => unreachable!("{:?}", const_),
}
}
all_constants_ok
Expand Down Expand Up @@ -122,36 +116,28 @@ pub(crate) fn codegen_constant<'tcx>(
fx: &mut FunctionCx<'_, '_, 'tcx>,
constant: &Constant<'tcx>,
) -> CValue<'tcx> {
let const_ = match fx.monomorphize(constant.literal) {
ConstantKind::Ty(ct) => ct,
ConstantKind::Val(val, ty) => return codegen_const_value(fx, val, ty),
};
let const_val = match const_.kind() {
ConstKind::Value(valtree) => fx.tcx.valtree_to_const_val((const_.ty(), valtree)),
ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted })
let (const_val, ty) = match fx.monomorphize(constant.literal) {
ConstantKind::Ty(const_) => unreachable!("{:?}", const_),
ConstantKind::Unevaluated(ty::Unevaluated { def, substs, promoted }, ty)
if fx.tcx.is_static(def.did) =>
{
assert!(substs.is_empty());
assert!(promoted.is_none());

return codegen_static_ref(fx, def.did, fx.layout_of(const_.ty())).to_cvalue(fx);
return codegen_static_ref(fx, def.did, fx.layout_of(ty)).to_cvalue(fx);
}
ConstKind::Unevaluated(unevaluated) => {
ConstantKind::Unevaluated(unevaluated, ty) => {
match fx.tcx.const_eval_resolve(ParamEnv::reveal_all(), unevaluated, None) {
Ok(const_val) => const_val,
Ok(const_val) => (const_val, ty),
Err(_) => {
span_bug!(constant.span, "erroneous constant not captured by required_consts");
}
}
}
ConstKind::Param(_)
| ConstKind::Infer(_)
| ConstKind::Bound(_, _)
| ConstKind::Placeholder(_)
| ConstKind::Error(_) => unreachable!("{:?}", const_),
ConstantKind::Val(val, ty) => (val, ty),
};

codegen_const_value(fx, const_val, const_.ty())
codegen_const_value(fx, const_val, ty)
}

pub(crate) fn codegen_const_value<'tcx>(
Expand Down Expand Up @@ -496,6 +482,9 @@ pub(crate) fn mir_operand_get_const_val<'tcx>(
.eval_for_mir(fx.tcx, ParamEnv::reveal_all())
.try_to_value(fx.tcx),
ConstantKind::Val(val, _) => Some(val),
ConstantKind::Unevaluated(uv, _) => {
fx.tcx.const_eval_resolve(ParamEnv::reveal_all(), uv, None).ok()
}
},
// FIXME(rust-lang/rust#85105): Casts like `IMM8 as u32` result in the const being stored
// inside a temporary before being passed to the intrinsic requiring the const argument.
Expand Down
36 changes: 18 additions & 18 deletions compiler/rustc_codegen_ssa/src/mir/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,26 +25,26 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
constant: &mir::Constant<'tcx>,
) -> Result<ConstValue<'tcx>, ErrorHandled> {
let ct = self.monomorphize(constant.literal);
let ct = match ct {
mir::ConstantKind::Ty(ct) => ct,
let uv = match ct {
mir::ConstantKind::Ty(ct) => match ct.kind() {
ty::ConstKind::Unevaluated(uv) => uv.expand(),
Copy link
Contributor

Choose a reason for hiding this comment

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

again only ConstKind::Value should be possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we can still have ConstKind::Unevaluated here.

Copy link
Contributor

Choose a reason for hiding this comment

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

that seems wrong? the constant should be evaluated in monomorphize 🤔

maybe we should return an Err here if this fails to evaluate

let constant = constant.try_super_fold_with(self)?;
Ok(constant.eval(self.infcx.tcx, self.param_env))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's also not clear to me why we would want to error here if we can't evaluate yet, this is called all the time during typecheck, isn't it?

Copy link
Contributor

Choose a reason for hiding this comment

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

no, the normalize query isn't used during typeck, it's only used afterwards.

i guess it's fine to keep this for now 🤷 will be easier to fix this once your pr has landed

ty::ConstKind::Value(val) => {
return Ok(self.cx.tcx().valtree_to_const_val((ct.ty(), val)));
}
err => span_bug!(
constant.span,
"encountered bad ConstKind after monomorphizing: {:?}",
err
),
},
mir::ConstantKind::Unevaluated(uv, _) => uv,
mir::ConstantKind::Val(val, _) => return Ok(val),
};
match ct.kind() {
ty::ConstKind::Unevaluated(ct) => self
.cx
.tcx()
.const_eval_resolve(ty::ParamEnv::reveal_all(), ct, None)
.map_err(|err| {
self.cx.tcx().sess.span_err(constant.span, "erroneous constant encountered");
err
}),
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: {:?}",
err
),
}

self.cx.tcx().const_eval_resolve(ty::ParamEnv::reveal_all(), uv, None).map_err(|err| {
self.cx.tcx().sess.span_err(constant.span, "erroneous constant encountered");
err
})
b-naber marked this conversation as resolved.
Show resolved Hide resolved
}

/// process constant containing SIMD shuffle indices
Expand Down
14 changes: 13 additions & 1 deletion compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -564,8 +564,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
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)?;
Ok(self.eval_to_allocation(GlobalId { instance, promoted: uv.promoted })?.into())
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));

Comment on lines +567 to +575
Copy link
Member

Choose a reason for hiding this comment

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

This is a strange check. Why would CTFE care whether the constant is a valid valtree? What if uv has a type that does not even allow valtree construction, like a raw pointer?

Copy link
Member

@RalfJung RalfJung Nov 15, 2022

Choose a reason for hiding this comment

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

Oh, this is specifically for mir::ConstantKind::Ty, which I guess must always be valtree-constructible.

But then IMO it would make most sense to have a function that evaluates a ty::Const to a Valtree, and always go through that. Doesn't that exist as a query or so?

Copy link
Member

Choose a reason for hiding this comment

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

But then IMO it would make most sense to have a function that evaluates a ty::Const to a Valtree, and always go through that.

I will do that in #104317

Ok(self.eval_to_allocation(cid)?.into())
}
ty::ConstKind::Bound(..) | ty::ConstKind::Infer(..) => {
span_bug!(self.cur_span(), "const_to_op: Unexpected ConstKind {:?}", c)
Expand All @@ -586,6 +594,10 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
match val {
mir::ConstantKind::Ty(ct) => self.const_to_op(*ct, 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())
}
}
}

Expand Down
54 changes: 33 additions & 21 deletions compiler/rustc_const_eval/src/transform/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -346,31 +346,43 @@ where
};

// Check the qualifs of the value of `const` items.
if let Some(ct) = constant.literal.const_for_ty() {
if let ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs: _, promoted }) = ct.kind()
{
// Use qualifs of the type for the promoted. Promoteds in MIR body should be possible
// only for `NeedsNonConstDrop` with precise drop checking. This is the only const
// check performed after the promotion. Verify that with an assertion.
assert!(promoted.is_none() || Q::ALLOW_PROMOTED);
// Don't peek inside trait associated constants.
if promoted.is_none() && cx.tcx.trait_of_item(def.did).is_none() {
let qualifs = if let Some((did, param_did)) = def.as_const_arg() {
cx.tcx.at(constant.span).mir_const_qualif_const_arg((did, param_did))
} else {
cx.tcx.at(constant.span).mir_const_qualif(def.did)
};

if !Q::in_qualifs(&qualifs) {
return false;
}
// FIXME(valtrees): check whether const qualifs should behave the same
// way for type and mir constants.
let uneval = match constant.literal {
b-naber marked this conversation as resolved.
Show resolved Hide resolved
ConstantKind::Ty(ct) if matches!(ct.kind(), ty::ConstKind::Unevaluated(_)) => {
let ty::ConstKind::Unevaluated(uv) = ct.kind() else { unreachable!() };

// Just in case the type is more specific than
// the definition, e.g., impl associated const
// with type parameters, take it into account.
Some(uv.expand())
}
ConstantKind::Ty(_) => None,
ConstantKind::Unevaluated(uv, _) => Some(uv),
ConstantKind::Val(..) => None,
};

if let Some(ty::Unevaluated { def, substs: _, promoted }) = uneval {
// Use qualifs of the type for the promoted. Promoteds in MIR body should be possible
// only for `NeedsNonConstDrop` with precise drop checking. This is the only const
// check performed after the promotion. Verify that with an assertion.
assert!(promoted.is_none() || Q::ALLOW_PROMOTED);

// Don't peek inside trait associated constants.
if promoted.is_none() && cx.tcx.trait_of_item(def.did).is_none() {
let qualifs = if let Some((did, param_did)) = def.as_const_arg() {
cx.tcx.at(constant.span).mir_const_qualif_const_arg((did, param_did))
} else {
cx.tcx.at(constant.span).mir_const_qualif(def.did)
};

if !Q::in_qualifs(&qualifs) {
return false;
}

// Just in case the type is more specific than
// the definition, e.g., impl associated const
// with type parameters, take it into account.
}
}

// Otherwise use the qualifs of the type.
Q::in_any_value_of_ty(cx, constant.literal.ty())
}
12 changes: 3 additions & 9 deletions compiler/rustc_const_eval/src/transform/promote_consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -840,21 +840,15 @@ impl<'a, 'tcx> Promoter<'a, 'tcx> {
promoted.span = span;
promoted.local_decls[RETURN_PLACE] = LocalDecl::new(ty, span);
let substs = tcx.erase_regions(InternalSubsts::identity_for_item(tcx, def.did));
let _const = tcx.mk_const(ty::ConstS {
ty,
kind: ty::ConstKind::Unevaluated(ty::Unevaluated {
def,
substs,
promoted: Some(promoted_id),
}),
});
let uneval = ty::Unevaluated { def, substs, promoted: Some(promoted_id) };

Operand::Constant(Box::new(Constant {
span,
user_ty: None,
literal: ConstantKind::from_const(_const, tcx),
literal: ConstantKind::Unevaluated(uneval, ty),
}))
};

let blocks = self.source.basic_blocks.as_mut();
let local_decls = &mut self.source.local_decls;
let loc = candidate.location;
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_infer/src/infer/combine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -743,7 +743,8 @@ impl<'tcx> TypeRelation<'tcx> for Generalizer<'_, 'tcx> {
}
}
ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }) => {
assert_eq!(promoted, None);
assert_eq!(promoted, ());

let substs = self.relate_with_variance(
ty::Variance::Invariant,
ty::VarianceDiagInfo::default(),
Expand Down Expand Up @@ -964,13 +965,15 @@ impl<'tcx> TypeRelation<'tcx> for ConstInferUnifier<'_, 'tcx> {
}
}
ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }) => {
assert_eq!(promoted, None);
assert_eq!(promoted, ());

let substs = self.relate_with_variance(
ty::Variance::Invariant,
ty::VarianceDiagInfo::default(),
substs,
substs,
)?;

Ok(self.tcx().mk_const(ty::ConstS {
ty: c.ty(),
kind: ty::ConstKind::Unevaluated(ty::Unevaluated { def, substs, promoted }),
Expand Down
Loading