From 410188978622ae80ed051217ec65999e394a3c15 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Nov 2022 10:18:32 +0100 Subject: [PATCH 1/2] interpret: use Either over Result when it is not representing an error condition --- Cargo.lock | 3 ++ compiler/rustc_const_eval/Cargo.toml | 1 + .../src/const_eval/eval_queries.rs | 29 +++++++++-------- .../src/interpret/eval_context.rs | 32 +++++++++---------- .../interpret/intrinsics/caller_location.rs | 4 +-- .../rustc_const_eval/src/interpret/operand.rs | 32 ++++++++++--------- .../rustc_const_eval/src/interpret/place.rs | 28 ++++++++-------- .../src/interpret/projection.rs | 8 +++-- .../rustc_const_eval/src/interpret/step.rs | 8 +++-- .../src/interpret/validity.rs | 8 +++-- .../src/mir/interpret/allocation.rs | 8 +++-- .../rustc_middle/src/mir/interpret/value.rs | 12 ++++--- compiler/rustc_mir_build/Cargo.toml | 1 + .../src/thir/pattern/deconstruct_pat.rs | 28 ++++++++-------- compiler/rustc_mir_transform/Cargo.toml | 1 + .../rustc_mir_transform/src/const_prop.rs | 6 ++-- .../src/const_prop_lint.rs | 19 +++++++---- 17 files changed, 130 insertions(+), 98 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c867bb4ceaa..9059a336bd6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3330,6 +3330,7 @@ dependencies = [ name = "rustc_const_eval" version = "0.0.0" dependencies = [ + "either", "rustc_apfloat", "rustc_ast", "rustc_attr", @@ -3820,6 +3821,7 @@ dependencies = [ name = "rustc_mir_build" version = "0.0.0" dependencies = [ + "either", "rustc_apfloat", "rustc_arena", "rustc_ast", @@ -3866,6 +3868,7 @@ name = "rustc_mir_transform" version = "0.0.0" dependencies = [ "coverage_test_macros", + "either", "itertools", "rustc_ast", "rustc_attr", diff --git a/compiler/rustc_const_eval/Cargo.toml b/compiler/rustc_const_eval/Cargo.toml index e09a6d1d6f5..51489e29360 100644 --- a/compiler/rustc_const_eval/Cargo.toml +++ b/compiler/rustc_const_eval/Cargo.toml @@ -7,6 +7,7 @@ edition = "2021" [dependencies] tracing = "0.1" +either = "1" rustc_apfloat = { path = "../rustc_apfloat" } rustc_ast = { path = "../rustc_ast" } rustc_attr = { path = "../rustc_attr" } diff --git a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs index f5942deaf9f..c777a840f3f 100644 --- a/compiler/rustc_const_eval/src/const_eval/eval_queries.rs +++ b/compiler/rustc_const_eval/src/const_eval/eval_queries.rs @@ -1,10 +1,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, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, - RefTracking, StackPopCleanup, -}; +use std::borrow::Cow; +use std::convert::TryInto; + +use either::{Left, Right}; use rustc_hir::def::DefKind; use rustc_middle::mir; @@ -16,8 +13,14 @@ use rustc_middle::ty::print::with_no_trimmed_paths; use rustc_middle::ty::{self, TyCtxt}; use rustc_span::source_map::Span; use rustc_target::abi::{self, Abi}; -use std::borrow::Cow; -use std::convert::TryInto; + +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, InterpError, InterpResult, MPlaceTy, MemoryKind, OpTy, + RefTracking, StackPopCleanup, +}; const NOTE_ON_UNDEFINED_BEHAVIOR_ERROR: &str = "The rules on what exactly is undefined behavior aren't clear, \ so this check might be overzealous. Please open an issue on the rustc \ @@ -135,14 +138,14 @@ pub(super) fn op_to_const<'tcx>( _ => false, }; let immediate = if try_as_immediate { - Err(ecx.read_immediate(op).expect("normalization works on validated constants")) + Right(ecx.read_immediate(op).expect("normalization works on validated constants")) } else { // It is guaranteed that any non-slice scalar pair is actually ByRef here. // When we come back from raw const eval, we are always by-ref. The only way our op here is // by-val is if we are in destructure_mir_constant, i.e., if this is (a field of) something that we // "tried to make immediate" before. We wouldn't do that for non-slice scalar pairs or // structs containing such. - op.try_as_mplace() + op.as_mplace_or_imm() }; debug!(?immediate); @@ -168,9 +171,9 @@ pub(super) fn op_to_const<'tcx>( } }; match immediate { - Ok(ref mplace) => to_const_value(mplace), + Left(ref mplace) => to_const_value(mplace), // see comment on `let try_as_immediate` above - Err(imm) => match *imm { + Right(imm) => match *imm { _ if imm.layout.is_zst() => ConstValue::ZeroSized, Immediate::Scalar(x) => ConstValue::Scalar(x), Immediate::ScalarPair(a, b) => { diff --git a/compiler/rustc_const_eval/src/interpret/eval_context.rs b/compiler/rustc_const_eval/src/interpret/eval_context.rs index ed69d8554d2..225905cae3a 100644 --- a/compiler/rustc_const_eval/src/interpret/eval_context.rs +++ b/compiler/rustc_const_eval/src/interpret/eval_context.rs @@ -2,6 +2,8 @@ use std::cell::Cell; use std::fmt; use std::mem; +use either::{Either, Left, Right}; + use rustc_hir::{self as hir, def_id::DefId, definitions::DefPathData}; use rustc_index::vec::IndexVec; use rustc_middle::mir; @@ -121,13 +123,12 @@ pub struct Frame<'mir, 'tcx, Prov: Provenance = AllocId, Extra = ()> { //////////////////////////////////////////////////////////////////////////////// // Current position within the function //////////////////////////////////////////////////////////////////////////////// - /// If this is `Err`, we are not currently executing any particular statement in + /// If this is `Right`, we are not currently executing any particular statement in /// this frame (can happen e.g. during frame initialization, and during unwinding on /// frames without cleanup code). - /// We basically abuse `Result` as `Either`. /// /// Needs to be public because ConstProp does unspeakable things to it. - pub loc: Result, + pub loc: Either, } /// What we store about a frame in an interpreter backtrace. @@ -227,25 +228,24 @@ impl<'mir, 'tcx, Prov: Provenance> Frame<'mir, 'tcx, Prov> { impl<'mir, 'tcx, Prov: Provenance, Extra> Frame<'mir, 'tcx, Prov, Extra> { /// Get the current location within the Frame. /// - /// If this is `Err`, we are not currently executing any particular statement in + /// If this is `Left`, we are not currently executing any particular statement in /// this frame (can happen e.g. during frame initialization, and during unwinding on /// frames without cleanup code). - /// We basically abuse `Result` as `Either`. /// /// Used by priroda. - pub fn current_loc(&self) -> Result { + pub fn current_loc(&self) -> Either { self.loc } /// Return the `SourceInfo` of the current instruction. pub fn current_source_info(&self) -> Option<&mir::SourceInfo> { - self.loc.ok().map(|loc| self.body.source_info(loc)) + self.loc.left().map(|loc| self.body.source_info(loc)) } pub fn current_span(&self) -> Span { match self.loc { - Ok(loc) => self.body.source_info(loc).span, - Err(span) => span, + Left(loc) => self.body.source_info(loc).span, + Right(span) => span, } } } @@ -679,7 +679,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // first push a stack frame so we have access to the local substs let pre_frame = Frame { body, - loc: Err(body.span), // Span used for errors caused during preamble. + loc: Right(body.span), // Span used for errors caused during preamble. return_to_block, return_place: return_place.clone(), // empty local array, we fill it in below, after we are inside the stack frame and @@ -713,7 +713,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // done self.frame_mut().locals = locals; M::after_stack_push(self)?; - self.frame_mut().loc = Ok(mir::Location::START); + self.frame_mut().loc = Left(mir::Location::START); let span = info_span!("frame", "{}", instance); self.frame_mut().tracing_span.enter(span); @@ -724,7 +724,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Jump to the given block. #[inline] pub fn go_to_block(&mut self, target: mir::BasicBlock) { - self.frame_mut().loc = Ok(mir::Location { block: target, statement_index: 0 }); + self.frame_mut().loc = Left(mir::Location { block: target, statement_index: 0 }); } /// *Return* to the given `target` basic block. @@ -750,8 +750,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// unwinding, and doing so is UB. pub fn unwind_to_block(&mut self, target: StackPopUnwind) -> InterpResult<'tcx> { self.frame_mut().loc = match target { - StackPopUnwind::Cleanup(block) => Ok(mir::Location { block, statement_index: 0 }), - StackPopUnwind::Skip => Err(self.frame_mut().body.span), + StackPopUnwind::Cleanup(block) => Left(mir::Location { block, statement_index: 0 }), + StackPopUnwind::Skip => Right(self.frame_mut().body.span), StackPopUnwind::NotAllowed => { throw_ub_format!("unwinding past a stack frame that does not allow unwinding") } @@ -783,8 +783,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { assert_eq!( unwinding, match self.frame().loc { - Ok(loc) => self.body().basic_blocks[loc.block].is_cleanup, - Err(_) => true, + Left(loc) => self.body().basic_blocks[loc.block].is_cleanup, + Right(_) => true, } ); if unwinding && self.frame_idx() == 0 { diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs index 0e3867557ad..7d94a22c43d 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics/caller_location.rs @@ -19,8 +19,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { debug!("find_closest_untracked_caller_location: checking frame {:?}", frame.instance); // Assert that the frame we look at is actually executing code currently - // (`loc` is `Err` when we are unwinding and the frame does not require cleanup). - let loc = frame.loc.unwrap(); + // (`loc` is `Right` when we are unwinding and the frame does not require cleanup). + let loc = frame.loc.left().unwrap(); // This could be a non-`Call` terminator (such as `Drop`), or not a terminator at all // (such as `box`). Use the normal span by default. diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index cf9202540c7..1b00e5a8244 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -1,6 +1,8 @@ //! Functions concerning immediate values and operands, and reading from operands. //! All high-level functions to read from memory work on operands as sources. +use either::{Either, Left, Right}; + use rustc_hir::def::Namespace; use rustc_middle::ty::layout::{LayoutOf, PrimitiveExt, TyAndLayout}; use rustc_middle::ty::print::{FmtPrinter, PrettyPrinter}; @@ -261,9 +263,9 @@ impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> { layout: TyAndLayout<'tcx>, cx: &impl HasDataLayout, ) -> InterpResult<'tcx, Self> { - match self.try_as_mplace() { - Ok(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, cx)?.into()), - Err(imm) => { + match self.as_mplace_or_imm() { + Left(mplace) => Ok(mplace.offset_with_meta(offset, meta, layout, cx)?.into()), + Right(imm) => { assert!( matches!(*imm, Immediate::Uninit), "Scalar/ScalarPair cannot be offset into" @@ -353,8 +355,8 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { /// Try returning an immediate for the operand. If the layout does not permit loading this as an /// immediate, return where in memory we can find the data. - /// Note that for a given layout, this operation will either always fail or always - /// succeed! Whether it succeeds depends on whether the layout can be represented + /// Note that for a given layout, this operation will either always return Left or Right! + /// succeed! Whether it returns Left depends on whether the layout can be represented /// in an `Immediate`, not on which data is stored there currently. /// /// This is an internal function that should not usually be used; call `read_immediate` instead. @@ -362,16 +364,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { pub fn read_immediate_raw( &self, src: &OpTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, Result, MPlaceTy<'tcx, M::Provenance>>> { - Ok(match src.try_as_mplace() { - Ok(ref mplace) => { + ) -> InterpResult<'tcx, Either, MPlaceTy<'tcx, M::Provenance>>> { + Ok(match src.as_mplace_or_imm() { + Left(ref mplace) => { if let Some(val) = self.read_immediate_from_mplace_raw(mplace)? { - Ok(val) + Left(val) } else { - Err(*mplace) + Right(*mplace) } } - Err(val) => Ok(val), + Right(val) => Left(val), }) } @@ -390,7 +392,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) { span_bug!(self.cur_span(), "primitive read not possible for type: {:?}", op.layout.ty); } - let imm = self.read_immediate_raw(op)?.unwrap(); + let imm = self.read_immediate_raw(op)?.left().unwrap(); if matches!(*imm, Immediate::Uninit) { throw_ub!(InvalidUninitBytes(None)); } @@ -432,9 +434,9 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Basically we just transmute this place into an array following simd_size_and_type. // This only works in memory, but repr(simd) types should never be immediates anyway. assert!(op.layout.ty.is_simd()); - match op.try_as_mplace() { - Ok(mplace) => self.mplace_to_simd(&mplace), - Err(imm) => match *imm { + match op.as_mplace_or_imm() { + Left(mplace) => self.mplace_to_simd(&mplace), + Right(imm) => match *imm { Immediate::Uninit => { throw_ub!(InvalidUninitBytes(None)) } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 29d2312612e..69e07fa0b27 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -2,6 +2,8 @@ //! into a place. //! All high-level functions to write to memory work on places as destinations. +use either::{Either, Left, Right}; + use rustc_ast::Mutability; use rustc_middle::mir; use rustc_middle::ty; @@ -252,36 +254,36 @@ impl<'tcx, Prov: Provenance> MPlaceTy<'tcx, Prov> { // These are defined here because they produce a place. impl<'tcx, Prov: Provenance> OpTy<'tcx, Prov> { #[inline(always)] - pub fn try_as_mplace(&self) -> Result, ImmTy<'tcx, Prov>> { + pub fn as_mplace_or_imm(&self) -> Either, ImmTy<'tcx, Prov>> { match **self { Operand::Indirect(mplace) => { - Ok(MPlaceTy { mplace, layout: self.layout, align: self.align.unwrap() }) + Left(MPlaceTy { mplace, layout: self.layout, align: self.align.unwrap() }) } - Operand::Immediate(imm) => Err(ImmTy::from_immediate(imm, self.layout)), + Operand::Immediate(imm) => Right(ImmTy::from_immediate(imm, self.layout)), } } #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980) pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Prov> { - self.try_as_mplace().unwrap() + self.as_mplace_or_imm().left().unwrap() } } impl<'tcx, Prov: Provenance> PlaceTy<'tcx, Prov> { /// A place is either an mplace or some local. #[inline] - pub fn try_as_mplace(&self) -> Result, (usize, mir::Local)> { + pub fn as_mplace_or_local(&self) -> Either, (usize, mir::Local)> { match **self { - Place::Ptr(mplace) => Ok(MPlaceTy { mplace, layout: self.layout, align: self.align }), - Place::Local { frame, local } => Err((frame, local)), + Place::Ptr(mplace) => Left(MPlaceTy { mplace, layout: self.layout, align: self.align }), + Place::Local { frame, local } => Right((frame, local)), } } #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] // only in debug builds due to perf (see #98980) pub fn assert_mem_place(&self) -> MPlaceTy<'tcx, Prov> { - self.try_as_mplace().unwrap() + self.as_mplace_or_local().left().unwrap() } } @@ -569,9 +571,9 @@ where } pub fn write_uninit(&mut self, dest: &PlaceTy<'tcx, M::Provenance>) -> InterpResult<'tcx> { - let mplace = match dest.try_as_mplace() { - Ok(mplace) => mplace, - Err((frame, local)) => { + let mplace = match dest.as_mplace_or_local() { + Left(mplace) => mplace, + Right((frame, local)) => { match M::access_local_mut(self, frame, local)? { Operand::Immediate(local) => { *local = Immediate::Uninit; @@ -639,7 +641,7 @@ where // Let us see if the layout is simple so we take a shortcut, // avoid force_allocation. let src = match self.read_immediate_raw(src)? { - Ok(src_val) => { + Left(src_val) => { // FIXME(const_prop): Const-prop can possibly evaluate an // unsized copy operation when it thinks that the type is // actually sized, due to a trivially false where-clause @@ -669,7 +671,7 @@ where ) }; } - Err(mplace) => mplace, + Right(mplace) => mplace, }; // Slow path, this does not fit into an immediate. Just memcpy. trace!("copy_op: {:?} <- {:?}: {}", *dest, src, dest.layout.ty); diff --git a/compiler/rustc_const_eval/src/interpret/projection.rs b/compiler/rustc_const_eval/src/interpret/projection.rs index 6b2e2bb8aca..4966fd6ea80 100644 --- a/compiler/rustc_const_eval/src/interpret/projection.rs +++ b/compiler/rustc_const_eval/src/interpret/projection.rs @@ -7,6 +7,8 @@ //! but we still need to do bounds checking and adjust the layout. To not duplicate that with MPlaceTy, we actually //! implement the logic on OpTy, and MPlaceTy calls that. +use either::{Left, Right}; + use rustc_middle::mir; use rustc_middle::ty; use rustc_middle::ty::layout::LayoutOf; @@ -84,13 +86,13 @@ where base: &OpTy<'tcx, M::Provenance>, field: usize, ) -> InterpResult<'tcx, OpTy<'tcx, M::Provenance>> { - let base = match base.try_as_mplace() { - Ok(ref mplace) => { + let base = match base.as_mplace_or_imm() { + Left(ref mplace) => { // We can reuse the mplace field computation logic for indirect operands. let field = self.mplace_field(mplace, field)?; return Ok(field.into()); } - Err(value) => value, + Right(value) => value, }; let field_layout = base.layout.field(self, field); diff --git a/compiler/rustc_const_eval/src/interpret/step.rs b/compiler/rustc_const_eval/src/interpret/step.rs index 3c286fa61be..73f8bf4362e 100644 --- a/compiler/rustc_const_eval/src/interpret/step.rs +++ b/compiler/rustc_const_eval/src/interpret/step.rs @@ -2,6 +2,8 @@ //! //! The main entry point is the `step` method. +use either::Either; + use rustc_middle::mir; use rustc_middle::mir::interpret::{InterpResult, Scalar}; use rustc_middle::ty::layout::LayoutOf; @@ -46,7 +48,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(false); } - let Ok(loc) = self.frame().loc else { + let Either::Left(loc) = self.frame().loc else { // We are unwinding and this fn has no cleanup code. // Just go on unwinding. trace!("unwinding: skipping frame"); @@ -61,7 +63,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Make sure we are not updating `statement_index` of the wrong frame. assert_eq!(old_frames, self.frame_idx()); // Advance the program counter. - self.frame_mut().loc.as_mut().unwrap().statement_index += 1; + self.frame_mut().loc.as_mut().left().unwrap().statement_index += 1; return Ok(true); } @@ -305,7 +307,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.eval_terminator(terminator)?; if !self.stack().is_empty() { - if let Ok(loc) = self.frame().loc { + if let Either::Left(loc) = self.frame().loc { info!("// executing {:?}", loc.block); } } diff --git a/compiler/rustc_const_eval/src/interpret/validity.rs b/compiler/rustc_const_eval/src/interpret/validity.rs index 8aa56c275d9..cd7a472c0f0 100644 --- a/compiler/rustc_const_eval/src/interpret/validity.rs +++ b/compiler/rustc_const_eval/src/interpret/validity.rs @@ -8,6 +8,8 @@ use std::convert::TryFrom; use std::fmt::{Display, Write}; use std::num::NonZeroUsize; +use either::{Left, Right}; + use rustc_ast::Mutability; use rustc_data_structures::fx::FxHashSet; use rustc_hir as hir; @@ -852,9 +854,9 @@ impl<'rt, 'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> return Ok(()); } // Now that we definitely have a non-ZST array, we know it lives in memory. - let mplace = match op.try_as_mplace() { - Ok(mplace) => mplace, - Err(imm) => match *imm { + let mplace = match op.as_mplace_or_imm() { + Left(mplace) => mplace, + Right(imm) => match *imm { Immediate::Uninit => throw_validation_failure!(self.path, { "uninitialized bytes" }), Immediate::Scalar(..) | Immediate::ScalarPair(..) => diff --git a/compiler/rustc_middle/src/mir/interpret/allocation.rs b/compiler/rustc_middle/src/mir/interpret/allocation.rs index 68024070346..f8a69f3c7d5 100644 --- a/compiler/rustc_middle/src/mir/interpret/allocation.rs +++ b/compiler/rustc_middle/src/mir/interpret/allocation.rs @@ -11,6 +11,8 @@ use std::hash; use std::ops::Range; use std::ptr; +use either::{Left, Right}; + use rustc_ast::Mutability; use rustc_data_structures::intern::Interned; use rustc_span::DUMMY_SP; @@ -503,11 +505,11 @@ impl Allocation { // `to_bits_or_ptr_internal` is the right method because we just want to store this data // as-is into memory. let (bytes, provenance) = match val.to_bits_or_ptr_internal(range.size)? { - Err(val) => { - let (provenance, offset) = val.into_parts(); + Right(ptr) => { + let (provenance, offset) = ptr.into_parts(); (u128::from(offset.bytes()), Some(provenance)) } - Ok(data) => (data, None), + Left(data) => (data, None), }; let endian = cx.data_layout().endian; diff --git a/compiler/rustc_middle/src/mir/interpret/value.rs b/compiler/rustc_middle/src/mir/interpret/value.rs index ac5fddb7ad1..770c3ed05e8 100644 --- a/compiler/rustc_middle/src/mir/interpret/value.rs +++ b/compiler/rustc_middle/src/mir/interpret/value.rs @@ -1,6 +1,8 @@ use std::convert::{TryFrom, TryInto}; use std::fmt; +use either::{Either, Left, Right}; + use rustc_apfloat::{ ieee::{Double, Single}, Float, @@ -293,10 +295,10 @@ impl Scalar { pub fn to_bits_or_ptr_internal( self, target_size: Size, - ) -> Result>, ScalarSizeMismatch> { + ) -> Result>, ScalarSizeMismatch> { assert_ne!(target_size.bytes(), 0, "you should never look at the bits of a ZST"); Ok(match self { - Scalar::Int(int) => Ok(int.to_bits(target_size).map_err(|size| { + Scalar::Int(int) => Left(int.to_bits(target_size).map_err(|size| { ScalarSizeMismatch { target_size: target_size.bytes(), data_size: size.bytes() } })?), Scalar::Ptr(ptr, sz) => { @@ -306,7 +308,7 @@ impl Scalar { data_size: sz.into(), }); } - Err(ptr) + Right(ptr) } }) } @@ -318,8 +320,8 @@ impl<'tcx, Prov: Provenance> Scalar { .to_bits_or_ptr_internal(cx.pointer_size()) .map_err(|s| err_ub!(ScalarSizeMismatch(s)))? { - Err(ptr) => Ok(ptr.into()), - Ok(bits) => { + Right(ptr) => Ok(ptr.into()), + Left(bits) => { let addr = u64::try_from(bits).unwrap(); Ok(Pointer::from_addr(addr)) } diff --git a/compiler/rustc_mir_build/Cargo.toml b/compiler/rustc_mir_build/Cargo.toml index c726fa3a33f..2baa3bfcb64 100644 --- a/compiler/rustc_mir_build/Cargo.toml +++ b/compiler/rustc_mir_build/Cargo.toml @@ -8,6 +8,7 @@ edition = "2021" [dependencies] rustc_arena = { path = "../rustc_arena" } tracing = "0.1" +either = "1" rustc_middle = { path = "../rustc_middle" } rustc_apfloat = { path = "../rustc_apfloat" } rustc_attr = { path = "../rustc_attr" } diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 595abc8f668..4cde19f846f 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -42,16 +42,17 @@ //! wildcards, see [`SplitWildcard`]; for integer ranges, see [`SplitIntRange`]; for slices, see //! [`SplitVarLenSlice`]. -use self::Constructor::*; -use self::SliceKind::*; +use std::cell::Cell; +use std::cmp::{self, max, min, Ordering}; +use std::fmt; +use std::iter::{once, IntoIterator}; +use std::ops::RangeInclusive; -use super::compare_const_vals; -use super::usefulness::{MatchCheckCtxt, PatCtxt}; +use smallvec::{smallvec, SmallVec}; use rustc_data_structures::captures::Captures; -use rustc_index::vec::Idx; - use rustc_hir::{HirId, RangeEnd}; +use rustc_index::vec::Idx; use rustc_middle::mir::{self, Field}; use rustc_middle::thir::{FieldPat, Pat, PatKind, PatRange}; use rustc_middle::ty::layout::IntegerExt; @@ -61,12 +62,11 @@ use rustc_session::lint; use rustc_span::{Span, DUMMY_SP}; use rustc_target::abi::{Integer, Size, VariantIdx}; -use smallvec::{smallvec, SmallVec}; -use std::cell::Cell; -use std::cmp::{self, max, min, Ordering}; -use std::fmt; -use std::iter::{once, IntoIterator}; -use std::ops::RangeInclusive; +use self::Constructor::*; +use self::SliceKind::*; + +use super::compare_const_vals; +use super::usefulness::{MatchCheckCtxt, PatCtxt}; /// Recursively expand this pattern into its subpatterns. Only useful for or-patterns. fn expand_or_pat<'p, 'tcx>(pat: &'p Pat<'tcx>) -> Vec<&'p Pat<'tcx>> { @@ -147,7 +147,9 @@ impl IntRange { // straight to the result, after doing a bit of checking. (We // could remove this branch and just fall through, which // is more general but much slower.) - if let Ok(Ok(bits)) = scalar.to_bits_or_ptr_internal(target_size) { + if let either::Left(bits) = + scalar.to_bits_or_ptr_internal(target_size).unwrap() + { return Some(bits); } else { return None; diff --git a/compiler/rustc_mir_transform/Cargo.toml b/compiler/rustc_mir_transform/Cargo.toml index 53545cff0cb..962536669e0 100644 --- a/compiler/rustc_mir_transform/Cargo.toml +++ b/compiler/rustc_mir_transform/Cargo.toml @@ -9,6 +9,7 @@ edition = "2021" itertools = "0.10.1" smallvec = { version = "1.8.1", features = ["union", "may_dangle"] } tracing = "0.1" +either = "1" rustc_ast = { path = "../rustc_ast" } rustc_attr = { path = "../rustc_attr" } rustc_data_structures = { path = "../rustc_data_structures" } diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 3f5890040c3..3420096fe02 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -3,6 +3,8 @@ use std::cell::Cell; +use either::Left; + use rustc_ast::Mutability; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def::DefKind; @@ -429,7 +431,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // Try to read the local as an immediate so that if it is representable as a scalar, we can // handle it as such, but otherwise, just return the value as is. Some(match self.ecx.read_immediate_raw(&op) { - Ok(Ok(imm)) => imm.into(), + Ok(Left(imm)) => imm.into(), _ => op, }) } @@ -743,7 +745,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // FIXME> figure out what to do when read_immediate_raw fails let imm = self.use_ecx(|this| this.ecx.read_immediate_raw(value)); - if let Some(Ok(imm)) = imm { + if let Some(Left(imm)) = imm { match *imm { interpret::Immediate::Scalar(scalar) => { *rval = Rvalue::Use(self.operand_from_scalar( diff --git a/compiler/rustc_mir_transform/src/const_prop_lint.rs b/compiler/rustc_mir_transform/src/const_prop_lint.rs index 786063d538c..0ab67228f3f 100644 --- a/compiler/rustc_mir_transform/src/const_prop_lint.rs +++ b/compiler/rustc_mir_transform/src/const_prop_lint.rs @@ -1,10 +1,10 @@ //! Propagates constants for early reporting of statically known //! assertion failures -use crate::const_prop::CanConstProp; -use crate::const_prop::ConstPropMachine; -use crate::const_prop::ConstPropMode; -use crate::MirLint; +use std::cell::Cell; + +use either::{Left, Right}; + use rustc_const_eval::interpret::Immediate; use rustc_const_eval::interpret::{ self, InterpCx, InterpResult, LocalState, LocalValue, MemoryKind, OpTy, Scalar, StackPopCleanup, @@ -26,12 +26,17 @@ use rustc_session::lint; use rustc_span::Span; use rustc_target::abi::{HasDataLayout, Size, TargetDataLayout}; use rustc_trait_selection::traits; -use std::cell::Cell; + +use crate::const_prop::CanConstProp; +use crate::const_prop::ConstPropMachine; +use crate::const_prop::ConstPropMode; +use crate::MirLint; /// The maximum number of bytes that we'll allocate space for a local or the return value. /// Needed for #66397, because otherwise we eval into large places and that can cause OOM or just /// Severely regress performance. const MAX_ALLOC_LIMIT: u64 = 1024; + pub struct ConstProp; impl<'tcx> MirLint<'tcx> for ConstProp { @@ -243,7 +248,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // Try to read the local as an immediate so that if it is representable as a scalar, we can // handle it as such, but otherwise, just return the value as is. Some(match self.ecx.read_immediate_raw(&op) { - Ok(Ok(imm)) => imm.into(), + Ok(Left(imm)) => imm.into(), _ => op, }) } @@ -266,7 +271,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { F: FnOnce(&mut Self) -> InterpResult<'tcx, T>, { // Overwrite the PC -- whatever the interpreter does to it does not make any sense anyway. - self.ecx.frame_mut().loc = Err(source_info.span); + self.ecx.frame_mut().loc = Right(source_info.span); match f(self) { Ok(val) => Some(val), Err(error) => { From 09a887cebf917af04a45d37d9c39d6bf3072f6e1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 18 Nov 2022 14:24:48 +0100 Subject: [PATCH 2/2] review feedback --- compiler/rustc_const_eval/src/interpret/operand.rs | 10 +++++----- compiler/rustc_const_eval/src/interpret/place.rs | 4 ++-- .../src/thir/pattern/deconstruct_pat.rs | 8 +------- compiler/rustc_mir_transform/src/const_prop.rs | 6 +++--- 4 files changed, 11 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_const_eval/src/interpret/operand.rs b/compiler/rustc_const_eval/src/interpret/operand.rs index 1b00e5a8244..3eb2b3a0b1b 100644 --- a/compiler/rustc_const_eval/src/interpret/operand.rs +++ b/compiler/rustc_const_eval/src/interpret/operand.rs @@ -364,16 +364,16 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { pub fn read_immediate_raw( &self, src: &OpTy<'tcx, M::Provenance>, - ) -> InterpResult<'tcx, Either, MPlaceTy<'tcx, M::Provenance>>> { + ) -> InterpResult<'tcx, Either, ImmTy<'tcx, M::Provenance>>> { Ok(match src.as_mplace_or_imm() { Left(ref mplace) => { if let Some(val) = self.read_immediate_from_mplace_raw(mplace)? { - Left(val) + Right(val) } else { - Right(*mplace) + Left(*mplace) } } - Right(val) => Left(val), + Right(val) => Right(val), }) } @@ -392,7 +392,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) { span_bug!(self.cur_span(), "primitive read not possible for type: {:?}", op.layout.ty); } - let imm = self.read_immediate_raw(op)?.left().unwrap(); + let imm = self.read_immediate_raw(op)?.right().unwrap(); if matches!(*imm, Immediate::Uninit) { throw_ub!(InvalidUninitBytes(None)); } diff --git a/compiler/rustc_const_eval/src/interpret/place.rs b/compiler/rustc_const_eval/src/interpret/place.rs index 69e07fa0b27..c47cfe8bb69 100644 --- a/compiler/rustc_const_eval/src/interpret/place.rs +++ b/compiler/rustc_const_eval/src/interpret/place.rs @@ -641,7 +641,7 @@ where // Let us see if the layout is simple so we take a shortcut, // avoid force_allocation. let src = match self.read_immediate_raw(src)? { - Left(src_val) => { + Right(src_val) => { // FIXME(const_prop): Const-prop can possibly evaluate an // unsized copy operation when it thinks that the type is // actually sized, due to a trivially false where-clause @@ -671,7 +671,7 @@ where ) }; } - Right(mplace) => mplace, + Left(mplace) => mplace, }; // Slow path, this does not fit into an immediate. Just memcpy. trace!("copy_op: {:?} <- {:?}: {}", *dest, src, dest.layout.ty); diff --git a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs index 4cde19f846f..d60e8722cb6 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/deconstruct_pat.rs @@ -147,13 +147,7 @@ impl IntRange { // straight to the result, after doing a bit of checking. (We // could remove this branch and just fall through, which // is more general but much slower.) - if let either::Left(bits) = - scalar.to_bits_or_ptr_internal(target_size).unwrap() - { - return Some(bits); - } else { - return None; - } + return scalar.to_bits_or_ptr_internal(target_size).unwrap().left(); } mir::ConstantKind::Ty(c) => match c.kind() { ty::ConstKind::Value(_) => bug!( diff --git a/compiler/rustc_mir_transform/src/const_prop.rs b/compiler/rustc_mir_transform/src/const_prop.rs index 3420096fe02..b0514e03356 100644 --- a/compiler/rustc_mir_transform/src/const_prop.rs +++ b/compiler/rustc_mir_transform/src/const_prop.rs @@ -3,7 +3,7 @@ use std::cell::Cell; -use either::Left; +use either::Right; use rustc_ast::Mutability; use rustc_data_structures::fx::FxHashSet; @@ -431,7 +431,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // Try to read the local as an immediate so that if it is representable as a scalar, we can // handle it as such, but otherwise, just return the value as is. Some(match self.ecx.read_immediate_raw(&op) { - Ok(Left(imm)) => imm.into(), + Ok(Right(imm)) => imm.into(), _ => op, }) } @@ -745,7 +745,7 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { // FIXME> figure out what to do when read_immediate_raw fails let imm = self.use_ecx(|this| this.ecx.read_immediate_raw(value)); - if let Some(Left(imm)) = imm { + if let Some(Right(imm)) = imm { match *imm { interpret::Immediate::Scalar(scalar) => { *rval = Rvalue::Use(self.operand_from_scalar(