From e41e694d9e5a6bcb2109c936df9757879a65eb59 Mon Sep 17 00:00:00 2001 From: Jethro Beekman Date: Tue, 5 Feb 2019 16:56:19 +0530 Subject: [PATCH 01/23] Clarify guarantees for `Box` allocation --- src/liballoc/alloc.rs | 3 +++ src/liballoc/boxed.rs | 10 ++++++++++ 2 files changed, 13 insertions(+) diff --git a/src/liballoc/alloc.rs b/src/liballoc/alloc.rs index ec652df3b37a4..f3877e51a6633 100644 --- a/src/liballoc/alloc.rs +++ b/src/liballoc/alloc.rs @@ -34,6 +34,9 @@ extern "Rust" { /// This type implements the [`Alloc`] trait by forwarding calls /// to the allocator registered with the `#[global_allocator]` attribute /// if there is one, or the `std` crate’s default. +/// +/// Note: while this type is unstable, the functionality it provides can be +/// accessed through the [free functions in `alloc`](index.html#functions). #[unstable(feature = "allocator_api", issue = "32838")] #[derive(Copy, Clone, Default, Debug)] pub struct Global; diff --git a/src/liballoc/boxed.rs b/src/liballoc/boxed.rs index 8e01e12e0b8de..462f65ed79455 100644 --- a/src/liballoc/boxed.rs +++ b/src/liballoc/boxed.rs @@ -4,6 +4,16 @@ //! heap allocation in Rust. Boxes provide ownership for this allocation, and //! drop their contents when they go out of scope. //! +//! For non-zero-sized values, a [`Box`] will use the [`Global`] allocator for +//! its allocation. It is valid to convert both ways between a [`Box`] and a +//! raw pointer allocated with the [`Global`] allocator, given that the +//! [`Layout`] used with the allocator is correct for the type. More precisely, +//! a `value: *mut T` that has been allocated with the [`Global`] allocator +//! with `Layout::for_value(&*value)` may be converted into a box using +//! `Box::::from_raw(value)`. Conversely, the memory backing a `value: *mut +//! T` obtained from `Box::::into_raw` may be deallocated using the +//! [`Global`] allocator with `Layout::for_value(&*value)`. +//! //! # Examples //! //! Move a value from the stack to the heap by creating a [`Box`]: From 70c5af85e09be583128df5eda6b4de25a23387c1 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 13 Feb 2019 13:46:45 -0800 Subject: [PATCH 02/23] Avoid allocation in std::sys::unix::weak If we add a terminating NUL to the name in the `weak!` macro, then `fetch()` can use `CStr::from_bytes_with_nul()` instead of `CString`. --- src/libstd/sys/unix/weak.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys/unix/weak.rs b/src/libstd/sys/unix/weak.rs index d0242ca74229a..9b80ad8d9b27f 100644 --- a/src/libstd/sys/unix/weak.rs +++ b/src/libstd/sys/unix/weak.rs @@ -18,7 +18,7 @@ use libc; -use ffi::CString; +use ffi::CStr; use marker; use mem; use sync::atomic::{AtomicUsize, Ordering}; @@ -26,7 +26,7 @@ use sync::atomic::{AtomicUsize, Ordering}; macro_rules! weak { (fn $name:ident($($t:ty),*) -> $ret:ty) => ( static $name: ::sys::weak::Weak $ret> = - ::sys::weak::Weak::new(stringify!($name)); + ::sys::weak::Weak::new(concat!(stringify!($name), '\0')); ) } @@ -61,7 +61,7 @@ impl Weak { } unsafe fn fetch(name: &str) -> usize { - let name = match CString::new(name) { + let name = match CStr::from_bytes_with_nul(name.as_bytes()) { Ok(cstr) => cstr, Err(..) => return 0, }; From 33d80bfaa0f2a4ca996a942e6d65a932e72fec1b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Wed, 13 Feb 2019 14:07:08 -0800 Subject: [PATCH 03/23] Return without a reference in unix Weak::get() --- src/libstd/sys/unix/weak.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libstd/sys/unix/weak.rs b/src/libstd/sys/unix/weak.rs index 9b80ad8d9b27f..b60e241f10cee 100644 --- a/src/libstd/sys/unix/weak.rs +++ b/src/libstd/sys/unix/weak.rs @@ -45,16 +45,15 @@ impl Weak { } } - pub fn get(&self) -> Option<&F> { + pub fn get(&self) -> Option { assert_eq!(mem::size_of::(), mem::size_of::()); unsafe { if self.addr.load(Ordering::SeqCst) == 1 { self.addr.store(fetch(self.name), Ordering::SeqCst); } - if self.addr.load(Ordering::SeqCst) == 0 { - None - } else { - mem::transmute::<&AtomicUsize, Option<&F>>(&self.addr) + match self.addr.load(Ordering::SeqCst) { + 0 => None, + addr => Some(mem::transmute_copy::(&addr)), } } } From 235a6b7d065a2fc55ceee323e85b9309b16e84bf Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:32:22 +0100 Subject: [PATCH 04/23] Expose const -> op functions that don't allow violiting const eval invariants --- src/librustc_mir/const_eval.rs | 4 +- src/librustc_mir/interpret/operand.rs | 77 +++++++++++------------- src/librustc_mir/transform/const_prop.rs | 2 +- 3 files changed, 37 insertions(+), 46 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 7be7f4b439289..a4b2d6d36878d 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -476,7 +476,7 @@ pub fn const_field<'a, 'tcx>( let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); let result = (|| { // get the operand again - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(value), value.ty)?; + let op = ecx.const_to_op(value, None)?; // downcast let down = match variant { None => op, @@ -502,7 +502,7 @@ pub fn const_variant_index<'a, 'tcx>( ) -> EvalResult<'tcx, VariantIdx> { trace!("const_variant_index: {:?}", val); let ecx = mk_eval_cx(tcx, DUMMY_SP, param_env); - let op = ecx.lazy_const_to_op(ty::LazyConst::Evaluated(val), val.ty)?; + let op = ecx.const_to_op(val, None)?; Ok(ecx.read_discriminant(op)?.1) } diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 7da907028eebf..4f34ffc128e69 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -12,7 +12,7 @@ use rustc::mir::interpret::{ EvalResult, EvalErrorKind, }; use super::{ - EvalContext, Machine, AllocMap, Allocation, AllocationExtra, + EvalContext, Machine, MemPlace, MPlaceTy, PlaceTy, Place, MemoryKind, }; pub use rustc::mir::interpret::ScalarMaybeUndef; @@ -545,14 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => { - let layout = from_known_layout(layout, || { - let ty = self.monomorphize(mir_op.ty(self.mir(), *self.tcx))?; - self.layout_of(ty) - })?; - let op = self.const_value_to_op(*constant.literal)?; - OpTy { op, layout } - } + Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -568,38 +561,55 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. - fn const_value_to_op( + // Used when Miri runs into a constant, and by CTFE. + pub fn lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, - ) -> EvalResult<'tcx, Operand> { - trace!("const_value_to_op: {:?}", val); - let val = match val { + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + trace!("const_to_op: {:?}", val); + match val { ty::LazyConst::Unevaluated(def_id, substs) => { let instance = self.resolve(def_id, substs)?; - return Ok(*OpTy::from(self.const_eval_raw(GlobalId { + return Ok(OpTy::from(self.const_eval_raw(GlobalId { instance, promoted: None, })?)); }, - ty::LazyConst::Evaluated(c) => c, - }; - match val.val { + ty::LazyConst::Evaluated(c) => self.const_to_op(c, layout), + } + } + + // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + pub fn const_to_op( + &self, + val: ty::Const<'tcx>, + layout: Option>, + ) -> EvalResult<'tcx, OpTy<'tcx, M::PointerTag>> { + let layout = from_known_layout(layout, || { + let ty = self.monomorphize(val.ty)?; + self.layout_of(ty) + })?; + let op = match val.val { ConstValue::ByRef(id, alloc, offset) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. - Ok(Operand::Indirect( + Operand::Indirect( MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) - ).with_default_tag()) + ).with_default_tag() }, ConstValue::Slice(a, b) => - Ok(Operand::Immediate(Immediate::ScalarPair( + Operand::Immediate(Immediate::ScalarPair( a.into(), Scalar::from_uint(b, self.tcx.data_layout.pointer_size).into(), - )).with_default_tag()), + )).with_default_tag(), ConstValue::Scalar(x) => - Ok(Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag()), - } + Operand::Immediate(Immediate::Scalar(x.into())).with_default_tag(), + }; + Ok(OpTy { + op, + layout, + }) } /// Read discriminant, return the runtime value as well as the variant index. @@ -699,23 +709,4 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } }) } - -} - -impl<'a, 'mir, 'tcx, M> EvalContext<'a, 'mir, 'tcx, M> -where - M: Machine<'a, 'mir, 'tcx, PointerTag=()>, - // FIXME: Working around https://github.com/rust-lang/rust/issues/24159 - M::MemoryMap: AllocMap, Allocation<(), M::AllocExtra>)>, - M::AllocExtra: AllocationExtra<(), M::MemoryExtra>, -{ - // FIXME: CTFE should use allocations, then we can remove this. - pub(crate) fn lazy_const_to_op( - &self, - cnst: ty::LazyConst<'tcx>, - ty: ty::Ty<'tcx>, - ) -> EvalResult<'tcx, OpTy<'tcx>> { - let op = self.const_value_to_op(cnst)?; - Ok(OpTy { op, layout: self.layout_of(ty)? }) - } } diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index 7da00c4ea0c36..d69a5130b24d0 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, c.ty) { + match self.ecx.lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From bd18cc5708328600a94a02444caf27a5fb6ca20c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 12:36:23 +0100 Subject: [PATCH 05/23] Remove an intermediate value from discriminant reading --- src/librustc_mir/interpret/operand.rs | 2 +- src/librustc_mir/interpret/step.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 4f34ffc128e69..a638c008e760f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -508,7 +508,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> // Evaluate a place with the goal of reading from it. This lets us sometimes // avoid allocations. - fn eval_place_to_op( + pub(super) fn eval_place_to_op( &self, mir_place: &mir::Place<'tcx>, layout: Option>, diff --git a/src/librustc_mir/interpret/step.rs b/src/librustc_mir/interpret/step.rs index 97ef2b5fa3485..656c13c16d9ed 100644 --- a/src/librustc_mir/interpret/step.rs +++ b/src/librustc_mir/interpret/step.rs @@ -266,8 +266,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } Discriminant(ref place) => { - let place = self.eval_place(place)?; - let discr_val = self.read_discriminant(self.place_to_op(place)?)?.0; + let op = self.eval_place_to_op(place, None)?; + let discr_val = self.read_discriminant(op)?.0; let size = dest.layout.size; self.write_scalar(Scalar::from_uint(discr_val, size), dest)?; } From b2bf37aec075099a8b58c1a52ebca944f318d530 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 13:27:54 +0100 Subject: [PATCH 06/23] Burn some invariants we keep up into code --- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/interpret/operand.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index a4b2d6d36878d..9eac125d7a434 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -76,7 +76,7 @@ pub fn op_to_const<'tcx>( _ => false, }; let normalized_op = if normalize { - ecx.try_read_immediate(op)? + Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { match *op { Operand::Indirect(mplace) => Err(mplace), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index a638c008e760f..0394ad2b0a65f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -269,7 +269,7 @@ pub(super) fn from_known_layout<'tcx>( impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> { /// Try reading an immediate in memory; this is interesting particularly for ScalarPair. /// Returns `None` if the layout does not permit loading this as a value. - pub(super) fn try_read_immediate_from_mplace( + fn try_read_immediate_from_mplace( &self, mplace: MPlaceTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Option>> { @@ -323,7 +323,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> /// 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 /// in a `Immediate`, not on which data is stored there currently. - pub(crate) fn try_read_immediate( + pub(super) fn try_read_immediate( &self, src: OpTy<'tcx, M::PointerTag>, ) -> EvalResult<'tcx, Result, MemPlace>> { From f7c493121d4989895dd9c213ed4e877429229b86 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:29:27 +0100 Subject: [PATCH 07/23] Reuse the `Pointer` type instead of passing reassembling it at many use sites --- src/librustc/ich/impls_ty.rs | 2 +- src/librustc/mir/interpret/value.rs | 5 ++--- src/librustc/ty/structural_impls.rs | 4 ++-- src/librustc_codegen_llvm/consts.rs | 2 +- src/librustc_codegen_ssa/mir/operand.rs | 4 ++-- src/librustc_codegen_ssa/mir/place.rs | 4 ++-- src/librustc_mir/const_eval.rs | 2 +- src/librustc_mir/hair/pattern/_match.rs | 12 +++++------- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/monomorphize/collector.rs | 2 +- src/librustc_typeck/check/mod.rs | 2 +- 11 files changed, 20 insertions(+), 23 deletions(-) diff --git a/src/librustc/ich/impls_ty.rs b/src/librustc/ich/impls_ty.rs index 6f04a68a6ed61..04e03d0d95aa2 100644 --- a/src/librustc/ich/impls_ty.rs +++ b/src/librustc/ich/impls_ty.rs @@ -307,7 +307,7 @@ impl_stable_hash_for!( impl<'tcx> for enum mir::interpret::ConstValue<'tcx> [ mir::interpret::ConstValue ] { Scalar(val), Slice(a, b), - ByRef(id, alloc, offset), + ByRef(ptr, alloc), } ); impl_stable_hash_for!(struct crate::mir::interpret::RawConst<'tcx> { diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 5ec7de4308a13..1e5ba2c176bd2 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -31,9 +31,8 @@ pub enum ConstValue<'tcx> { /// it. Slice(Scalar, u64), - /// An allocation together with an offset into the allocation. - /// Invariant: the `AllocId` matches the allocation. - ByRef(AllocId, &'tcx Allocation, Size), + /// An allocation together with a pointer into the allocation. + ByRef(Pointer, &'tcx Allocation), } #[cfg(target_arch = "x86_64")] diff --git a/src/librustc/ty/structural_impls.rs b/src/librustc/ty/structural_impls.rs index d09cfa84a1690..0d1dc4ee2032f 100644 --- a/src/librustc/ty/structural_impls.rs +++ b/src/librustc/ty/structural_impls.rs @@ -499,8 +499,8 @@ impl<'a, 'tcx> Lift<'tcx> for ConstValue<'a> { match *self { ConstValue::Scalar(x) => Some(ConstValue::Scalar(x)), ConstValue::Slice(x, y) => Some(ConstValue::Slice(x, y)), - ConstValue::ByRef(x, alloc, z) => Some(ConstValue::ByRef( - x, alloc.lift_to_tcx(tcx)?, z, + ConstValue::ByRef(ptr, alloc) => Some(ConstValue::ByRef( + ptr, alloc.lift_to_tcx(tcx)?, )), } } diff --git a/src/librustc_codegen_llvm/consts.rs b/src/librustc_codegen_llvm/consts.rs index ca9e2c87be237..26d3bd9124707 100644 --- a/src/librustc_codegen_llvm/consts.rs +++ b/src/librustc_codegen_llvm/consts.rs @@ -71,7 +71,7 @@ pub fn codegen_static_initializer( let static_ = cx.tcx.const_eval(param_env.and(cid))?; let alloc = match static_.val { - ConstValue::ByRef(_, alloc, n) if n.bytes() == 0 => alloc, + ConstValue::ByRef(ptr, alloc) if ptr.offset.bytes() == 0 => alloc, _ => bug!("static const eval returned {:#?}", static_), }; Ok((const_alloc_to_llvm(cx, alloc), alloc)) diff --git a/src/librustc_codegen_ssa/mir/operand.rs b/src/librustc_codegen_ssa/mir/operand.rs index 2c6d968bb032a..3cac1befaf4ef 100644 --- a/src/librustc_codegen_ssa/mir/operand.rs +++ b/src/librustc_codegen_ssa/mir/operand.rs @@ -101,8 +101,8 @@ impl<'a, 'tcx: 'a, V: CodegenObject> OperandRef<'tcx, V> { let b_llval = bx.cx().const_usize(b); OperandValue::Pair(a_llval, b_llval) }, - ConstValue::ByRef(_, alloc, offset) => { - return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, offset))); + ConstValue::ByRef(ptr, alloc) => { + return Ok(bx.load_operand(bx.cx().from_const_alloc(layout, alloc, ptr.offset))); }, }; diff --git a/src/librustc_codegen_ssa/mir/place.rs b/src/librustc_codegen_ssa/mir/place.rs index 9d6826d8756b7..1edcbfead2c94 100644 --- a/src/librustc_codegen_ssa/mir/place.rs +++ b/src/librustc_codegen_ssa/mir/place.rs @@ -417,8 +417,8 @@ impl<'a, 'tcx: 'a, Bx: BuilderMethods<'a, 'tcx>> FunctionCx<'a, 'tcx, Bx> { let layout = cx.layout_of(self.monomorphize(&ty)); match bx.tcx().const_eval(param_env.and(cid)) { Ok(val) => match val.val { - mir::interpret::ConstValue::ByRef(_, alloc, offset) => { - bx.cx().from_const_alloc(layout, alloc, offset) + mir::interpret::ConstValue::ByRef(ptr, alloc) => { + bx.cx().from_const_alloc(layout, alloc, ptr.offset) } _ => bug!("promoteds should have an allocation: {:?}", val), }, diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 9eac125d7a434..94b9f0eadd0e7 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -96,7 +96,7 @@ pub fn op_to_const<'tcx>( // FIXME shouldn't it be the case that `mark_static_initialized` has already // interned this? I thought that is the entire point of that `FinishStatic` stuff? let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr.alloc_id, alloc, ptr.offset) + ConstValue::ByRef(ptr, alloc) }, Ok(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), diff --git a/src/librustc_mir/hair/pattern/_match.rs b/src/librustc_mir/hair/pattern/_match.rs index 1c7e1aa4d71e0..bf2f0d5d81f47 100644 --- a/src/librustc_mir/hair/pattern/_match.rs +++ b/src/librustc_mir/hair/pattern/_match.rs @@ -172,7 +172,7 @@ use rustc::ty::{self, Ty, TyCtxt, TypeFoldable, Const}; use rustc::ty::layout::{Integer, IntegerExt, VariantIdx, Size}; use rustc::mir::Field; -use rustc::mir::interpret::{ConstValue, Pointer, Scalar}; +use rustc::mir::interpret::{ConstValue, Scalar}; use rustc::util::common::ErrorReported; use syntax::attr::{SignedInt, UnsignedInt}; @@ -214,9 +214,8 @@ impl<'a, 'tcx> LiteralExpander<'a, 'tcx> { match (val, &crty.sty, &rty.sty) { // the easy case, deref a reference (ConstValue::Scalar(Scalar::Ptr(p)), x, y) if x == y => ConstValue::ByRef( - p.alloc_id, + p, self.tcx.alloc_map.lock().unwrap_memory(p.alloc_id), - p.offset, ), // unsize array to slice if pattern is array but match value or other patterns are slice (ConstValue::Scalar(Scalar::Ptr(p)), ty::Array(t, n), ty::Slice(u)) => { @@ -1428,7 +1427,7 @@ fn slice_pat_covered_by_const<'tcx>( suffix: &[Pattern<'tcx>] ) -> Result { let data: &[u8] = match (const_val.val, &const_val.ty.sty) { - (ConstValue::ByRef(id, alloc, offset), ty::Array(t, n)) => { + (ConstValue::ByRef(ptr, alloc), ty::Array(t, n)) => { if *t != tcx.types.u8 { // FIXME(oli-obk): can't mix const patterns with slice patterns and get // any sort of exhaustiveness/unreachable check yet @@ -1436,7 +1435,6 @@ fn slice_pat_covered_by_const<'tcx>( // are definitely unreachable. return Ok(false); } - let ptr = Pointer::new(id, offset); let n = n.assert_usize(tcx).unwrap(); alloc.get_bytes(&tcx, ptr, Size::from_bytes(n)).unwrap() }, @@ -1778,8 +1776,8 @@ fn specialize<'p, 'a: 'p, 'tcx: 'a>( let (opt_ptr, n, ty) = match value.ty.sty { ty::TyKind::Array(t, n) => { match value.val { - ConstValue::ByRef(id, alloc, offset) => ( - Some((Pointer::new(id, offset), alloc)), + ConstValue::ByRef(ptr, alloc) => ( + Some((ptr, alloc)), n.unwrap_usize(cx.tcx), t, ), diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 0394ad2b0a65f..1d6aff749c593 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -591,11 +591,11 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> self.layout_of(ty) })?; let op = match val.val { - ConstValue::ByRef(id, alloc, offset) => { + ConstValue::ByRef(ptr, alloc) => { // We rely on mutability being set correctly in that allocation to prevent writes // where none should happen -- and for `static mut`, we copy on demand anyway. Operand::Indirect( - MemPlace::from_ptr(Pointer::new(id, offset), alloc.align) + MemPlace::from_ptr(ptr, alloc.align) ).with_default_tag() }, ConstValue::Slice(a, b) => diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index a76aa7454cbe4..32c9d5c0c4467 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -1257,7 +1257,7 @@ fn collect_const<'a, 'tcx>( ConstValue::Slice(Scalar::Ptr(ptr), _) | ConstValue::Scalar(Scalar::Ptr(ptr)) => collect_miri(tcx, ptr.alloc_id, output), - ConstValue::ByRef(_id, alloc, _offset) => { + ConstValue::ByRef(_ptr, alloc) => { for &((), id) in alloc.relocations.values() { collect_miri(tcx, id, output); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 91e44a1588268..cbc8fde53a263 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -1464,7 +1464,7 @@ fn maybe_check_static_with_link_section(tcx: TyCtxt, id: DefId, span: Span) { }; let param_env = ty::ParamEnv::reveal_all(); if let Ok(static_) = tcx.const_eval(param_env.and(cid)) { - let alloc = if let ConstValue::ByRef(_, allocation, _) = static_.val { + let alloc = if let ConstValue::ByRef(_, allocation) = static_.val { allocation } else { bug!("Matching on non-ByRef static") From 7db96a37e7b4b38cdd0354d715cecd56dfdd03b0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:33:54 +0100 Subject: [PATCH 08/23] Reintroduce the invariant comment for clarity --- src/librustc/mir/interpret/value.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/librustc/mir/interpret/value.rs b/src/librustc/mir/interpret/value.rs index 1e5ba2c176bd2..956182fc8b275 100644 --- a/src/librustc/mir/interpret/value.rs +++ b/src/librustc/mir/interpret/value.rs @@ -32,6 +32,7 @@ pub enum ConstValue<'tcx> { Slice(Scalar, u64), /// An allocation together with a pointer into the allocation. + /// Invariant: the pointer's `AllocId` resolves to the allocation. ByRef(Pointer, &'tcx Allocation), } From bee3c670dbf974d097f41447a1214b27bbf9acca Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 16 Feb 2019 14:52:34 +0100 Subject: [PATCH 09/23] Update src/librustc_mir/interpret/operand.rs Co-Authored-By: oli-obk --- src/librustc_mir/interpret/operand.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 1d6aff749c593..3ec042efb26f8 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -580,7 +580,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and (indirectly through lazy_const_to_op) by CTFE. + // Used when Miri runs into a constant, and by CTFE. pub fn const_to_op( &self, val: ty::Const<'tcx>, From 525983a2a4ac3029dd9979d924ef444f48a4d7b3 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Fri, 15 Feb 2019 21:05:47 +0100 Subject: [PATCH 10/23] Make validity checking use `MPlaceTy` instead of `OpTy` --- src/librustc_mir/const_eval.rs | 12 +++++------- src/librustc_mir/interpret/place.rs | 2 +- src/librustc_mir/interpret/validity.rs | 23 +++++++++++------------ 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 94b9f0eadd0e7..3c2c0af0bae80 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -523,13 +523,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( let cid = key.value; let ecx = mk_eval_cx(tcx, tcx.def_span(key.value.instance.def_id()), key.param_env); let val = (|| { - let op = ecx.raw_const_to_mplace(constant)?.into(); - // FIXME: Once the visitor infrastructure landed, change validation to - // work directly on `MPlaceTy`. - let mut ref_tracking = RefTracking::new(op); - while let Some((op, path)) = ref_tracking.todo.pop() { + let mplace = ecx.raw_const_to_mplace(constant)?; + let mut ref_tracking = RefTracking::new(mplace); + while let Some((mplace, path)) = ref_tracking.todo.pop() { ecx.validate_operand( - op, + mplace.into(), path, Some(&mut ref_tracking), true, // const mode @@ -538,7 +536,7 @@ fn validate_and_turn_into_const<'a, 'tcx>( // Now that we validated, turn this into a proper constant. let def_id = cid.instance.def.def_id(); let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - op_to_const(&ecx, op, normalize) + op_to_const(&ecx, mplace.into(), normalize) })(); val.map_err(|error| { diff --git a/src/librustc_mir/interpret/place.rs b/src/librustc_mir/interpret/place.rs index b29e09900f6b1..abc18f1364f87 100644 --- a/src/librustc_mir/interpret/place.rs +++ b/src/librustc_mir/interpret/place.rs @@ -58,7 +58,7 @@ impl<'tcx, Tag> ::std::ops::Deref for PlaceTy<'tcx, Tag> { } /// A MemPlace with its layout. Constructing it is only possible in this module. -#[derive(Copy, Clone, Debug)] +#[derive(Copy, Clone, Debug, Hash, Eq, PartialEq)] pub struct MPlaceTy<'tcx, Tag=()> { mplace: MemPlace, pub layout: TyLayout<'tcx>, diff --git a/src/librustc_mir/interpret/validity.rs b/src/librustc_mir/interpret/validity.rs index 8b97d9ded7449..0163d53f027cf 100644 --- a/src/librustc_mir/interpret/validity.rs +++ b/src/librustc_mir/interpret/validity.rs @@ -11,7 +11,7 @@ use rustc::mir::interpret::{ }; use super::{ - OpTy, Machine, EvalContext, ValueVisitor, + OpTy, Machine, EvalContext, ValueVisitor, MPlaceTy, }; macro_rules! validation_failure { @@ -74,13 +74,13 @@ pub enum PathElem { } /// State for tracking recursive validation of references -pub struct RefTracking<'tcx, Tag> { - pub seen: FxHashSet<(OpTy<'tcx, Tag>)>, - pub todo: Vec<(OpTy<'tcx, Tag>, Vec)>, +pub struct RefTracking { + pub seen: FxHashSet, + pub todo: Vec<(T, Vec)>, } -impl<'tcx, Tag: Copy+Eq+Hash> RefTracking<'tcx, Tag> { - pub fn new(op: OpTy<'tcx, Tag>) -> Self { +impl<'tcx, T: Copy + Eq + Hash> RefTracking { + pub fn new(op: T) -> Self { let mut ref_tracking = RefTracking { seen: FxHashSet::default(), todo: vec![(op, Vec::new())], @@ -151,7 +151,7 @@ struct ValidityVisitor<'rt, 'a: 'rt, 'mir: 'rt, 'tcx: 'a+'rt+'mir, M: Machine<'a /// starts must not be changed! `visit_fields` and `visit_array` rely on /// this stack discipline. path: Vec, - ref_tracking: Option<&'rt mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&'rt mut RefTracking>>, const_mode: bool, ecx: &'rt EvalContext<'a, 'mir, 'tcx, M>, } @@ -399,16 +399,15 @@ impl<'rt, 'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> // before. Proceed recursively even for integer pointers, no // reason to skip them! They are (recursively) valid for some ZST, // but not for others (e.g., `!` is a ZST). - let op = place.into(); - if ref_tracking.seen.insert(op) { - trace!("Recursing below ptr {:#?}", *op); + if ref_tracking.seen.insert(place) { + trace!("Recursing below ptr {:#?}", *place); // We need to clone the path anyway, make sure it gets created // with enough space for the additional `Deref`. let mut new_path = Vec::with_capacity(self.path.len()+1); new_path.clone_from(&self.path); new_path.push(PathElem::Deref); // Remember to come back to this later. - ref_tracking.todo.push((op, new_path)); + ref_tracking.todo.push((place, new_path)); } } } @@ -598,7 +597,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> &self, op: OpTy<'tcx, M::PointerTag>, path: Vec, - ref_tracking: Option<&mut RefTracking<'tcx, M::PointerTag>>, + ref_tracking: Option<&mut RefTracking>>, const_mode: bool, ) -> EvalResult<'tcx> { trace!("validate_operand: {:?}, {:?}", *op, op.layout.ty); From 27e438ad948f9e430281e77b0abe3885b64f3bd0 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:51:53 +0100 Subject: [PATCH 11/23] Make `may_normalize` explicit in the type system --- src/librustc_mir/const_eval.rs | 72 ++++++++++++++++++---------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/src/librustc_mir/const_eval.rs b/src/librustc_mir/const_eval.rs index 3c2c0af0bae80..2f8e3189d12e9 100644 --- a/src/librustc_mir/const_eval.rs +++ b/src/librustc_mir/const_eval.rs @@ -21,7 +21,7 @@ use syntax::ast::Mutability; use syntax::source_map::{Span, DUMMY_SP}; use crate::interpret::{self, - PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Operand, Immediate, Scalar, Pointer, + PlaceTy, MPlaceTy, MemPlace, OpTy, ImmTy, Immediate, Scalar, Pointer, RawConst, ConstValue, EvalResult, EvalError, EvalErrorKind, GlobalId, EvalContext, StackPopCleanup, Allocation, AllocId, MemoryKind, @@ -62,45 +62,46 @@ pub(crate) fn eval_promoted<'a, 'mir, 'tcx>( eval_body_using_ecx(&mut ecx, cid, Some(mir), param_env) } -// FIXME: These two conversion functions are bad hacks. We should just always use allocations. -pub fn op_to_const<'tcx>( +fn mplace_to_const<'tcx>( + ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, + mplace: MPlaceTy<'tcx>, +) -> EvalResult<'tcx, ty::Const<'tcx>> { + let MemPlace { ptr, align, meta } = *mplace; + // extract alloc-offset pair + assert!(meta.is_none()); + let ptr = ptr.to_ptr()?; + let alloc = ecx.memory.get(ptr.alloc_id)?; + assert!(alloc.align >= align); + assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= mplace.layout.size.bytes()); + let mut alloc = alloc.clone(); + alloc.align = align; + // FIXME shouldn't it be the case that `mark_static_initialized` has already + // interned this? I thought that is the entire point of that `FinishStatic` stuff? + let alloc = ecx.tcx.intern_const_alloc(alloc); + let val = ConstValue::ByRef(ptr, alloc); + Ok(ty::Const { val, ty: mplace.layout.ty }) +} + +fn op_to_const<'tcx>( ecx: &CompileTimeEvalContext<'_, '_, 'tcx>, op: OpTy<'tcx>, - may_normalize: bool, ) -> EvalResult<'tcx, ty::Const<'tcx>> { // We do not normalize just any data. Only scalar layout and slices. - let normalize = may_normalize - && match op.layout.abi { - layout::Abi::Scalar(..) => true, - layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), - _ => false, - }; + let normalize = match op.layout.abi { + layout::Abi::Scalar(..) => true, + layout::Abi::ScalarPair(..) => op.layout.ty.is_slice(), + _ => false, + }; let normalized_op = if normalize { - Ok(*ecx.read_immediate(op).expect("normalization works on validated constants")) + Err(*ecx.read_immediate(op).expect("normalization works on validated constants")) } else { - match *op { - Operand::Indirect(mplace) => Err(mplace), - Operand::Immediate(val) => Ok(val) - } + op.try_as_mplace() }; let val = match normalized_op { - Err(MemPlace { ptr, align, meta }) => { - // extract alloc-offset pair - assert!(meta.is_none()); - let ptr = ptr.to_ptr()?; - let alloc = ecx.memory.get(ptr.alloc_id)?; - assert!(alloc.align >= align); - assert!(alloc.bytes.len() as u64 - ptr.offset.bytes() >= op.layout.size.bytes()); - let mut alloc = alloc.clone(); - alloc.align = align; - // FIXME shouldn't it be the case that `mark_static_initialized` has already - // interned this? I thought that is the entire point of that `FinishStatic` stuff? - let alloc = ecx.tcx.intern_const_alloc(alloc); - ConstValue::ByRef(ptr, alloc) - }, - Ok(Immediate::Scalar(x)) => + Ok(mplace) => return mplace_to_const(ecx, mplace), + Err(Immediate::Scalar(x)) => ConstValue::Scalar(x.not_undef()?), - Ok(Immediate::ScalarPair(a, b)) => + Err(Immediate::ScalarPair(a, b)) => ConstValue::Slice(a.not_undef()?, b.to_usize(ecx)?), }; Ok(ty::Const { val, ty: op.layout.ty }) @@ -486,7 +487,7 @@ pub fn const_field<'a, 'tcx>( let field = ecx.operand_field(down, field.index() as u64)?; // and finally move back to the const world, always normalizing because // this is not called for statics. - op_to_const(&ecx, field, true) + op_to_const(&ecx, field) })(); result.map_err(|error| { let err = error_to_const_error(&ecx, error); @@ -535,8 +536,11 @@ fn validate_and_turn_into_const<'a, 'tcx>( } // Now that we validated, turn this into a proper constant. let def_id = cid.instance.def.def_id(); - let normalize = tcx.is_static(def_id).is_none() && cid.promoted.is_none(); - op_to_const(&ecx, mplace.into(), normalize) + if tcx.is_static(def_id).is_some() || cid.promoted.is_some() { + mplace_to_const(&ecx, mplace) + } else { + op_to_const(&ecx, mplace.into()) + } })(); val.map_err(|error| { From 4fdeb2da747231b03f85786f4ef6ce04d88da864 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 14:54:02 +0100 Subject: [PATCH 12/23] Add `eval` prefix to clarify what the function does --- src/librustc_mir/interpret/operand.rs | 4 ++-- src/librustc_mir/transform/const_prop.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 3ec042efb26f8..08f6667039a34 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -545,7 +545,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> Move(ref place) => self.eval_place_to_op(place, layout)?, - Constant(ref constant) => self.lazy_const_to_op(*constant.literal, layout)?, + Constant(ref constant) => self.eval_lazy_const_to_op(*constant.literal, layout)?, }; trace!("{:?}: {:?}", mir_op, *op); Ok(op) @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by CTFE. - pub fn lazy_const_to_op( + pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, diff --git a/src/librustc_mir/transform/const_prop.rs b/src/librustc_mir/transform/const_prop.rs index d69a5130b24d0..1acc7b6e0c57f 100644 --- a/src/librustc_mir/transform/const_prop.rs +++ b/src/librustc_mir/transform/const_prop.rs @@ -253,7 +253,7 @@ impl<'a, 'mir, 'tcx> ConstPropagator<'a, 'mir, 'tcx> { source_info: SourceInfo, ) -> Option> { self.ecx.tcx.span = source_info.span; - match self.ecx.lazy_const_to_op(*c.literal, None) { + match self.ecx.eval_lazy_const_to_op(*c.literal, None) { Ok(op) => { Some((op, c.span)) }, From 4b085337bbb3434604f90503538e0211eb3bb8f6 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:05:14 +0100 Subject: [PATCH 13/23] Update docs and visibilities of const to op methods --- src/librustc_mir/interpret/operand.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 08f6667039a34..311c5c6d10be3 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -561,7 +561,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> .collect() } - // Used when Miri runs into a constant, and by CTFE. + // Used when Miri runs into a constant, and by const propagation. pub fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, @@ -580,8 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when Miri runs into a constant, and by CTFE. - pub fn const_to_op( + // Used when the miri-engine runs into a constant. + crate fn const_to_op( &self, val: ty::Const<'tcx>, layout: Option>, From 1fe7eb00944c3c41059e16daa7b401bc8b04447c Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Sat, 16 Feb 2019 15:16:02 +0100 Subject: [PATCH 14/23] Limit the visibility further and expand on a comment --- src/librustc_mir/interpret/operand.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/librustc_mir/interpret/operand.rs b/src/librustc_mir/interpret/operand.rs index 311c5c6d10be3..de362a7a96a7f 100644 --- a/src/librustc_mir/interpret/operand.rs +++ b/src/librustc_mir/interpret/operand.rs @@ -562,7 +562,7 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } // Used when Miri runs into a constant, and by const propagation. - pub fn eval_lazy_const_to_op( + crate fn eval_lazy_const_to_op( &self, val: ty::LazyConst<'tcx>, layout: Option>, @@ -580,7 +580,8 @@ impl<'a, 'mir, 'tcx, M: Machine<'a, 'mir, 'tcx>> EvalContext<'a, 'mir, 'tcx, M> } } - // Used when the miri-engine runs into a constant. + // Used when the miri-engine runs into a constant and for extracting information from constants + // in patterns via the `const_eval` module crate fn const_to_op( &self, val: ty::Const<'tcx>, From 06511573f236a07b8ecd7c3e3d5fef1d53e59352 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Mon, 11 Feb 2019 16:51:32 +0100 Subject: [PATCH 15/23] Remove sys::*::Stderr Write implementation --- src/libstd/io/mod.rs | 3 +++ src/libstd/io/stdio.rs | 8 ++++++-- src/libstd/sys/cloudabi/stdio.rs | 13 ------------- src/libstd/sys/redox/stdio.rs | 15 +-------------- src/libstd/sys/sgx/stdio.rs | 13 ------------- src/libstd/sys/unix/stdio.rs | 15 +-------------- src/libstd/sys/wasm/stdio.rs | 11 +---------- src/libstd/sys/windows/stdio.rs | 15 +-------------- 8 files changed, 13 insertions(+), 80 deletions(-) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index c0570ae60a19c..4fd114884e367 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -286,6 +286,9 @@ pub use self::stdio::{_print, _eprint}; #[doc(no_inline, hidden)] pub use self::stdio::{set_panic, set_print}; +// Used inside the standard library for panic output. +pub(crate) use self::stdio::stderr_raw; + pub mod prelude; mod buffered; mod cursor; diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 4068c0f9c7de5..4fc11db3ff52a 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -32,7 +32,9 @@ struct StdoutRaw(stdio::Stdout); /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stderr_raw` function. -struct StderrRaw(stdio::Stderr); +/// +/// Not exposed, but used inside the standard library for panic output. +pub(crate) struct StderrRaw(stdio::Stderr); /// Constructs a new raw handle to the standard input of this process. /// @@ -61,7 +63,9 @@ fn stdout_raw() -> io::Result { stdio::Stdout::new().map(StdoutRaw) } /// /// The returned handle has no external synchronization or buffering layered on /// top. -fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } +/// +/// Not exposed, but used inside the standard library for panic output. +pub(crate) fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } impl Read for StdinRaw { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } diff --git a/src/libstd/sys/cloudabi/stdio.rs b/src/libstd/sys/cloudabi/stdio.rs index 2cd3477cd519d..8ec92326dbd8c 100644 --- a/src/libstd/sys/cloudabi/stdio.rs +++ b/src/libstd/sys/cloudabi/stdio.rs @@ -49,19 +49,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(abi::errno::BADF as i32) } diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index e814b0942c14d..66f84e1752752 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -47,19 +47,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(::sys::syscall::EBADF as i32) } @@ -67,5 +54,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } diff --git a/src/libstd/sys/sgx/stdio.rs b/src/libstd/sys/sgx/stdio.rs index 6f206cd9a51d7..c8efb931d6fe5 100644 --- a/src/libstd/sys/sgx/stdio.rs +++ b/src/libstd/sys/sgx/stdio.rs @@ -46,19 +46,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn is_ebadf(err: &io::Error) -> bool { diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index 715f2eafb2d9b..e23723222be35 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -47,19 +47,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(libc::EBADF as i32) } @@ -67,5 +54,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index 201619b2fb139..015d3f2066042 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -45,15 +45,6 @@ impl Stderr { } } -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - (&*self).write(data) - } - fn flush(&mut self) -> io::Result<()> { - (&*self).flush() - } -} - pub const STDIN_BUF_SIZE: usize = 0; pub fn is_ebadf(_err: &io::Error) -> bool { @@ -62,7 +53,7 @@ pub fn is_ebadf(_err: &io::Error) -> bool { pub fn panic_output() -> Option { if cfg!(feature = "wasm_syscall") { - Stderr::new().ok() + io::stderr_raw().ok() } else { None } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 0ea19a855257b..45a12d075bad0 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -165,19 +165,6 @@ impl Stderr { } } -// FIXME: right now this raw stderr handle is used in a few places because -// std::io::stderr_raw isn't exposed, but once that's exposed this impl -// should go away -impl io::Write for Stderr { - fn write(&mut self, data: &[u8]) -> io::Result { - Stderr::write(self, data) - } - - fn flush(&mut self) -> io::Result<()> { - Stderr::flush(self) - } -} - impl Output { pub fn handle(&self) -> c::HANDLE { match *self { @@ -216,5 +203,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = 8 * 1024; pub fn panic_output() -> Option { - Stderr::new().ok() + io::stderr_raw().ok() } From cc20ed678e4fc782cbbe55501bb3e300b5430c37 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 14 Feb 2019 06:29:10 +0100 Subject: [PATCH 16/23] Remove unused Read implementation on sys::Windows::Stdin --- src/libstd/sys/windows/stdio.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 45a12d075bad0..617124c36b30a 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,7 +1,5 @@ #![unstable(issue = "0", feature = "windows_stdio")] -use io::prelude::*; - use cmp; use io::{self, Cursor}; use ptr; @@ -130,13 +128,6 @@ impl Stdin { } } -#[unstable(reason = "not public", issue = "0", feature = "fd_read")] -impl<'a> Read for &'a Stdin { - fn read(&mut self, buf: &mut [u8]) -> io::Result { - (**self).read(buf) - } -} - impl Stdout { pub fn new() -> io::Result { Ok(Stdout) From f411852add58637a3b8628a8f70146106bdc9719 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Thu, 14 Feb 2019 06:35:20 +0100 Subject: [PATCH 17/23] Refactor Windows stdio and remove stdin double buffering --- src/libstd/sys/windows/process.rs | 4 +- src/libstd/sys/windows/stdio.rs | 290 ++++++++++++++++++------------ 2 files changed, 178 insertions(+), 116 deletions(-) diff --git a/src/libstd/sys/windows/process.rs b/src/libstd/sys/windows/process.rs index 08a166bd8c504..2527168a968c4 100644 --- a/src/libstd/sys/windows/process.rs +++ b/src/libstd/sys/windows/process.rs @@ -252,9 +252,9 @@ impl Stdio { // should still be unavailable so propagate the // INVALID_HANDLE_VALUE. Stdio::Inherit => { - match stdio::get(stdio_id) { + match stdio::get_handle(stdio_id) { Ok(io) => { - let io = Handle::new(io.handle()); + let io = Handle::new(io); let ret = io.duplicate(0, true, c::DUPLICATE_SAME_ACCESS); io.into_raw(); diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 617124c36b30a..ccfe0ced82fbc 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,131 +1,226 @@ #![unstable(issue = "0", feature = "windows_stdio")] +use cell::Cell; use cmp; -use io::{self, Cursor}; +use io; use ptr; use str; -use sync::Mutex; use sys::c; use sys::cvt; use sys::handle::Handle; -pub enum Output { - Console(c::HANDLE), - Pipe(c::HANDLE), -} - +// Don't cache handles but get them fresh for every read/write. This allows us to track changes to +// the value over time (such as if a process calls `SetStdHandle` while it's running). See #40490. pub struct Stdin { - utf8: Mutex>>, + high_surrogate: Cell, } pub struct Stdout; pub struct Stderr; -pub fn get(handle: c::DWORD) -> io::Result { - let handle = unsafe { c::GetStdHandle(handle) }; +// Apparently Windows doesn't handle large reads on stdin or writes to stdout/stderr well (see +// #13304 for details). +// +// From MSDN (2011): "The storage for this buffer is allocated from a shared heap for the +// process that is 64 KB in size. The maximum size of the buffer will depend on heap usage." +// +// We choose the cap at 8 KiB because libuv does the same, and it seems to be acceptable so far. +const MAX_BUFFER_SIZE: usize = 8192; + +// The standard buffer size of BufReader for Stdin should be able to hold 3x more bytes than there +// are `u16`'s in MAX_BUFFER_SIZE. This ensures the read data can always be completely decoded from +// UTF-16 to UTF-8. +pub const STDIN_BUF_SIZE: usize = MAX_BUFFER_SIZE / 2 * 3; + +pub fn get_handle(handle_id: c::DWORD) -> io::Result { + let handle = unsafe { c::GetStdHandle(handle_id) }; if handle == c::INVALID_HANDLE_VALUE { Err(io::Error::last_os_error()) } else if handle.is_null() { Err(io::Error::from_raw_os_error(c::ERROR_INVALID_HANDLE as i32)) } else { - let mut out = 0; - match unsafe { c::GetConsoleMode(handle, &mut out) } { - 0 => Ok(Output::Pipe(handle)), - _ => Ok(Output::Console(handle)), - } + Ok(handle) } } -fn write(handle: c::DWORD, data: &[u8]) -> io::Result { - let handle = match get(handle)? { - Output::Console(c) => c, - Output::Pipe(p) => { - let handle = Handle::new(p); - let ret = handle.write(data); - handle.into_raw(); - return ret - } - }; +fn is_console(handle: c::HANDLE) -> bool { + // `GetConsoleMode` will return false (0) if this is a pipe (we don't care about the reported + // mode). This will only detect Windows Console, not other terminals connected to a pipe like + // MSYS. Which is exactly what we need, as only Windows Console needs a conversion to UTF-16. + let mut mode = 0; + unsafe { c::GetConsoleMode(handle, &mut mode) != 0 } +} - // As with stdin on windows, stdout often can't handle writes of large - // sizes. For an example, see #14940. For this reason, don't try to - // write the entire output buffer on windows. - // - // For some other references, it appears that this problem has been - // encountered by others [1] [2]. We choose the number 8K just because - // libuv does the same. +fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { + let handle = get_handle(handle_id)?; + if !is_console(handle) { + let handle = Handle::new(handle); + let ret = handle.write(data); + handle.into_raw(); // Don't close the handle + return ret; + } + + // As the console is meant for presenting text, we assume bytes of `data` come from a string + // and are encoded as UTF-8, which needs to be encoded as UTF-16. // - // [1]: https://tahoe-lafs.org/trac/tahoe-lafs/ticket/1232 - // [2]: http://www.mail-archive.com/log4net-dev@logging.apache.org/msg00661.html - const OUT_MAX: usize = 8192; - let len = cmp::min(data.len(), OUT_MAX); + // If the data is not valid UTF-8 we write out as many bytes as are valid. + // Only when there are no valid bytes (which will happen on the next call), return an error. + let len = cmp::min(data.len(), MAX_BUFFER_SIZE); let utf8 = match str::from_utf8(&data[..len]) { Ok(s) => s, - Err(ref e) if e.valid_up_to() == 0 => return Err(invalid_encoding()), + Err(ref e) if e.valid_up_to() == 0 => { + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ + see https://github.com/rust-lang/rust/issues/23344")) + }, Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(), }; let utf16 = utf8.encode_utf16().collect::>(); + + let mut written = write_u16s(handle, &utf16)?; + + // Figure out how many bytes of as UTF-8 were written away as UTF-16. + if written >= utf16.len() { + Ok(utf8.len()) + } else { + // Make sure we didn't end up writing only half of a surrogate pair (even though the chance + // is tiny). Because it is not possible for user code to re-slice `data` in such a way that + // a missing surrogate can be produced (and also because of the UTF-8 validation above), + // write the missing surrogate out now. + // Buffering it would mean we have to lie about the number of bytes written. + let first_char_remaining = utf16[written]; + if first_char_remaining >= 0xDCEE && first_char_remaining <= 0xDFFF { // low surrogate + // We just hope this works, and give up otherwise + let _ = write_u16s(handle, &utf16[written..written]); + written += 1; + } + // Calculate the number of bytes of `utf8` that were actually written. + let mut count = 0; + for ch in utf16[..written].iter() { + count += match ch { + 0x0000 ..= 0x007F => 1, + 0x0080 ..= 0x07FF => 2, + 0xDCEE ..= 0xDFFF => 1, // Low surrogate. We already counted 3 bytes for the other. + _ => 3, + }; + } + Ok(count) + } +} + +fn write_u16s(handle: c::HANDLE, data: &[u16]) -> io::Result { let mut written = 0; cvt(unsafe { c::WriteConsoleW(handle, - utf16.as_ptr() as c::LPCVOID, - utf16.len() as u32, + data.as_ptr() as c::LPCVOID, + data.len() as u32, &mut written, ptr::null_mut()) })?; - - // FIXME if this only partially writes the utf16 buffer then we need to - // figure out how many bytes of `data` were actually written - assert_eq!(written as usize, utf16.len()); - Ok(utf8.len()) + Ok(written as usize) } impl Stdin { pub fn new() -> io::Result { - Ok(Stdin { - utf8: Mutex::new(Cursor::new(Vec::new())), - }) + Ok(Stdin { high_surrogate: Cell::new(0) }) } pub fn read(&self, buf: &mut [u8]) -> io::Result { - let handle = match get(c::STD_INPUT_HANDLE)? { - Output::Console(c) => c, - Output::Pipe(p) => { - let handle = Handle::new(p); - let ret = handle.read(buf); - handle.into_raw(); - return ret - } + let handle = get_handle(c::STD_INPUT_HANDLE)?; + if !is_console(handle) { + let handle = Handle::new(handle); + let ret = handle.read(buf); + handle.into_raw(); // Don't close the handle + return ret; + } + + if buf.len() == 0 { + return Ok(0); + } else if buf.len() < 4 { + return Err(io::Error::new(io::ErrorKind::InvalidInput, + "Windows stdin in console mode does not support a buffer too small to; \ + guarantee holding one arbitrary UTF-8 character (4 bytes)")) + } + + let mut utf16_buf = [0u16; MAX_BUFFER_SIZE / 2]; + // In the worst case, an UTF-8 string can take 3 bytes for every `u16` of an UTF-16. So + // we can read at most a third of `buf.len()` chars and uphold the guarantee no data gets + // lost. + let amount = cmp::min(buf.len() / 3, utf16_buf.len()); + let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; + let utf16 = &utf16_buf[..read]; + + // FIXME: it would be nice if we could directly decode into the buffer instead of doing an + // allocation. + let data = match String::from_utf16(&utf16) { + Ok(utf8) => utf8.into_bytes(), + Err(..) => { + // We can't really do any better than forget all data and return an error. + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdin in console mode does not support non-UTF-16 input; \ + encountered unpaired surrogate")) + }, }; - let mut utf8 = self.utf8.lock().unwrap(); - // Read more if the buffer is empty - if utf8.position() as usize == utf8.get_ref().len() { - let mut utf16 = vec![0u16; 0x1000]; - let mut num = 0; - let mut input_control = readconsole_input_control(CTRL_Z_MASK); - cvt(unsafe { - c::ReadConsoleW(handle, - utf16.as_mut_ptr() as c::LPVOID, - utf16.len() as u32, - &mut num, - &mut input_control as c::PCONSOLE_READCONSOLE_CONTROL) - })?; - utf16.truncate(num as usize); - // FIXME: what to do about this data that has already been read? - let mut data = match String::from_utf16(&utf16) { - Ok(utf8) => utf8.into_bytes(), - Err(..) => return Err(invalid_encoding()), - }; - if let Some(&last_byte) = data.last() { - if last_byte == CTRL_Z { - data.pop(); - } + buf.copy_from_slice(&data); + Ok(data.len()) + } + + // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our + // buffer size, and keep it around for the next read hoping to put them together. + // This is a best effort, and may not work if we are not the only reader on Stdin. + pub fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) + -> io::Result + { + // Insert possibly remaining unpaired surrogate from last read. + let mut start = 0; + if self.high_surrogate.get() != 0 { + buf[0] = self.high_surrogate.replace(0); + start = 1; + if amount == 1 { + // Special case: `Stdin::read` guarantees we can always read at least one new `u16` + // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least + // 4 bytes. + amount = 2; + } + } + let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; + + if amount > 0 { + let last_char = buf[amount - 1]; + if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate + self.high_surrogate.set(last_char); + amount -= 1; } - *utf8 = Cursor::new(data); } + Ok(amount) + } +} + +fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { + // Configure the `pInputControl` parameter to not only return on `\r\n` but also Ctrl-Z, the + // traditional DOS method to indicate end of character stream / user input (SUB). + // See #38274 and https://stackoverflow.com/questions/43836040/win-api-readconsole. + const CTRL_Z: u16 = 0x1A; + const CTRL_Z_MASK: c::ULONG = 1 << CTRL_Z; + let mut input_control = c::CONSOLE_READCONSOLE_CONTROL { + nLength: ::mem::size_of::() as c::ULONG, + nInitialChars: 0, + dwCtrlWakeupMask: CTRL_Z_MASK, + dwControlKeyState: 0, + }; + + let mut amount = 0; + cvt(unsafe { + c::ReadConsoleW(handle, + buf.as_mut_ptr() as c::LPVOID, + buf.len() as u32, + &mut amount, + &mut input_control as c::PCONSOLE_READCONSOLE_CONTROL) + })?; - // MemReader shouldn't error here since we just filled it - utf8.read(buf) + if amount > 0 && buf[amount as usize - 1] == CTRL_Z { + amount -= 1; } + Ok(amount as usize) } impl Stdout { @@ -156,43 +251,10 @@ impl Stderr { } } -impl Output { - pub fn handle(&self) -> c::HANDLE { - match *self { - Output::Console(c) => c, - Output::Pipe(c) => c, - } - } -} - -fn invalid_encoding() -> io::Error { - io::Error::new(io::ErrorKind::InvalidData, - "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ - see https://github.com/rust-lang/rust/issues/23344") -} - -fn readconsole_input_control(wakeup_mask: c::ULONG) -> c::CONSOLE_READCONSOLE_CONTROL { - c::CONSOLE_READCONSOLE_CONTROL { - nLength: ::mem::size_of::() as c::ULONG, - nInitialChars: 0, - dwCtrlWakeupMask: wakeup_mask, - dwControlKeyState: 0, - } -} - -const CTRL_Z: u8 = 0x1A; -const CTRL_Z_MASK: c::ULONG = 0x4000000; //1 << 0x1A - pub fn is_ebadf(err: &io::Error) -> bool { err.raw_os_error() == Some(c::ERROR_INVALID_HANDLE as i32) } -// The default buffer capacity is 64k, but apparently windows -// doesn't like 64k reads on stdin. See #13304 for details, but the -// idea is that on windows we use a slightly smaller buffer that's -// been seen to be acceptable. -pub const STDIN_BUF_SIZE: usize = 8 * 1024; - pub fn panic_output() -> Option { io::stderr_raw().ok() } From b09803e8694b3ad83f988e30b4f0a3903ebe2632 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Tue, 19 Feb 2019 15:57:59 +0100 Subject: [PATCH 18/23] Address review comments --- src/libstd/sys/windows/stdio.rs | 58 ++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 22 deletions(-) diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index ccfe0ced82fbc..6b9b36f6a924b 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,6 +1,7 @@ #![unstable(issue = "0", feature = "windows_stdio")] use cell::Cell; +use char::decode_utf16; use cmp; use io; use ptr; @@ -64,22 +65,27 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { // // If the data is not valid UTF-8 we write out as many bytes as are valid. // Only when there are no valid bytes (which will happen on the next call), return an error. - let len = cmp::min(data.len(), MAX_BUFFER_SIZE); + let len = cmp::min(data.len(), MAX_BUFFER_SIZE / 2); let utf8 = match str::from_utf8(&data[..len]) { Ok(s) => s, Err(ref e) if e.valid_up_to() == 0 => { return Err(io::Error::new(io::ErrorKind::InvalidData, - "Windows stdio in console mode does not support non-UTF-8 byte sequences; \ - see https://github.com/rust-lang/rust/issues/23344")) + "Windows stdio in console mode does not support writing non-UTF-8 byte sequences")) }, Err(e) => str::from_utf8(&data[..e.valid_up_to()]).unwrap(), }; - let utf16 = utf8.encode_utf16().collect::>(); + let mut utf16 = [0u16; MAX_BUFFER_SIZE / 2]; + let mut len_utf16 = 0; + for (chr, dest) in utf8.encode_utf16().zip(utf16.iter_mut()) { + *dest = chr; + len_utf16 += 1; + } + let utf16 = &utf16[..len_utf16]; let mut written = write_u16s(handle, &utf16)?; // Figure out how many bytes of as UTF-8 were written away as UTF-16. - if written >= utf16.len() { + if written == utf16.len() { Ok(utf8.len()) } else { // Make sure we didn't end up writing only half of a surrogate pair (even though the chance @@ -90,7 +96,7 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { let first_char_remaining = utf16[written]; if first_char_remaining >= 0xDCEE && first_char_remaining <= 0xDFFF { // low surrogate // We just hope this works, and give up otherwise - let _ = write_u16s(handle, &utf16[written..written]); + let _ = write_u16s(handle, &utf16[written..written+1]); written += 1; } // Calculate the number of bytes of `utf8` that were actually written. @@ -103,6 +109,7 @@ fn write(handle_id: c::DWORD, data: &[u8]) -> io::Result { _ => 3, }; } + debug_assert!(String::from_utf16(&utf16[..written]).unwrap() == utf8[..count]); Ok(count) } } @@ -137,7 +144,7 @@ impl Stdin { return Ok(0); } else if buf.len() < 4 { return Err(io::Error::new(io::ErrorKind::InvalidInput, - "Windows stdin in console mode does not support a buffer too small to; \ + "Windows stdin in console mode does not support a buffer too small to \ guarantee holding one arbitrary UTF-8 character (4 bytes)")) } @@ -147,27 +154,14 @@ impl Stdin { // lost. let amount = cmp::min(buf.len() / 3, utf16_buf.len()); let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; - let utf16 = &utf16_buf[..read]; - // FIXME: it would be nice if we could directly decode into the buffer instead of doing an - // allocation. - let data = match String::from_utf16(&utf16) { - Ok(utf8) => utf8.into_bytes(), - Err(..) => { - // We can't really do any better than forget all data and return an error. - return Err(io::Error::new(io::ErrorKind::InvalidData, - "Windows stdin in console mode does not support non-UTF-16 input; \ - encountered unpaired surrogate")) - }, - }; - buf.copy_from_slice(&data); - Ok(data.len()) + utf16_to_utf8(&utf16_buf[..read], buf) } // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our // buffer size, and keep it around for the next read hoping to put them together. // This is a best effort, and may not work if we are not the only reader on Stdin. - pub fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) + fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) -> io::Result { // Insert possibly remaining unpaired surrogate from last read. @@ -223,6 +217,26 @@ fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { Ok(amount as usize) } +#[allow(unused)] +fn utf16_to_utf8(utf16: &[u16], utf8: &mut [u8]) -> io::Result { + let mut written = 0; + for chr in decode_utf16(utf16.iter().cloned()) { + match chr { + Ok(chr) => { + chr.encode_utf8(&mut utf8[written..]); + written += chr.len_utf8(); + } + Err(_) => { + // We can't really do any better than forget all data and return an error. + return Err(io::Error::new(io::ErrorKind::InvalidData, + "Windows stdin in console mode does not support non-UTF-16 input; \ + encountered unpaired surrogate")) + } + } + } + Ok(written) +} + impl Stdout { pub fn new() -> io::Result { Ok(Stdout) From 6464e32ea97fe0b18f75becc82cba9b19dfe453c Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Wed, 20 Feb 2019 08:18:29 +0100 Subject: [PATCH 19/23] Use standard Read/Write traits in sys::stdio --- src/libstd/sys/cloudabi/stdio.rs | 16 +++++-- src/libstd/sys/redox/stdio.rs | 22 +++++---- src/libstd/sys/sgx/stdio.rs | 22 +++++---- src/libstd/sys/unix/stdio.rs | 22 +++++---- src/libstd/sys/wasm/stdio.rs | 26 ++++++---- src/libstd/sys/windows/stdio.rs | 81 ++++++++++++++++++-------------- 6 files changed, 114 insertions(+), 75 deletions(-) diff --git a/src/libstd/sys/cloudabi/stdio.rs b/src/libstd/sys/cloudabi/stdio.rs index 8ec92326dbd8c..81d79213f615c 100644 --- a/src/libstd/sys/cloudabi/stdio.rs +++ b/src/libstd/sys/cloudabi/stdio.rs @@ -9,8 +9,10 @@ impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, _: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, _buf: &mut [u8]) -> io::Result { Ok(0) } } @@ -19,15 +21,17 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, _: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, _buf: &[u8]) -> io::Result { Err(io::Error::new( io::ErrorKind::BrokenPipe, "Stdout is not connected to any output in this environment", )) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -36,15 +40,17 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, _: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, _buf: &[u8]) -> io::Result { Err(io::Error::new( io::ErrorKind::BrokenPipe, "Stderr is not connected to any output in this environment", )) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index 66f84e1752752..b4eb01fd6cc52 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -8,10 +8,12 @@ pub struct Stderr(()); impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let fd = FileDesc::new(0); - let ret = fd.read(data); + let ret = fd.read(buf); fd.into_raw(); ret } @@ -19,30 +21,34 @@ impl Stdin { impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(1); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { cvt(syscall::fsync(1)).and(Ok(())) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(2); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { cvt(syscall::fsync(2)).and(Ok(())) } } diff --git a/src/libstd/sys/sgx/stdio.rs b/src/libstd/sys/sgx/stdio.rs index c8efb931d6fe5..57d66ed9a853c 100644 --- a/src/libstd/sys/sgx/stdio.rs +++ b/src/libstd/sys/sgx/stdio.rs @@ -16,32 +16,38 @@ fn with_std_fd R, R>(fd: abi::Fd, f: F) -> R { impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { - with_std_fd(abi::FD_STDIN, |fd| fd.read(data)) +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + with_std_fd(abi::FD_STDIN, |fd| fd.read(buf)) } } impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - with_std_fd(abi::FD_STDOUT, |fd| fd.write(data)) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + with_std_fd(abi::FD_STDOUT, |fd| fd.write(buf)) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDOUT, |fd| fd.flush()) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - with_std_fd(abi::FD_STDERR, |fd| fd.write(data)) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + with_std_fd(abi::FD_STDERR, |fd| fd.write(buf)) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { with_std_fd(abi::FD_STDERR, |fd| fd.flush()) } } diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index e23723222be35..82bd2fbf77612 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -8,10 +8,12 @@ pub struct Stderr(()); impl Stdin { pub fn new() -> io::Result { Ok(Stdin(())) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let fd = FileDesc::new(libc::STDIN_FILENO); - let ret = fd.read(data); + let ret = fd.read(buf); fd.into_raw(); // do not close this FD ret } @@ -19,30 +21,34 @@ impl Stdin { impl Stdout { pub fn new() -> io::Result { Ok(Stdout(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(libc::STDOUT_FILENO); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); // do not close this FD ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } impl Stderr { pub fn new() -> io::Result { Ok(Stderr(())) } +} - pub fn write(&self, data: &[u8]) -> io::Result { +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { let fd = FileDesc::new(libc::STDERR_FILENO); - let ret = fd.write(data); + let ret = fd.write(buf); fd.into_raw(); // do not close this FD ret } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index 015d3f2066042..657655004e9bf 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -9,9 +9,11 @@ impl Stdin { pub fn new() -> io::Result { Ok(Stdin) } +} - pub fn read(&self, data: &mut [u8]) -> io::Result { - Ok(ReadSysCall::perform(0, data)) +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { + Ok(ReadSysCall::perform(0, buf)) } } @@ -19,13 +21,15 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - WriteSysCall::perform(1, data); - Ok(data.len()) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + WriteSysCall::perform(1, buf); + Ok(buf.len()) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -34,13 +38,15 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - WriteSysCall::perform(2, data); - Ok(data.len()) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + WriteSysCall::perform(2, buf); + Ok(buf.len()) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 6b9b36f6a924b..5963541a89338 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -1,6 +1,5 @@ #![unstable(issue = "0", feature = "windows_stdio")] -use cell::Cell; use char::decode_utf16; use cmp; use io; @@ -13,7 +12,7 @@ use sys::handle::Handle; // Don't cache handles but get them fresh for every read/write. This allows us to track changes to // the value over time (such as if a process calls `SetStdHandle` while it's running). See #40490. pub struct Stdin { - high_surrogate: Cell, + surrogate: u16, } pub struct Stdout; pub struct Stderr; @@ -128,10 +127,12 @@ fn write_u16s(handle: c::HANDLE, data: &[u16]) -> io::Result { impl Stdin { pub fn new() -> io::Result { - Ok(Stdin { high_surrogate: Cell::new(0) }) + Ok(Stdin { surrogate: 0 }) } +} - pub fn read(&self, buf: &mut [u8]) -> io::Result { +impl io::Read for Stdin { + fn read(&mut self, buf: &mut [u8]) -> io::Result { let handle = get_handle(c::STD_INPUT_HANDLE)?; if !is_console(handle) { let handle = Handle::new(handle); @@ -153,40 +154,44 @@ impl Stdin { // we can read at most a third of `buf.len()` chars and uphold the guarantee no data gets // lost. let amount = cmp::min(buf.len() / 3, utf16_buf.len()); - let read = self.read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount)?; + let read = read_u16s_fixup_surrogates(handle, &mut utf16_buf, amount, &mut self.surrogate)?; utf16_to_utf8(&utf16_buf[..read], buf) } +} - // We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our - // buffer size, and keep it around for the next read hoping to put them together. - // This is a best effort, and may not work if we are not the only reader on Stdin. - fn read_u16s_fixup_surrogates(&self, handle: c::HANDLE, buf: &mut [u16], mut amount: usize) - -> io::Result - { - // Insert possibly remaining unpaired surrogate from last read. - let mut start = 0; - if self.high_surrogate.get() != 0 { - buf[0] = self.high_surrogate.replace(0); - start = 1; - if amount == 1 { - // Special case: `Stdin::read` guarantees we can always read at least one new `u16` - // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least - // 4 bytes. - amount = 2; - } + +// We assume that if the last `u16` is an unpaired surrogate they got sliced apart by our +// buffer size, and keep it around for the next read hoping to put them together. +// This is a best effort, and may not work if we are not the only reader on Stdin. +fn read_u16s_fixup_surrogates(handle: c::HANDLE, + buf: &mut [u16], + mut amount: usize, + surrogate: &mut u16) -> io::Result +{ + // Insert possibly remaining unpaired surrogate from last read. + let mut start = 0; + if *surrogate != 0 { + buf[0] = *surrogate; + *surrogate = 0; + start = 1; + if amount == 1 { + // Special case: `Stdin::read` guarantees we can always read at least one new `u16` + // and combine it with an unpaired surrogate, because the UTF-8 buffer is at least + // 4 bytes. + amount = 2; } - let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; + } + let mut amount = read_u16s(handle, &mut buf[start..amount])? + start; - if amount > 0 { - let last_char = buf[amount - 1]; - if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate - self.high_surrogate.set(last_char); - amount -= 1; - } + if amount > 0 { + let last_char = buf[amount - 1]; + if last_char >= 0xD800 && last_char <= 0xDBFF { // high surrogate + *surrogate = last_char; + amount -= 1; } - Ok(amount) } + Ok(amount) } fn read_u16s(handle: c::HANDLE, buf: &mut [u16]) -> io::Result { @@ -241,12 +246,14 @@ impl Stdout { pub fn new() -> io::Result { Ok(Stdout) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - write(c::STD_OUTPUT_HANDLE, data) +impl io::Write for Stdout { + fn write(&mut self, buf: &[u8]) -> io::Result { + write(c::STD_OUTPUT_HANDLE, buf) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } @@ -255,12 +262,14 @@ impl Stderr { pub fn new() -> io::Result { Ok(Stderr) } +} - pub fn write(&self, data: &[u8]) -> io::Result { - write(c::STD_ERROR_HANDLE, data) +impl io::Write for Stderr { + fn write(&mut self, buf: &[u8]) -> io::Result { + write(c::STD_ERROR_HANDLE, buf) } - pub fn flush(&self) -> io::Result<()> { + fn flush(&mut self) -> io::Result<()> { Ok(()) } } From 7cfddfb4e40584dc206cedef7c70b5fecb03d6a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ho=C3=A0ng=20=C4=90=E1=BB=A9c=20Hi=E1=BA=BFu?= Date: Sun, 16 Dec 2018 15:50:49 +0700 Subject: [PATCH 20/23] Improve parsing diagnostic for negative supertrait bounds --- src/libsyntax/parse/parser.rs | 80 ++++++++++++++++++++------- src/test/ui/parser/issue-33418.fixed | 14 +++++ src/test/ui/parser/issue-33418.rs | 16 ++++++ src/test/ui/parser/issue-33418.stderr | 42 ++++++++++++++ 4 files changed, 131 insertions(+), 21 deletions(-) create mode 100644 src/test/ui/parser/issue-33418.fixed create mode 100644 src/test/ui/parser/issue-33418.rs create mode 100644 src/test/ui/parser/issue-33418.stderr diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index e6a48912c48b4..5c2b02067d994 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1731,7 +1731,7 @@ impl<'a> Parser<'a> { } } else if self.eat_keyword(keywords::Impl) { // Always parse bounds greedily for better error recovery. - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; impl_dyn_multi = bounds.len() > 1 || self.prev_token_kind == PrevTokenKind::Plus; TyKind::ImplTrait(ast::DUMMY_NODE_ID, bounds) } else if self.check_keyword(keywords::Dyn) && @@ -1740,13 +1740,13 @@ impl<'a> Parser<'a> { !can_continue_type_after_non_fn_ident(t))) { self.bump(); // `dyn` // Always parse bounds greedily for better error recovery. - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; impl_dyn_multi = bounds.len() > 1 || self.prev_token_kind == PrevTokenKind::Plus; TyKind::TraitObject(bounds, TraitObjectSyntax::Dyn) } else if self.check(&token::Question) || self.check_lifetime() && self.look_ahead(1, |t| t.is_like_plus()) { // Bound list (trait object type) - TyKind::TraitObject(self.parse_generic_bounds_common(allow_plus)?, + TyKind::TraitObject(self.parse_generic_bounds_common(allow_plus, None)?, TraitObjectSyntax::None) } else if self.eat_lt() { // Qualified path @@ -1792,7 +1792,7 @@ impl<'a> Parser<'a> { let mut bounds = vec![GenericBound::Trait(poly_trait_ref, TraitBoundModifier::None)]; if parse_plus { self.eat_plus(); // `+`, or `+=` gets split and `+` is discarded - bounds.append(&mut self.parse_generic_bounds()?); + bounds.append(&mut self.parse_generic_bounds(None)?); } Ok(TyKind::TraitObject(bounds, TraitObjectSyntax::None)) } @@ -1817,7 +1817,7 @@ impl<'a> Parser<'a> { } self.bump(); // `+` - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; let sum_span = ty.span.to(self.prev_span); let mut err = struct_span_err!(self.sess.span_diagnostic, sum_span, E0178, @@ -5492,11 +5492,16 @@ impl<'a> Parser<'a> { /// TY_BOUND = TY_BOUND_NOPAREN | (TY_BOUND_NOPAREN) /// TY_BOUND_NOPAREN = [?] [for] SIMPLE_PATH (e.g., `?for<'a: 'b> m::Trait<'a>`) /// ``` - fn parse_generic_bounds_common(&mut self, allow_plus: bool) -> PResult<'a, GenericBounds> { + fn parse_generic_bounds_common(&mut self, + allow_plus: bool, + colon_span: Option) -> PResult<'a, GenericBounds> { let mut bounds = Vec::new(); + let mut negative_bounds = Vec::new(); + let mut last_plus_span = None; loop { // This needs to be synchronized with `Token::can_begin_bound`. let is_bound_start = self.check_path() || self.check_lifetime() || + self.check(&token::Not) || // used for error reporting only self.check(&token::Question) || self.check_keyword(keywords::For) || self.check(&token::OpenDelim(token::Paren)); @@ -5504,6 +5509,7 @@ impl<'a> Parser<'a> { let lo = self.span; let has_parens = self.eat(&token::OpenDelim(token::Paren)); let inner_lo = self.span; + let is_negative = self.eat(&token::Not); let question = if self.eat(&token::Question) { Some(self.prev_span) } else { None }; if self.token.is_lifetime() { if let Some(question_span) = question { @@ -5534,13 +5540,20 @@ impl<'a> Parser<'a> { if has_parens { self.expect(&token::CloseDelim(token::Paren))?; } - let poly_trait = PolyTraitRef::new(lifetime_defs, path, lo.to(self.prev_span)); - let modifier = if question.is_some() { - TraitBoundModifier::Maybe + let poly_span = lo.to(self.prev_span); + if is_negative { + negative_bounds.push( + last_plus_span.or(colon_span).unwrap() + .to(poly_span)); } else { - TraitBoundModifier::None - }; - bounds.push(GenericBound::Trait(poly_trait, modifier)); + let poly_trait = PolyTraitRef::new(lifetime_defs, path, poly_span); + let modifier = if question.is_some() { + TraitBoundModifier::Maybe + } else { + TraitBoundModifier::None + }; + bounds.push(GenericBound::Trait(poly_trait, modifier)); + } } } else { break @@ -5548,14 +5561,39 @@ impl<'a> Parser<'a> { if !allow_plus || !self.eat_plus() { break - } + } else { + last_plus_span = Some(self.prev_span); + } + } + + if !negative_bounds.is_empty() { + let plural = negative_bounds.len() > 1; + let mut err = self.struct_span_err(negative_bounds, + "negative trait bounds are not supported"); + let bound_list = colon_span.unwrap().to(self.prev_span); + let mut new_bound_list = String::new(); + if !bounds.is_empty() { + let mut snippets = bounds.iter().map(|bound| bound.span()) + .map(|span| self.sess.source_map().span_to_snippet(span)); + while let Some(Ok(snippet)) = snippets.next() { + new_bound_list.push_str(" + "); + new_bound_list.push_str(&snippet); + } + new_bound_list = new_bound_list.replacen(" +", ":", 1); + } + err.span_suggestion_short(bound_list, + &format!("remove the trait bound{}", + if plural { "s" } else { "" }), + new_bound_list, + Applicability::MachineApplicable); + err.emit(); } return Ok(bounds); } - fn parse_generic_bounds(&mut self) -> PResult<'a, GenericBounds> { - self.parse_generic_bounds_common(true) + fn parse_generic_bounds(&mut self, colon_span: Option) -> PResult<'a, GenericBounds> { + self.parse_generic_bounds_common(true, colon_span) } /// Parses bounds of a lifetime parameter `BOUND + BOUND + BOUND`, possibly with trailing `+`. @@ -5583,7 +5621,7 @@ impl<'a> Parser<'a> { // Parse optional colon and param bounds. let bounds = if self.eat(&token::Colon) { - self.parse_generic_bounds()? + self.parse_generic_bounds(None)? } else { Vec::new() }; @@ -5615,7 +5653,7 @@ impl<'a> Parser<'a> { // Parse optional colon and param bounds. let bounds = if self.eat(&token::Colon) { - self.parse_generic_bounds()? + self.parse_generic_bounds(None)? } else { Vec::new() }; @@ -6028,7 +6066,7 @@ impl<'a> Parser<'a> { // or with mandatory equality sign and the second type. let ty = self.parse_ty()?; if self.eat(&token::Colon) { - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; where_clause.predicates.push(ast::WherePredicate::BoundPredicate( ast::WhereBoundPredicate { span: lo.to(self.prev_span), @@ -6542,14 +6580,14 @@ impl<'a> Parser<'a> { // Parse optional colon and supertrait bounds. let bounds = if self.eat(&token::Colon) { - self.parse_generic_bounds()? + self.parse_generic_bounds(Some(self.prev_span))? } else { Vec::new() }; if self.eat(&token::Eq) { // it's a trait alias - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; tps.where_clause = self.parse_where_clause()?; self.expect(&token::Semi)?; if is_auto == IsAuto::Yes { @@ -7584,7 +7622,7 @@ impl<'a> Parser<'a> { tps.where_clause = self.parse_where_clause()?; let alias = if existential { self.expect(&token::Colon)?; - let bounds = self.parse_generic_bounds()?; + let bounds = self.parse_generic_bounds(None)?; AliasKind::Existential(bounds) } else { self.expect(&token::Eq)?; diff --git a/src/test/ui/parser/issue-33418.fixed b/src/test/ui/parser/issue-33418.fixed new file mode 100644 index 0000000000000..df11f2d855ce0 --- /dev/null +++ b/src/test/ui/parser/issue-33418.fixed @@ -0,0 +1,14 @@ +// run-rustfix + +trait Tr {} //~ ERROR negative trait bounds are not supported +trait Tr2: SuperA {} //~ ERROR negative trait bounds are not supported +trait Tr3: SuperB {} //~ ERROR negative trait bounds are not supported +trait Tr4: SuperB + SuperD {} +trait Tr5 {} + +trait SuperA {} +trait SuperB {} +trait SuperC {} +trait SuperD {} + +fn main() {} diff --git a/src/test/ui/parser/issue-33418.rs b/src/test/ui/parser/issue-33418.rs new file mode 100644 index 0000000000000..5bb5f2afca377 --- /dev/null +++ b/src/test/ui/parser/issue-33418.rs @@ -0,0 +1,16 @@ +// run-rustfix + +trait Tr: !SuperA {} //~ ERROR negative trait bounds are not supported +trait Tr2: SuperA + !SuperB {} //~ ERROR negative trait bounds are not supported +trait Tr3: !SuperA + SuperB {} //~ ERROR negative trait bounds are not supported +trait Tr4: !SuperA + SuperB //~ ERROR negative trait bounds are not supported + + !SuperC + SuperD {} +trait Tr5: !SuperA //~ ERROR negative trait bounds are not supported + + !SuperB {} + +trait SuperA {} +trait SuperB {} +trait SuperC {} +trait SuperD {} + +fn main() {} diff --git a/src/test/ui/parser/issue-33418.stderr b/src/test/ui/parser/issue-33418.stderr new file mode 100644 index 0000000000000..bfe44588a5b0b --- /dev/null +++ b/src/test/ui/parser/issue-33418.stderr @@ -0,0 +1,42 @@ +error: negative trait bounds are not supported + --> $DIR/issue-33418.rs:3:9 + | +LL | trait Tr: !SuperA {} //~ ERROR negative trait bounds are not supported + | ^^^^^^^^^ help: remove the trait bound + +error: negative trait bounds are not supported + --> $DIR/issue-33418.rs:4:19 + | +LL | trait Tr2: SuperA + !SuperB {} //~ ERROR negative trait bounds are not supported + | ---------^^^^^^^^^ + | | + | help: remove the trait bound + +error: negative trait bounds are not supported + --> $DIR/issue-33418.rs:5:10 + | +LL | trait Tr3: !SuperA + SuperB {} //~ ERROR negative trait bounds are not supported + | ^^^^^^^^^--------- + | | + | help: remove the trait bound + +error: negative trait bounds are not supported + --> $DIR/issue-33418.rs:6:10 + | +LL | trait Tr4: !SuperA + SuperB //~ ERROR negative trait bounds are not supported + | __________-^^^^^^^^ +LL | | + !SuperC + SuperD {} + | |_____^^^^^^^^^________- help: remove the trait bounds + +error: negative trait bounds are not supported + --> $DIR/issue-33418.rs:8:10 + | +LL | trait Tr5: !SuperA //~ ERROR negative trait bounds are not supported + | __________-^^^^^^^^ +LL | | + !SuperB {} + | | ^^^^^^^^- + | |_____________| + | help: remove the trait bounds + +error: aborting due to 5 previous errors + From 1a944b0d5bbc1b70423f6d710021bea5ba16f0b2 Mon Sep 17 00:00:00 2001 From: Paul Dicker Date: Sat, 23 Feb 2019 12:11:10 +0100 Subject: [PATCH 21/23] Remove pub(crate) from stderr_raw --- src/libstd/io/mod.rs | 3 --- src/libstd/io/stdio.rs | 8 ++------ src/libstd/sys/redox/stdio.rs | 2 +- src/libstd/sys/unix/stdio.rs | 2 +- src/libstd/sys/wasm/stdio.rs | 2 +- src/libstd/sys/windows/stdio.rs | 2 +- 6 files changed, 6 insertions(+), 13 deletions(-) diff --git a/src/libstd/io/mod.rs b/src/libstd/io/mod.rs index 4fd114884e367..c0570ae60a19c 100644 --- a/src/libstd/io/mod.rs +++ b/src/libstd/io/mod.rs @@ -286,9 +286,6 @@ pub use self::stdio::{_print, _eprint}; #[doc(no_inline, hidden)] pub use self::stdio::{set_panic, set_print}; -// Used inside the standard library for panic output. -pub(crate) use self::stdio::stderr_raw; - pub mod prelude; mod buffered; mod cursor; diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index 4fc11db3ff52a..4068c0f9c7de5 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -32,9 +32,7 @@ struct StdoutRaw(stdio::Stdout); /// /// This handle is not synchronized or buffered in any fashion. Constructed via /// the `std::io::stdio::stderr_raw` function. -/// -/// Not exposed, but used inside the standard library for panic output. -pub(crate) struct StderrRaw(stdio::Stderr); +struct StderrRaw(stdio::Stderr); /// Constructs a new raw handle to the standard input of this process. /// @@ -63,9 +61,7 @@ fn stdout_raw() -> io::Result { stdio::Stdout::new().map(StdoutRaw) } /// /// The returned handle has no external synchronization or buffering layered on /// top. -/// -/// Not exposed, but used inside the standard library for panic output. -pub(crate) fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } +fn stderr_raw() -> io::Result { stdio::Stderr::new().map(StderrRaw) } impl Read for StdinRaw { fn read(&mut self, buf: &mut [u8]) -> io::Result { self.0.read(buf) } diff --git a/src/libstd/sys/redox/stdio.rs b/src/libstd/sys/redox/stdio.rs index b4eb01fd6cc52..8571b38cefa1c 100644 --- a/src/libstd/sys/redox/stdio.rs +++ b/src/libstd/sys/redox/stdio.rs @@ -60,5 +60,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - io::stderr_raw().ok() + Stderr::new().ok() } diff --git a/src/libstd/sys/unix/stdio.rs b/src/libstd/sys/unix/stdio.rs index 82bd2fbf77612..56b75bf9f7931 100644 --- a/src/libstd/sys/unix/stdio.rs +++ b/src/libstd/sys/unix/stdio.rs @@ -60,5 +60,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { pub const STDIN_BUF_SIZE: usize = ::sys_common::io::DEFAULT_BUF_SIZE; pub fn panic_output() -> Option { - io::stderr_raw().ok() + Stderr::new().ok() } diff --git a/src/libstd/sys/wasm/stdio.rs b/src/libstd/sys/wasm/stdio.rs index 657655004e9bf..d7540fd815c98 100644 --- a/src/libstd/sys/wasm/stdio.rs +++ b/src/libstd/sys/wasm/stdio.rs @@ -59,7 +59,7 @@ pub fn is_ebadf(_err: &io::Error) -> bool { pub fn panic_output() -> Option { if cfg!(feature = "wasm_syscall") { - io::stderr_raw().ok() + Stderr::new().ok() } else { None } diff --git a/src/libstd/sys/windows/stdio.rs b/src/libstd/sys/windows/stdio.rs index 5963541a89338..99445f4e0d45d 100644 --- a/src/libstd/sys/windows/stdio.rs +++ b/src/libstd/sys/windows/stdio.rs @@ -279,5 +279,5 @@ pub fn is_ebadf(err: &io::Error) -> bool { } pub fn panic_output() -> Option { - io::stderr_raw().ok() + Stderr::new().ok() } From bde4d1945c909883a057f0e1d2a4594f4144a1b7 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Fri, 22 Feb 2019 13:21:42 +0100 Subject: [PATCH 22/23] rustdoc: support methods on primitives in intra-doc links --- src/librustdoc/clean/mod.rs | 6 ++-- .../passes/collect_intra_doc_links.rs | 35 +++++++++++++++++++ src/test/rustdoc/intra-link-prim-methods.rs | 3 ++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 src/test/rustdoc/intra-link-prim-methods.rs diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 172bd9180622f..b094a55a61707 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -976,11 +976,13 @@ impl Attributes { "https://doc.rust-lang.org/nightly", }; // This is a primitive so the url is done "by hand". + let tail = fragment.find('#').unwrap_or_else(|| fragment.len()); Some((s.clone(), - format!("{}{}std/primitive.{}.html", + format!("{}{}std/primitive.{}.html{}", url, if !url.ends_with('/') { "/" } else { "" }, - fragment))) + &fragment[..tail], + &fragment[tail..]))) } else { panic!("This isn't a primitive?!"); } diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index cf2c3aa484600..1795f761a82e9 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -1,6 +1,7 @@ use rustc::lint as lint; use rustc::hir; use rustc::hir::def::Def; +use rustc::hir::def_id::DefId; use rustc::ty; use syntax; use syntax::ast::{self, Ident, NodeId}; @@ -126,6 +127,17 @@ impl<'a, 'tcx, 'rcx> LinkCollector<'a, 'tcx, 'rcx> { path = name.clone(); } } + if let Some(prim) = is_primitive(&path, false) { + let did = primitive_impl(cx, &path).ok_or(())?; + return cx.tcx.associated_items(did) + .find(|item| item.ident.name == item_name) + .and_then(|item| match item.kind { + ty::AssociatedKind::Method => Some("method"), + _ => None, + }) + .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_name)))) + .ok_or(()); + } // FIXME: `with_scope` requires the `NodeId` of a module. let ty = cx.resolver.borrow_mut() @@ -589,3 +601,26 @@ fn is_primitive(path_str: &str, is_val: bool) -> Option { PRIMITIVES.iter().find(|x| x.0 == path_str).map(|x| x.1) } } + +fn primitive_impl(cx: &DocContext, path_str: &str) -> Option { + let tcx = cx.tcx; + match path_str { + "u8" => tcx.lang_items().u8_impl(), + "u16" => tcx.lang_items().u16_impl(), + "u32" => tcx.lang_items().u32_impl(), + "u64" => tcx.lang_items().u64_impl(), + "u128" => tcx.lang_items().u128_impl(), + "usize" => tcx.lang_items().usize_impl(), + "i8" => tcx.lang_items().i8_impl(), + "i16" => tcx.lang_items().i16_impl(), + "i32" => tcx.lang_items().i32_impl(), + "i64" => tcx.lang_items().i64_impl(), + "i128" => tcx.lang_items().i128_impl(), + "isize" => tcx.lang_items().isize_impl(), + "f32" => tcx.lang_items().f32_impl(), + "f64" => tcx.lang_items().f64_impl(), + "str" => tcx.lang_items().str_impl(), + "char" => tcx.lang_items().char_impl(), + _ => None, + } +} diff --git a/src/test/rustdoc/intra-link-prim-methods.rs b/src/test/rustdoc/intra-link-prim-methods.rs new file mode 100644 index 0000000000000..af0426b22c557 --- /dev/null +++ b/src/test/rustdoc/intra-link-prim-methods.rs @@ -0,0 +1,3 @@ +#![deny(intra_doc_link_resolution_failure)] + +//! A [`char`] and its [`char::len_utf8`]. From aa0fa752c455eca11db012be76caa94b15804350 Mon Sep 17 00:00:00 2001 From: Trevor Spiteri Date: Sat, 23 Feb 2019 18:02:40 +0100 Subject: [PATCH 23/23] fix build for Rust 2018 now that #58100 has been merged --- src/librustdoc/passes/collect_intra_doc_links.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 1795f761a82e9..42fa3e2006b42 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -602,7 +602,7 @@ fn is_primitive(path_str: &str, is_val: bool) -> Option { } } -fn primitive_impl(cx: &DocContext, path_str: &str) -> Option { +fn primitive_impl(cx: &DocContext<'_, '_, '_>, path_str: &str) -> Option { let tcx = cx.tcx; match path_str { "u8" => tcx.lang_items().u8_impl(),