Skip to content

Commit

Permalink
Auto merge of #98957 - RalfJung:zst-are-different, r=lcnr,oli-obk
Browse files Browse the repository at this point in the history
 don't allow ZST in ScalarInt

There are several indications that we should not ZST as a ScalarInt:
- We had two ways to have ZST valtrees, either an empty `Branch` or a `Leaf` with a ZST in it.
  `ValTree::zst()` used the former, but the latter could possibly arise as well.
- Likewise, the interpreter had `Immediate::Uninit` and `Immediate::Scalar(Scalar::ZST)`.
- LLVM codegen already had to special-case ZST ScalarInt.

So I propose we stop using ScalarInt to represent ZST (which are clearly not integers). Instead, we can add new ZST variants to those types that did not have other variants which could be used for this purpose.

Based on #98831. Only the commits starting from "don't allow ZST in ScalarInt" are new.

r? `@oli-obk`
  • Loading branch information
bors committed Jul 9, 2022
2 parents 6c20ab7 + 4e7aaf1 commit f893495
Show file tree
Hide file tree
Showing 139 changed files with 312 additions and 291 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_codegen_cranelift/src/constant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ pub(crate) fn codegen_const_value<'tcx>(
}

match const_val {
ConstValue::ZeroSized => unreachable!(), // we already handles ZST above
ConstValue::Scalar(x) => match x {
Scalar::Int(int) => {
if fx.clif_type(layout.ty).is_some() {
Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_codegen_gcc/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ use rustc_codegen_ssa::traits::{
StaticMethods,
};
use rustc_middle::mir::Mutability;
use rustc_middle::ty::ScalarInt;
use rustc_middle::ty::layout::{TyAndLayout, LayoutOf};
use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc, Scalar};
use rustc_target::abi::{self, HasDataLayout, Pointer, Size};
Expand Down Expand Up @@ -159,13 +158,13 @@ impl<'gcc, 'tcx> ConstMethods<'tcx> for CodegenCx<'gcc, 'tcx> {
None
}

fn zst_to_backend(&self, _ty: Type<'gcc>) -> RValue<'gcc> {
self.const_undef(self.type_ix(0))
}

fn scalar_to_backend(&self, cv: Scalar, layout: abi::Scalar, ty: Type<'gcc>) -> RValue<'gcc> {
let bitsize = if layout.is_bool() { 1 } else { layout.size(self).bits() };
match cv {
Scalar::Int(ScalarInt::ZST) => {
assert_eq!(0, layout.size(self).bytes());
self.const_undef(self.type_ix(0))
}
Scalar::Int(int) => {
let data = int.assert_bits(layout.size(self));

Expand Down
9 changes: 4 additions & 5 deletions compiler/rustc_codegen_llvm/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ use rustc_codegen_ssa::traits::*;
use rustc_middle::bug;
use rustc_middle::mir::interpret::{ConstAllocation, GlobalAlloc, Scalar};
use rustc_middle::ty::layout::{LayoutOf, TyAndLayout};
use rustc_middle::ty::ScalarInt;
use rustc_target::abi::{self, AddressSpace, HasDataLayout, Pointer, Size};

use libc::{c_char, c_uint};
Expand Down Expand Up @@ -223,13 +222,13 @@ impl<'ll, 'tcx> ConstMethods<'tcx> for CodegenCx<'ll, 'tcx> {
})
}

fn zst_to_backend(&self, _llty: &'ll Type) -> &'ll Value {
self.const_undef(self.type_ix(0))
}

fn scalar_to_backend(&self, cv: Scalar, layout: abi::Scalar, llty: &'ll Type) -> &'ll Value {
let bitsize = if layout.is_bool() { 1 } else { layout.size(self).bits() };
match cv {
Scalar::Int(ScalarInt::ZST) => {
assert_eq!(0, layout.size(self).bytes());
self.const_undef(self.type_ix(0))
}
Scalar::Int(int) => {
let data = int.assert_bits(layout.size(self));
let llval = self.const_uint_big(self.type_ix(bitsize), data);
Expand Down
4 changes: 4 additions & 0 deletions compiler/rustc_codegen_ssa/src/mir/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ impl<'a, 'tcx, V: CodegenObject> OperandRef<'tcx, V> {
let llval = bx.scalar_to_backend(x, scalar, bx.immediate_backend_type(layout));
OperandValue::Immediate(llval)
}
ConstValue::ZeroSized => {
let llval = bx.zst_to_backend(bx.immediate_backend_type(layout));
OperandValue::Immediate(llval)
}
ConstValue::Slice { data, start, end } => {
let Abi::ScalarPair(a_scalar, _) = layout.abi else {
bug!("from_const: invalid ScalarPair layout: {:#?}", layout);
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_codegen_ssa/src/traits/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ pub trait ConstMethods<'tcx>: BackendTypes {
fn const_data_from_alloc(&self, alloc: ConstAllocation<'tcx>) -> Self::Value;

fn scalar_to_backend(&self, cv: Scalar, layout: abi::Scalar, llty: Self::Type) -> Self::Value;
fn zst_to_backend(&self, llty: Self::Type) -> Self::Value;
fn from_const_alloc(
&self,
layout: TyAndLayout<'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_const_eval/src/const_eval/eval_queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::{CompileTimeEvalContext, CompileTimeInterpreter, ConstEvalErr};
use crate::interpret::eval_nullary_intrinsic;
use crate::interpret::{
intern_const_alloc_recursive, Allocation, ConstAlloc, ConstValue, CtfeValidationMode, GlobalId,
Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking, Scalar,
Immediate, InternKind, InterpCx, InterpResult, MPlaceTy, MemoryKind, OpTy, RefTracking,
ScalarMaybeUninit, StackPopCleanup,
};

Expand Down Expand Up @@ -157,7 +157,7 @@ pub(super) fn op_to_const<'tcx>(
"this MPlaceTy must come from a validated constant, thus we can assume the \
alignment is correct",
);
ConstValue::Scalar(Scalar::ZST)
ConstValue::ZeroSized
}
}
};
Expand Down
8 changes: 2 additions & 6 deletions compiler/rustc_const_eval/src/const_eval/valtrees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ pub fn valtree_to_const_value<'tcx>(
match ty.kind() {
ty::FnDef(..) => {
assert!(valtree.unwrap_branch().is_empty());
ConstValue::Scalar(Scalar::ZST)
ConstValue::ZeroSized
}
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => match valtree {
ty::ValTree::Leaf(scalar_int) => ConstValue::Scalar(Scalar::Int(scalar_int)),
Expand Down Expand Up @@ -344,11 +344,7 @@ fn valtree_into_mplace<'tcx>(

match ty.kind() {
ty::FnDef(_, _) => {
ecx.write_immediate(
Immediate::Scalar(ScalarMaybeUninit::Scalar(Scalar::ZST)),
&place.into(),
)
.unwrap();
ecx.write_immediate(Immediate::Uninit, &place.into()).unwrap();
}
ty::Bool | ty::Int(_) | ty::Uint(_) | ty::Float(_) | ty::Char => {
let scalar_int = valtree.unwrap_leaf();
Expand Down
13 changes: 7 additions & 6 deletions compiler/rustc_const_eval/src/interpret/operand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {

let Some(alloc) = self.get_place_alloc(mplace)? else {
return Ok(Some(ImmTy {
// zero-sized type
imm: Scalar::ZST.into(),
// zero-sized type can be left uninit
imm: Immediate::Uninit,
layout: mplace.layout,
}));
};
Expand Down Expand Up @@ -441,8 +441,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
// This makes several assumptions about what layouts we will encounter; we match what
// codegen does as good as we can (see `extract_field` in `rustc_codegen_ssa/src/mir/operand.rs`).
let field_val: Immediate<_> = match (*base, base.layout.abi) {
// the field contains no information
_ if field_layout.is_zst() => Scalar::ZST.into(),
// the field contains no information, can be left uninit
_ if field_layout.is_zst() => Immediate::Uninit,
// the field covers the entire type
_ if field_layout.size == base.layout.size => {
assert!(match (base.layout.abi, field_layout.abi) {
Expand Down Expand Up @@ -553,8 +553,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
) -> InterpResult<'tcx, OpTy<'tcx, M::PointerTag>> {
let layout = self.layout_of_local(frame, local, layout)?;
let op = if layout.is_zst() {
// Do not read from ZST, they might not be initialized
Operand::Immediate(Scalar::ZST.into())
// Bypass `access_local` (helps in ConstProp)
Operand::Immediate(Immediate::Uninit)
} else {
*M::access_local(frame, local)?
};
Expand Down Expand Up @@ -709,6 +709,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> {
Operand::Indirect(MemPlace::from_ptr(ptr.into()))
}
ConstValue::Scalar(x) => Operand::Immediate(tag_scalar(x)?.into()),
ConstValue::ZeroSized => Operand::Immediate(Immediate::Uninit),
ConstValue::Slice { data, start, end } => {
// We rely on mutability being set correctly in `data` to prevent writes
// where none should happen.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@
#![feature(drain_filter)]
#![feature(intra_doc_pointers)]
#![feature(yeet_expr)]
#![feature(const_option)]
#![recursion_limit = "512"]
#![allow(rustc::potential_query_instability)]

Expand Down
14 changes: 6 additions & 8 deletions compiler/rustc_middle/src/mir/interpret/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ pub struct ConstAlloc<'tcx> {
#[derive(Copy, Clone, Debug, Eq, PartialEq, PartialOrd, Ord, TyEncodable, TyDecodable, Hash)]
#[derive(HashStable)]
pub enum ConstValue<'tcx> {
/// Used only for types with `layout::abi::Scalar` ABI and ZSTs.
/// Used only for types with `layout::abi::Scalar` ABI.
///
/// Not using the enum `Value` to encode that this must not be `Uninit`.
Scalar(Scalar),

/// Only used for ZSTs.
ZeroSized,

/// Used only for `&[u8]` and `&str`
Slice { data: ConstAllocation<'tcx>, start: usize, end: usize },

Expand All @@ -55,6 +58,7 @@ impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> {
fn lift_to_tcx(self, tcx: TyCtxt<'tcx>) -> Option<ConstValue<'tcx>> {
Some(match self {
ConstValue::Scalar(s) => ConstValue::Scalar(s),
ConstValue::ZeroSized => ConstValue::ZeroSized,
ConstValue::Slice { data, start, end } => {
ConstValue::Slice { data: tcx.lift(data)?, start, end }
}
Expand All @@ -69,7 +73,7 @@ impl<'tcx> ConstValue<'tcx> {
#[inline]
pub fn try_to_scalar(&self) -> Option<Scalar<AllocId>> {
match *self {
ConstValue::ByRef { .. } | ConstValue::Slice { .. } => None,
ConstValue::ByRef { .. } | ConstValue::Slice { .. } | ConstValue::ZeroSized => None,
ConstValue::Scalar(val) => Some(val),
}
}
Expand Down Expand Up @@ -111,10 +115,6 @@ impl<'tcx> ConstValue<'tcx> {
pub fn from_machine_usize(i: u64, cx: &impl HasDataLayout) -> Self {
ConstValue::Scalar(Scalar::from_machine_usize(i, cx))
}

pub fn zst() -> Self {
Self::Scalar(Scalar::ZST)
}
}

/// A `Scalar` represents an immediate, primitive value existing outside of a
Expand Down Expand Up @@ -194,8 +194,6 @@ impl<Tag> From<ScalarInt> for Scalar<Tag> {
}

impl<Tag> Scalar<Tag> {
pub const ZST: Self = Scalar::Int(ScalarInt::ZST);

#[inline(always)]
pub fn from_pointer(ptr: Pointer<Tag>, cx: &impl HasDataLayout) -> Self {
Scalar::Ptr(ptr, u8::try_from(cx.pointer_size().bytes()).unwrap())
Expand Down
11 changes: 9 additions & 2 deletions compiler/rustc_middle/src/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1711,7 +1711,7 @@ impl<'tcx> Operand<'tcx> {
Operand::Constant(Box::new(Constant {
span,
user_ty: None,
literal: ConstantKind::Val(ConstValue::zst(), ty),
literal: ConstantKind::Val(ConstValue::ZeroSized, ty),
}))
}

Expand Down Expand Up @@ -2196,7 +2196,7 @@ impl<'tcx> ConstantKind<'tcx> {

#[inline]
pub fn zero_sized(ty: Ty<'tcx>) -> Self {
let cv = ConstValue::Scalar(Scalar::ZST);
let cv = ConstValue::ZeroSized;
Self::Val(cv, ty)
}

Expand Down Expand Up @@ -2772,6 +2772,13 @@ fn pretty_print_const_value<'tcx>(
fmt.write_str(&cx.into_buffer())?;
return Ok(());
}
(ConstValue::ZeroSized, ty::FnDef(d, s)) => {
let mut cx = FmtPrinter::new(tcx, Namespace::ValueNS);
cx.print_alloc_ids = true;
let cx = cx.print_value_path(*d, s)?;
fmt.write_str(&cx.into_buffer())?;
return Ok(());
}
// FIXME(oli-obk): also pretty print arrays and other aggregate constants by reading
// their fields instead of just dumping the memory.
_ => {}
Expand Down
3 changes: 3 additions & 0 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,9 @@ impl<'tcx> Visitor<'tcx> for ExtraComments<'tcx> {
self.push(&format!("+ user_ty: {:?}", user_ty));
}

// FIXME: this is a poor version of `pretty_print_const_value`.
let fmt_val = |val: &ConstValue<'tcx>| match val {
ConstValue::ZeroSized => format!("<ZST>"),
ConstValue::Scalar(s) => format!("Scalar({:?})", s),
ConstValue::Slice { .. } => format!("Slice(..)"),
ConstValue::ByRef { .. } => format!("ByRef(..)"),
Expand Down Expand Up @@ -679,6 +681,7 @@ pub fn write_allocations<'tcx>(
ConstValue::Scalar(interpret::Scalar::Int { .. }) => {
Either::Left(Either::Right(std::iter::empty()))
}
ConstValue::ZeroSized => Either::Left(Either::Right(std::iter::empty())),
ConstValue::ByRef { alloc, .. } | ConstValue::Slice { data: alloc, .. } => {
Either::Right(alloc_ids_from_alloc(alloc))
}
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_middle/src/thir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,6 +419,10 @@ pub enum ExprKind<'tcx> {
lit: ty::ScalarInt,
user_ty: Option<Canonical<'tcx, UserType<'tcx>>>,
},
/// A literal of a ZST type.
ZstLiteral {
user_ty: Option<Canonical<'tcx, UserType<'tcx>>>,
},
/// Associated constants and named constants
NamedConst {
def_id: DefId,
Expand Down Expand Up @@ -454,12 +458,6 @@ pub enum ExprKind<'tcx> {
},
}

impl<'tcx> ExprKind<'tcx> {
pub fn zero_sized_literal(user_ty: Option<Canonical<'tcx, UserType<'tcx>>>) -> Self {
ExprKind::NonHirLiteral { lit: ty::ScalarInt::ZST, user_ty }
}
}

/// Represents the association of a field identifier and an expression.
///
/// This is used in struct constructors.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/thir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp
Closure { closure_id: _, substs: _, upvars: _, movability: _, fake_reads: _ } => {}
Literal { lit: _, neg: _ } => {}
NonHirLiteral { lit: _, user_ty: _ } => {}
ZstLiteral { user_ty: _ } => {}
NamedConst { def_id: _, substs: _, user_ty: _ } => {}
ConstParam { param: _, def_id: _ } => {}
StaticRef { alloc_id: _, ty: _, def_id: _ } => {}
Expand Down
Loading

0 comments on commit f893495

Please sign in to comment.