Skip to content

Commit

Permalink
Auto merge of #135709 - lqd:bring-back-len, r=compiler-errors
Browse files Browse the repository at this point in the history
Temporarily bring back `Rvalue::Len`

r? `@compiler-errors` as requested in #135671 (comment)

> However, in the mean time, I'd rather we not crunch trying to find and more importantly validate the soundness of a solution 🤔

Agreed. To fix the IMO P-critical #135671 for which we somehow didn't have test coverage, this PR temporarily reverts:
- #133734
- its bugfix #134371
- #134330

cc `@scottmcm`

I added the few samples from that issue as a test, but we can add more in the future, in particular it seems `@steffahn` [will work on that](#135671 (comment)).

Fixes #135671. And if we want to land this, it should also be nominated for beta backport.
  • Loading branch information
bors committed Jan 19, 2025
2 parents 9857284 + c69dea9 commit c62b732
Show file tree
Hide file tree
Showing 136 changed files with 1,964 additions and 1,542 deletions.
10 changes: 8 additions & 2 deletions compiler/rustc_borrowck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,6 +829,7 @@ use self::ReadOrWrite::{Activation, Read, Reservation, Write};

#[derive(Copy, Clone, PartialEq, Eq, Debug)]
enum ArtificialField {
ArrayLength,
FakeBorrow,
}

Expand Down Expand Up @@ -1338,11 +1339,16 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
);
}

&Rvalue::Discriminant(place) => {
&(Rvalue::Len(place) | Rvalue::Discriminant(place)) => {
let af = match *rvalue {
Rvalue::Len(..) => Some(ArtificialField::ArrayLength),
Rvalue::Discriminant(..) => None,
_ => unreachable!(),
};
self.access_place(
location,
(place, span),
(Shallow(None), Read(ReadKind::Copy)),
(Shallow(af), Read(ReadKind::Copy)),
LocalMutationIsAllowed::No,
state,
);
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_borrowck/src/places_conflict.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,8 @@ fn place_components_conflict<'tcx>(
let base_ty = base.ty(body, tcx).ty;

match (elem, base_ty.kind(), access) {
(_, _, Shallow(Some(ArtificialField::FakeBorrow))) => {
(_, _, Shallow(Some(ArtificialField::ArrayLength)))
| (_, _, Shallow(Some(ArtificialField::FakeBorrow))) => {
// The array length is like additional fields on the
// type; it does not overlap any existing data there.
// Furthermore, if cannot actually be a prefix of any
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,11 +300,16 @@ impl<'a, 'tcx> LoanInvalidationsGenerator<'a, 'tcx> {
self.consume_operand(location, op);
}

&Rvalue::Discriminant(place) => {
&(Rvalue::Len(place) | Rvalue::Discriminant(place)) => {
let af = match rvalue {
Rvalue::Len(..) => Some(ArtificialField::ArrayLength),
Rvalue::Discriminant(..) => None,
_ => unreachable!(),
};
self.access_place(
location,
place,
(Shallow(None), Read(ReadKind::Copy)),
(Shallow(af), Read(ReadKind::Copy)),
LocalMutationIsAllowed::No,
);
}
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_borrowck/src/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2235,6 +2235,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {

Rvalue::RawPtr(..)
| Rvalue::ThreadLocalRef(..)
| Rvalue::Len(..)
| Rvalue::Discriminant(..)
| Rvalue::NullaryOp(NullOp::OffsetOf(..), _) => {}
}
Expand All @@ -2250,6 +2251,7 @@ impl<'a, 'tcx> TypeChecker<'a, 'tcx> {
| Rvalue::Repeat(..)
| Rvalue::Ref(..)
| Rvalue::RawPtr(..)
| Rvalue::Len(..)
| Rvalue::Cast(..)
| Rvalue::ShallowInitBox(..)
| Rvalue::BinaryOp(..)
Expand Down
6 changes: 6 additions & 0 deletions compiler/rustc_codegen_cranelift/src/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,12 @@ fn codegen_stmt<'tcx>(
fx.bcx.ins().nop();
}
}
Rvalue::Len(place) => {
let place = codegen_place(fx, place);
let usize_layout = fx.layout_of(fx.tcx.types.usize);
let len = codegen_array_len(fx, place);
lval.write_cvalue(fx, CValue::by_val(len, usize_layout));
}
Rvalue::ShallowInitBox(ref operand, content_ty) => {
let content_ty = fx.monomorphize(content_ty);
let box_layout = fx.layout_of(Ty::new_box(fx.tcx, content_ty));
Expand Down
29 changes: 28 additions & 1 deletion compiler/rustc_codegen_ssa/src/mir/rvalue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ use rustc_session::config::OptLevel;
use rustc_span::{DUMMY_SP, Span};
use tracing::{debug, instrument};

use super::FunctionCx;
use super::operand::{OperandRef, OperandValue};
use super::place::PlaceRef;
use super::{FunctionCx, LocalRef};
use crate::common::IntPredicate;
use crate::traits::*;
use crate::{MemFlags, base};
Expand Down Expand Up @@ -607,6 +607,14 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
self.codegen_place_to_pointer(bx, place, mk_ptr)
}

mir::Rvalue::Len(place) => {
let size = self.evaluate_array_len(bx, place);
OperandRef {
val: OperandValue::Immediate(size),
layout: bx.cx().layout_of(bx.tcx().types.usize),
}
}

mir::Rvalue::BinaryOp(op_with_overflow, box (ref lhs, ref rhs))
if let Some(op) = op_with_overflow.overflowing_to_wrapping() =>
{
Expand Down Expand Up @@ -806,6 +814,24 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
}
}

fn evaluate_array_len(&mut self, bx: &mut Bx, place: mir::Place<'tcx>) -> Bx::Value {
// ZST are passed as operands and require special handling
// because codegen_place() panics if Local is operand.
if let Some(index) = place.as_local() {
if let LocalRef::Operand(op) = self.locals[index] {
if let ty::Array(_, n) = op.layout.ty.kind() {
let n = n
.try_to_target_usize(bx.tcx())
.expect("expected monomorphic const in codegen");
return bx.cx().const_usize(n);
}
}
}
// use common size calculation for non zero-sized types
let cg_value = self.codegen_place(bx, place.as_ref());
cg_value.len(bx.cx())
}

/// Codegen an `Rvalue::RawPtr` or `Rvalue::Ref`
fn codegen_place_to_pointer(
&mut self,
Expand Down Expand Up @@ -1077,6 +1103,7 @@ impl<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> {
mir::Rvalue::Ref(..) |
mir::Rvalue::CopyForDeref(..) |
mir::Rvalue::RawPtr(..) |
mir::Rvalue::Len(..) |
mir::Rvalue::Cast(..) | // (*)
mir::Rvalue::ShallowInitBox(..) | // (*)
mir::Rvalue::BinaryOp(..) |
Expand Down
28 changes: 7 additions & 21 deletions compiler/rustc_const_eval/src/check_consts/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,8 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
Rvalue::Use(_)
| Rvalue::CopyForDeref(..)
| Rvalue::Repeat(..)
| Rvalue::Discriminant(..) => {}
| Rvalue::Discriminant(..)
| Rvalue::Len(_) => {}

Rvalue::Aggregate(kind, ..) => {
if let AggregateKind::Coroutine(def_id, ..) = kind.as_ref()
Expand Down Expand Up @@ -579,27 +580,12 @@ impl<'tcx> Visitor<'tcx> for Checker<'_, 'tcx> {
) => {}
Rvalue::ShallowInitBox(_, _) => {}

Rvalue::UnaryOp(op, operand) => {
Rvalue::UnaryOp(_, operand) => {
let ty = operand.ty(self.body, self.tcx);
match op {
UnOp::Not | UnOp::Neg => {
if is_int_bool_float_or_char(ty) {
// Int, bool, float, and char operations are fine.
} else {
span_bug!(
self.span,
"non-primitive type in `Rvalue::UnaryOp{op:?}`: {ty:?}",
);
}
}
UnOp::PtrMetadata => {
if !ty.is_ref() && !ty.is_unsafe_ptr() {
span_bug!(
self.span,
"non-pointer type in `Rvalue::UnaryOp({op:?})`: {ty:?}",
);
}
}
if is_int_bool_float_or_char(ty) {
// Int, bool, float, and char operations are fine.
} else {
span_bug!(self.span, "non-primitive type in `Rvalue::UnaryOp`: {:?}", ty);
}
}

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_const_eval/src/check_consts/qualifs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,9 @@ where
Q::in_any_value_of_ty(cx, rvalue.ty(cx.body, cx.tcx))
}

Rvalue::Discriminant(place) => in_place::<Q, _>(cx, in_local, place.as_ref()),
Rvalue::Discriminant(place) | Rvalue::Len(place) => {
in_place::<Q, _>(cx, in_local, place.as_ref())
}

Rvalue::CopyForDeref(place) => in_place::<Q, _>(cx, in_local, place.as_ref()),

Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_const_eval/src/check_consts/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ where
| mir::Rvalue::CopyForDeref(..)
| mir::Rvalue::ThreadLocalRef(..)
| mir::Rvalue::Repeat(..)
| mir::Rvalue::Len(..)
| mir::Rvalue::BinaryOp(..)
| mir::Rvalue::NullaryOp(..)
| mir::Rvalue::UnaryOp(..)
Expand Down
21 changes: 9 additions & 12 deletions compiler/rustc_const_eval/src/interpret/step.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,12 @@ use rustc_middle::ty::layout::FnAbiOf;
use rustc_middle::ty::{self, Instance, Ty};
use rustc_middle::{bug, mir, span_bug};
use rustc_span::source_map::Spanned;
use rustc_span::{DesugaringKind, Span};
use rustc_target::callconv::FnAbi;
use tracing::{info, instrument, trace};

use super::{
FnArg, FnVal, ImmTy, Immediate, InterpCx, InterpResult, Machine, MemPlaceMeta, PlaceTy,
Projectable, interp_ok, throw_ub,
Projectable, Scalar, interp_ok, throw_ub,
};
use crate::util;

Expand Down Expand Up @@ -81,9 +80,7 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
use rustc_middle::mir::StatementKind::*;

match &stmt.kind {
Assign(box (place, rvalue)) => {
self.eval_rvalue_into_place(rvalue, *place, stmt.source_info.span)?
}
Assign(box (place, rvalue)) => self.eval_rvalue_into_place(rvalue, *place)?,

SetDiscriminant { place, variant_index } => {
let dest = self.eval_place(**place)?;
Expand Down Expand Up @@ -162,7 +159,6 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
&mut self,
rvalue: &mir::Rvalue<'tcx>,
place: mir::Place<'tcx>,
span: Span,
) -> InterpResult<'tcx> {
let dest = self.eval_place(place)?;
// FIXME: ensure some kind of non-aliasing between LHS and RHS?
Expand Down Expand Up @@ -218,6 +214,12 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
self.write_repeat(operand, &dest)?;
}

Len(place) => {
let src = self.eval_place(place)?;
let len = src.len(self)?;
self.write_scalar(Scalar::from_target_usize(len, self), &dest)?;
}

Ref(_, borrow_kind, place) => {
let src = self.eval_place(place)?;
let place = self.force_allocation(&src)?;
Expand Down Expand Up @@ -248,13 +250,8 @@ impl<'tcx, M: Machine<'tcx>> InterpCx<'tcx, M> {
let src = self.eval_place(place)?;
let place = self.force_allocation(&src)?;
let mut val = ImmTy::from_immediate(place.to_ref(self), dest.layout);
if !place_base_raw
&& span.desugaring_kind() != Some(DesugaringKind::IndexBoundsCheckReborrow)
{
if !place_base_raw {
// If this was not already raw, it needs retagging.
// As a special hack, we exclude the desugared `PtrMetadata(&raw const *_n)`
// from indexing. (Really we should not do any retag on `&raw` but that does not
// currently work with Stacked Borrows.)
val = M::retag_ptr_value(self, mir::RetagKind::Raw, &val)?;
}
self.write_immediate(*val, &dest)?;
Expand Down
3 changes: 0 additions & 3 deletions compiler/rustc_middle/src/mir/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,9 +467,6 @@ impl<'tcx> Const<'tcx> {
let const_val = tcx.valtree_to_const_val((ty, valtree));
Self::Val(const_val, ty)
}
ty::ConstKind::Unevaluated(ty::UnevaluatedConst { def, args }) => {
Self::Unevaluated(UnevaluatedConst { def, args, promoted: None }, ty)
}
_ => Self::Ty(ty, c),
}
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/pretty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,6 +1068,7 @@ impl<'tcx> Debug for Rvalue<'tcx> {
pretty_print_const(b, fmt, false)?;
write!(fmt, "]")
}
Len(ref a) => write!(fmt, "Len({a:?})"),
Cast(ref kind, ref place, ref ty) => {
with_no_trimmed_paths!(write!(fmt, "{place:?} as {ty} ({kind:?})"))
}
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ impl<'tcx> Rvalue<'tcx> {
| Rvalue::Ref(_, _, _)
| Rvalue::ThreadLocalRef(_)
| Rvalue::RawPtr(_, _)
| Rvalue::Len(_)
| Rvalue::Cast(
CastKind::IntToInt
| CastKind::FloatToInt
Expand Down
10 changes: 10 additions & 0 deletions compiler/rustc_middle/src/mir/syntax.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1351,6 +1351,16 @@ pub enum Rvalue<'tcx> {
/// model.
RawPtr(Mutability, Place<'tcx>),

/// Yields the length of the place, as a `usize`.
///
/// If the type of the place is an array, this is the array length. For slices (`[T]`, not
/// `&[T]`) this accesses the place's metadata to determine the length. This rvalue is
/// ill-formed for places of other types.
///
/// This cannot be a `UnOp(PtrMetadata, _)` because that expects a value, and we only
/// have a place, and `UnOp(PtrMetadata, RawPtr(place))` is not a thing.
Len(Place<'tcx>),

/// Performs essentially all of the casts that can be performed via `as`.
///
/// This allows for casts from/to a variety of types.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_middle/src/mir/tcx.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ impl<'tcx> Rvalue<'tcx> {
let place_ty = place.ty(local_decls, tcx).ty;
Ty::new_ptr(tcx, place_ty, mutability)
}
Rvalue::Len(..) => tcx.types.usize,
Rvalue::Cast(.., ty) => ty,
Rvalue::BinaryOp(op, box (ref lhs, ref rhs)) => {
let lhs_ty = lhs.ty(local_decls, tcx);
Expand Down
8 changes: 8 additions & 0 deletions compiler/rustc_middle/src/mir/visit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,14 @@ macro_rules! make_mir_visitor {
self.visit_place(path, ctx, location);
}

Rvalue::Len(path) => {
self.visit_place(
path,
PlaceContext::NonMutatingUse(NonMutatingUseContext::Inspect),
location
);
}

Rvalue::Cast(_cast_kind, operand, ty) => {
self.visit_operand(operand, location);
self.visit_ty($(& $mutability)? *ty, TyContext::Location(location));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ impl<'a, 'tcx> ParseCtxt<'a, 'tcx> {
let offset = self.parse_operand(args[1])?;
Ok(Rvalue::BinaryOp(BinOp::Offset, Box::new((ptr, offset))))
},
@call(mir_len, args) => Ok(Rvalue::Len(self.parse_place(args[0])?)),
@call(mir_ptr_metadata, args) => Ok(Rvalue::UnaryOp(UnOp::PtrMetadata, self.parse_operand(args[0])?)),
@call(mir_copy_for_deref, args) => Ok(Rvalue::CopyForDeref(self.parse_place(args[0])?)),
ExprKind::Borrow { borrow_kind, arg } => Ok(
Expand Down
Loading

0 comments on commit c62b732

Please sign in to comment.