From 9f8b099846f3bb5cd5f17698b6e653e0271f057e Mon Sep 17 00:00:00 2001 From: Saleem Jaffer Date: Mon, 29 Jul 2019 20:17:52 +0530 Subject: [PATCH] code review fixes --- src/librustc/mir/interpret/error.rs | 14 +++++++------- src/librustc/mir/interpret/mod.rs | 4 ++-- src/librustc/mir/interpret/pointer.rs | 2 +- src/librustc_mir/const_eval.rs | 8 ++++---- src/librustc_mir/interpret/cast.rs | 4 ++-- src/librustc_mir/interpret/eval_context.rs | 19 ++++++++++++------- src/librustc_mir/interpret/intern.rs | 4 ++-- src/librustc_mir/interpret/intrinsics.rs | 4 ++-- src/librustc_mir/interpret/machine.rs | 6 +++--- src/librustc_mir/interpret/memory.rs | 11 ++++++----- src/librustc_mir/interpret/operand.rs | 8 +++++--- src/librustc_mir/interpret/snapshot.rs | 4 ++-- src/librustc_mir/interpret/terminator.rs | 4 ++-- src/librustc_mir/interpret/validity.rs | 9 +++++---- src/librustc_mir/transform/const_prop.rs | 4 ++-- 15 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/librustc/mir/interpret/error.rs b/src/librustc/mir/interpret/error.rs index b347000ae046..36635e62c15e 100644 --- a/src/librustc/mir/interpret/error.rs +++ b/src/librustc/mir/interpret/error.rs @@ -314,12 +314,12 @@ impl fmt::Debug for PanicInfo { #[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] pub enum InvalidProgramInfo<'tcx> { - /// Resolution can fail if we are in a too generic context + /// Resolution can fail if we are in a too generic context. TooGeneric, /// Cannot compute this constant because it depends on another one - /// which already produced an error + /// which already produced an error. ReferencedConstant, - /// Abort in case type errors are reached + /// Abort in case type errors are reached. TypeckError, /// An error occurred during layout computation. Layout(layout::LayoutError<'tcx>), @@ -362,7 +362,7 @@ impl fmt::Debug for UndefinedBehaviourInfo { } #[derive(Clone, RustcEncodable, RustcDecodable, HashStable)] -pub enum UnsupportedInfo<'tcx> { +pub enum UnsupportedOpInfo<'tcx> { /// Handle cases which for which we do not have a fixed variant. Unimplemented(String), @@ -426,9 +426,9 @@ pub enum UnsupportedInfo<'tcx> { PathNotFound(Vec), } -impl fmt::Debug for UnsupportedInfo<'tcx> { +impl fmt::Debug for UnsupportedOpInfo<'tcx> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - use UnsupportedInfo::*; + use UnsupportedOpInfo::*; match self { PointerOutOfBounds { ptr, msg, allocation_size } => { write!(f, "{} failed: pointer must be in-bounds at offset {}, \ @@ -574,7 +574,7 @@ pub enum InterpError<'tcx> { UndefinedBehaviour(UndefinedBehaviourInfo), /// The program did something the interpreter does not support (some of these *might* be UB /// but the interpreter is not sure). - Unsupported(UnsupportedInfo<'tcx>), + Unsupported(UnsupportedOpInfo<'tcx>), /// The program was invalid (ill-typed, not sufficiently monomorphized, ...). InvalidProgram(InvalidProgramInfo<'tcx>), /// The program exhausted the interpreter's resources (stack/heap too big, diff --git a/src/librustc/mir/interpret/mod.rs b/src/librustc/mir/interpret/mod.rs index 4b09da87d314..e4bf8efad72e 100644 --- a/src/librustc/mir/interpret/mod.rs +++ b/src/librustc/mir/interpret/mod.rs @@ -4,7 +4,7 @@ macro_rules! err { ($($tt:tt)*) => { Err($crate::mir::interpret::InterpError::Unsupported( - $crate::mir::interpret::UnsupportedInfo::$($tt)* + $crate::mir::interpret::UnsupportedOpInfo::$($tt)* ).into()) }; } @@ -52,7 +52,7 @@ mod pointer; pub use self::error::{ InterpErrorInfo, InterpResult, InterpError, AssertMessage, ConstEvalErr, struct_error, - FrameInfo, ConstEvalRawResult, ConstEvalResult, ErrorHandled, PanicInfo, UnsupportedInfo, + FrameInfo, ConstEvalRawResult, ConstEvalResult, ErrorHandled, PanicInfo, UnsupportedOpInfo, InvalidProgramInfo, ResourceExhaustionInfo, UndefinedBehaviourInfo, }; diff --git a/src/librustc/mir/interpret/pointer.rs b/src/librustc/mir/interpret/pointer.rs index faca04412402..fbc726deb3dc 100644 --- a/src/librustc/mir/interpret/pointer.rs +++ b/src/librustc/mir/interpret/pointer.rs @@ -196,7 +196,7 @@ impl<'tcx, Tag> Pointer { msg: CheckInAllocMsg, ) -> InterpResult<'tcx, ()> { if self.offset > allocation_size { - err!(PointerOutOfBounds { ptr: self.erase_tag(),msg,allocation_size }) + err!(PointerOutOfBounds { ptr: self.erase_tag(), msg, allocation_size }) } else { Ok(()) } diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index bbee3b392345..e059d766a6f5 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -24,8 +24,7 @@ use crate::interpret::{self, RawConst, ConstValue, InterpResult, InterpErrorInfo, InterpError, GlobalId, InterpCx, StackPopCleanup, Allocation, AllocId, MemoryKind, - snapshot, RefTracking, intern_const_alloc_recursive, UnsupportedInfo::*, - InvalidProgramInfo::*, + snapshot, RefTracking, intern_const_alloc_recursive, UnsupportedOpInfo, }; /// Number of steps until the detector even starts doing anything. @@ -184,7 +183,7 @@ fn eval_body_using_ecx<'mir, 'tcx>( impl<'tcx> Into> for ConstEvalError { fn into(self) -> InterpErrorInfo<'tcx> { - InterpError::Unsupported(MachineError(self.to_string())).into() + InterpError::Unsupported(UnsupportedOpInfo::MachineError(self.to_string())).into() } } @@ -361,7 +360,7 @@ impl<'mir, 'tcx> interpret::Machine<'mir, 'tcx> for CompileTimeInterpreter<'mir, Ok(Some(match ecx.load_mir(instance.def) { Ok(body) => body, Err(err) => { - if let InterpError::Unsupported(NoMirFor(ref path)) = err.kind { + if let InterpError::Unsupported(UnsupportedOpInfo::NoMirFor(ref path)) = err.kind { return Err( ConstEvalError::NeedsRfc(format!("calling extern function `{}`", path)) .into(), @@ -698,6 +697,7 @@ pub fn const_eval_raw_provider<'tcx>( // promoting runtime code is only allowed to error if it references broken constants // any other kind of error will be reported to the user as a deny-by-default lint _ => if let Some(p) = cid.promoted { + use crate::interpret::InvalidProgramInfo::*; let span = tcx.promoted_mir(def_id)[p].span; if let InterpError::InvalidProgram(ReferencedConstant) = err.error { err.report_as_error( diff --git a/src/librustc_mir/interpret/cast.rs b/src/librustc_mir/interpret/cast.rs index 7157e714f052..c7a56c29b4b5 100644 --- a/src/librustc_mir/interpret/cast.rs +++ b/src/librustc_mir/interpret/cast.rs @@ -9,8 +9,7 @@ use rustc_apfloat::{Float, FloatConvert}; use rustc::mir::interpret::{ Scalar, InterpResult, Pointer, PointerArithmetic, InterpError, }; -use rustc::mir::{CastKind, interpret::{InvalidProgramInfo::*}}; - +use rustc::mir::CastKind; use super::{InterpCx, Machine, PlaceTy, OpTy, Immediate, FnVal}; @@ -75,6 +74,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { } Pointer(PointerCast::ReifyFnPointer) => { + use rustc::mir::interpret::InvalidProgramInfo::TooGeneric; // The src operand does not matter, just its type match src.layout.ty.sty { ty::FnDef(def_id, substs) => { diff --git a/src/librustc_mir/interpret/eval_context.rs b/src/librustc_mir/interpret/eval_context.rs index 1ee6871f5efc..76de3b32fc7f 100644 --- a/src/librustc_mir/interpret/eval_context.rs +++ b/src/librustc_mir/interpret/eval_context.rs @@ -17,7 +17,7 @@ use rustc::mir::interpret::{ ErrorHandled, GlobalId, Scalar, Pointer, FrameInfo, AllocId, InterpResult, InterpError, - truncate, sign_extend, InvalidProgramInfo::*, + truncate, sign_extend, InvalidProgramInfo, }; use rustc_data_structures::fx::FxHashMap; @@ -190,8 +190,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> LayoutOf for InterpCx<'mir, 'tcx, M> { #[inline] fn layout_of(&self, ty: Ty<'tcx>) -> Self::TyLayout { - self.tcx.layout_of(self.param_env.and(ty)) - .map_err(|layout| InterpError::InvalidProgram(Layout(layout)).into()) + self.tcx + .layout_of(self.param_env.and(ty)) + .map_err(|layout| { + InterpError::InvalidProgram(InvalidProgramInfo::Layout(layout)).into() + }) } } @@ -302,7 +305,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { &substs, )), None => if substs.needs_subst() { - err_inval!(TooGeneric).into() + err_inval!(TooGeneric) } else { Ok(substs) }, @@ -323,7 +326,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.param_env, def_id, substs, - ).ok_or_else(|| InterpError::InvalidProgram(TooGeneric).into()) + ).ok_or_else(|| InterpError::InvalidProgram(InvalidProgramInfo::TooGeneric).into()) } pub fn load_mir( @@ -694,8 +697,10 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // `Memory::get_static_alloc` which has to use `const_eval_raw` to avoid cycles. let val = self.tcx.const_eval_raw(param_env.and(gid)).map_err(|err| { match err { - ErrorHandled::Reported => InterpError::InvalidProgram(ReferencedConstant), - ErrorHandled::TooGeneric => InterpError::InvalidProgram(TooGeneric), + ErrorHandled::Reported => + InterpError::InvalidProgram(InvalidProgramInfo::ReferencedConstant), + ErrorHandled::TooGeneric => + InterpError::InvalidProgram(InvalidProgramInfo::TooGeneric), } })?; self.raw_const_to_mplace(val) diff --git a/src/librustc_mir/interpret/intern.rs b/src/librustc_mir/interpret/intern.rs index 261cfbce1714..af7da6fa5322 100644 --- a/src/librustc_mir/interpret/intern.rs +++ b/src/librustc_mir/interpret/intern.rs @@ -5,7 +5,7 @@ use rustc::ty::{Ty, TyCtxt, ParamEnv, self}; use rustc::mir::interpret::{ - InterpResult, ErrorHandled, UnsupportedInfo::*, + InterpResult, ErrorHandled, UnsupportedOpInfo, }; use rustc::hir; use rustc::hir::def_id::DefId; @@ -293,7 +293,7 @@ pub fn intern_const_alloc_recursive( if let Err(error) = interned { // This can happen when e.g. the tag of an enum is not a valid discriminant. We do have // to read enum discriminants in order to find references in enum variant fields. - if let InterpError::Unsupported(ValidationFailure(_)) = error.kind { + if let InterpError::Unsupported(UnsupportedOpInfo::ValidationFailure(_)) = error.kind { let err = crate::const_eval::error_to_const_error(&ecx, error); match err.struct_error(ecx.tcx, "it is undefined behavior to use this value") { Ok(mut diag) => { diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index 9605395b84b1..0b4df6cc7bc5 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -7,7 +7,7 @@ use rustc::ty; use rustc::ty::layout::{LayoutOf, Primitive, Size}; use rustc::mir::BinOp; use rustc::mir::interpret::{ - InterpResult, InterpError, Scalar, PanicInfo, UnsupportedInfo::*, + InterpResult, InterpError, Scalar, PanicInfo, UnsupportedOpInfo, }; use super::{ @@ -100,7 +100,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let bits = self.read_scalar(args[0])?.to_bits(layout_of.size)?; let kind = match layout_of.abi { ty::layout::Abi::Scalar(ref scalar) => scalar.value, - _ => Err(InterpError::Unsupported(TypeNotPrimitive(ty)))?, + _ => Err(InterpError::Unsupported(UnsupportedOpInfo::TypeNotPrimitive(ty)))?, }; let out_val = if intrinsic_name.ends_with("_nonzero") { if bits == 0 { diff --git a/src/librustc_mir/interpret/machine.rs b/src/librustc_mir/interpret/machine.rs index 4f04803addd8..064ca0f4253b 100644 --- a/src/librustc_mir/interpret/machine.rs +++ b/src/librustc_mir/interpret/machine.rs @@ -11,7 +11,7 @@ use rustc::ty::{self, TyCtxt}; use super::{ Allocation, AllocId, InterpResult, InterpError, Scalar, AllocationExtra, - InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, UnsupportedInfo::* + InterpCx, PlaceTy, OpTy, ImmTy, MemoryKind, Pointer, Memory, UnsupportedOpInfo, }; /// Whether this kind of memory is allowed to leak @@ -240,9 +240,9 @@ pub trait Machine<'mir, 'tcx>: Sized { int: u64, ) -> InterpResult<'tcx, Pointer> { Err((if int == 0 { - InterpError::Unsupported(InvalidNullPointerUsage) + InterpError::Unsupported(UnsupportedOpInfo::InvalidNullPointerUsage) } else { - InterpError::Unsupported(ReadBytesAsPointer) + InterpError::Unsupported(UnsupportedOpInfo::ReadBytesAsPointer) }).into()) } diff --git a/src/librustc_mir/interpret/memory.rs b/src/librustc_mir/interpret/memory.rs index 72feb73f751a..edaf7855e872 100644 --- a/src/librustc_mir/interpret/memory.rs +++ b/src/librustc_mir/interpret/memory.rs @@ -19,8 +19,7 @@ use syntax::ast::Mutability; use super::{ Pointer, AllocId, Allocation, GlobalId, AllocationExtra, InterpResult, Scalar, InterpError, GlobalAlloc, PointerArithmetic, - Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, UnsupportedInfo::*, - InvalidProgramInfo::* + Machine, AllocMap, MayLeak, ErrorHandled, CheckInAllocMsg, InvalidProgramInfo, }; #[derive(Debug, PartialEq, Eq, Copy, Clone, Hash)] @@ -438,9 +437,11 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { assert!(tcx.is_static(def_id)); match err { ErrorHandled::Reported => - InterpError::InvalidProgram(ReferencedConstant), + InterpError::InvalidProgram( + InvalidProgramInfo::ReferencedConstant + ), ErrorHandled::TooGeneric => - InterpError::InvalidProgram(TooGeneric), + InterpError::InvalidProgram(InvalidProgramInfo::TooGeneric), } })?; // Make sure we use the ID of the resolved memory, not the lazy one! @@ -590,7 +591,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> Memory<'mir, 'tcx, M> { } else { match self.tcx.alloc_map.lock().get(id) { Some(GlobalAlloc::Function(instance)) => Ok(FnVal::Instance(instance)), - _ => Err(InterpError::Unsupported(ExecuteMemory).into()), + _ => err!(ExecuteMemory), } } } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1896c3cda13d..4dd308f5bf4d 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,7 +12,7 @@ use rustc::mir::interpret::{ GlobalId, AllocId, ConstValue, Pointer, Scalar, InterpResult, InterpError, - sign_extend, truncate, UnsupportedInfo::*, + sign_extend, truncate, UnsupportedOpInfo, }; use super::{ InterpCx, Machine, @@ -331,8 +331,9 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { ) -> InterpResult<'tcx, &str> { let len = mplace.len(self)?; let bytes = self.memory.read_bytes(mplace.ptr, Size::from_bytes(len as u64))?; - let str = ::std::str::from_utf8(bytes) - .map_err(|err| InterpError::Unsupported(ValidationFailure(err.to_string())))?; + let str = ::std::str::from_utf8(bytes).map_err(|err| { + InterpError::Unsupported(UnsupportedOpInfo::ValidationFailure(err.to_string())) + })?; Ok(str) } @@ -603,6 +604,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { let raw_discr = discr_val.to_scalar_or_undef(); trace!("discr value: {:?}", raw_discr); // post-process + use rustc::mir::interpret::UnsupportedOpInfo::InvalidDiscriminant; Ok(match *discr_kind { layout::DiscriminantKind::Tag => { let bits_discr = match raw_discr.to_bits(discr_val.layout.size) { diff --git a/src/librustc_mir/interpret/snapshot.rs b/src/librustc_mir/interpret/snapshot.rs index 112e592329f5..ebf7dd3b6628 100644 --- a/src/librustc_mir/interpret/snapshot.rs +++ b/src/librustc_mir/interpret/snapshot.rs @@ -12,7 +12,7 @@ use rustc::mir; use rustc::mir::interpret::{ AllocId, Pointer, Scalar, Relocations, Allocation, UndefMask, - InterpResult, InterpError, ResourceExhaustionInfo::*, + InterpResult, InterpError, ResourceExhaustionInfo, }; use rustc::ty::{self, TyCtxt}; @@ -77,7 +77,7 @@ impl<'mir, 'tcx> InfiniteLoopDetector<'mir, 'tcx> { } // Second cycle - Err(InterpError::ResourceExhaustion(InfiniteLoop).into()) + Err(InterpError::ResourceExhaustion(ResourceExhaustionInfo::InfiniteLoop).into()) } } diff --git a/src/librustc_mir/interpret/terminator.rs b/src/librustc_mir/interpret/terminator.rs index 14b92f2b3d49..b1706e72dd47 100644 --- a/src/librustc_mir/interpret/terminator.rs +++ b/src/librustc_mir/interpret/terminator.rs @@ -9,7 +9,7 @@ use rustc_target::spec::abi::Abi; use super::{ InterpResult, PointerArithmetic, InterpError, Scalar, InterpCx, Machine, Immediate, OpTy, ImmTy, PlaceTy, MPlaceTy, StackPopCleanup, FnVal, - UnsupportedInfo::*, + UnsupportedOpInfo, }; impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { @@ -221,7 +221,7 @@ impl<'mir, 'tcx, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { return Ok(()); } let caller_arg = caller_arg.next() - .ok_or_else(|| InterpError::Unsupported(FunctionArgCountMismatch))?; + .ok_or_else(|| InterpError::Unsupported(UnsupportedOpInfo::FunctionArgCountMismatch))?; if rust_abi { debug_assert!(!caller_arg.layout.is_zst(), "ZSTs must have been already filtered out"); } diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index ea8b88e51f0a..9404711bd970 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use std::hash::Hash; use super::{ GlobalAlloc, InterpResult, InterpError, - OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, UnsupportedInfo::*, + OpTy, Machine, InterpCx, ValueVisitor, MPlaceTy, UnsupportedOpInfo, }; macro_rules! validation_failure { @@ -297,11 +297,11 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> match self.walk_value(op) { Ok(()) => Ok(()), Err(err) => match err.kind { - InterpError::Unsupported(InvalidDiscriminant(val)) => + InterpError::Unsupported(UnsupportedOpInfo::InvalidDiscriminant(val)) => validation_failure!( val, self.path, "a valid enum discriminant" ), - InterpError::Unsupported(ReadPointerAsBytes) => + InterpError::Unsupported(UnsupportedOpInfo::ReadPointerAsBytes) => validation_failure!( "a pointer", self.path, "plain (non-pointer) bytes" ), @@ -405,6 +405,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> "{:?} did not pass access check for size {:?}, align {:?}", ptr, size, align ); + use super::UnsupportedOpInfo::*; match err.kind { InterpError::Unsupported(InvalidNullPointerUsage) => return validation_failure!("NULL reference", self.path), @@ -608,7 +609,7 @@ impl<'rt, 'mir, 'tcx, M: Machine<'mir, 'tcx>> ValueVisitor<'mir, 'tcx, M> Err(err) => { // For some errors we might be able to provide extra information match err.kind { - InterpError::Unsupported(ReadUndefBytes(offset)) => { + InterpError::Unsupported(UnsupportedOpInfo::ReadUndefBytes(offset)) => { // Some byte was undefined, determine which // element that byte belongs to so we can // provide an index. diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index ec8a33f7a43f..191c6f2dc8ae 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -257,9 +257,9 @@ impl<'mir, 'tcx> ConstPropagator<'mir, 'tcx> { Err(error) => { let diagnostic = error_to_const_error(&self.ecx, error); use rustc::mir::interpret::InterpError::*; - use rustc::mir::interpret::UnsupportedInfo::*; + use rustc::mir::interpret::UnsupportedOpInfo::*; match diagnostic.error { - Exit(_) => {}, + Exit(_) => bug!("the CTFE program cannot exit"), | Unsupported(OutOfTls) | Unsupported(TlsOutOfBounds)