From de8660ab610f046bbdeb1a90f2c94cba1d3d3185 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 24 Jun 2019 09:15:07 +0100 Subject: [PATCH 1/5] typeck: merge opaque type inference logic This commit merges the logic used for opaque type type inference for impl Trait and non-impl Trait cases. This fixes an ICE where existential types used in the return types of functions would be allowed to have an out-of-scope generic type parameter. --- src/librustc/infer/opaque_types/mod.rs | 76 +++++++++- src/librustc_typeck/check/writeback.rs | 153 ++------------------ src/test/ui/impl-trait/issue-55872-1.rs | 22 +++ src/test/ui/impl-trait/issue-55872-1.stderr | 33 +++++ src/test/ui/impl-trait/issue-55872-2.rs | 20 +++ src/test/ui/impl-trait/issue-55872-2.stderr | 21 +++ src/test/ui/impl-trait/issue-55872.rs | 19 +++ src/test/ui/impl-trait/issue-55872.stderr | 12 ++ 8 files changed, 217 insertions(+), 139 deletions(-) create mode 100644 src/test/ui/impl-trait/issue-55872-1.rs create mode 100644 src/test/ui/impl-trait/issue-55872-1.stderr create mode 100644 src/test/ui/impl-trait/issue-55872-2.rs create mode 100644 src/test/ui/impl-trait/issue-55872-2.stderr create mode 100644 src/test/ui/impl-trait/issue-55872.rs create mode 100644 src/test/ui/impl-trait/issue-55872.stderr diff --git a/src/librustc/infer/opaque_types/mod.rs b/src/librustc/infer/opaque_types/mod.rs index f43e3fa0b7787..9c105b6b891f9 100644 --- a/src/librustc/infer/opaque_types/mod.rs +++ b/src/librustc/infer/opaque_types/mod.rs @@ -4,6 +4,7 @@ use crate::hir::Node; use crate::infer::outlives::free_region_map::FreeRegionRelations; use crate::infer::{self, InferCtxt, InferOk, TypeVariableOrigin, TypeVariableOriginKind}; use crate::middle::region; +use crate::mir::interpret::ConstValue; use crate::traits::{self, PredicateObligation}; use crate::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder, TypeVisitor}; use crate::ty::subst::{InternalSubsts, Kind, SubstsRef, UnpackedKind}; @@ -553,6 +554,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { def_id: DefId, opaque_defn: &OpaqueTypeDecl<'tcx>, instantiated_ty: Ty<'tcx>, + span: Span, ) -> Ty<'tcx> { debug!( "infer_opaque_definition_from_instantiation(def_id={:?}, instantiated_ty={:?})", @@ -584,6 +586,7 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { def_id, map, instantiated_ty, + span, )); debug!("infer_opaque_definition_from_instantiation: definition_ty={:?}", definition_ty); @@ -761,6 +764,9 @@ struct ReverseMapper<'tcx> { /// initially `Some`, set to `None` once error has been reported hidden_ty: Option>, + + /// Span of function being checked. + span: Span, } impl ReverseMapper<'tcx> { @@ -770,6 +776,7 @@ impl ReverseMapper<'tcx> { opaque_type_def_id: DefId, map: FxHashMap, Kind<'tcx>>, hidden_ty: Ty<'tcx>, + span: Span, ) -> Self { Self { tcx, @@ -778,6 +785,7 @@ impl ReverseMapper<'tcx> { map, map_missing_regions_to_empty: false, hidden_ty: Some(hidden_ty), + span, } } @@ -812,10 +820,11 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> { _ => { } } + let generics = self.tcx().generics_of(self.opaque_type_def_id); match self.map.get(&r.into()).map(|k| k.unpack()) { Some(UnpackedKind::Lifetime(r1)) => r1, Some(u) => panic!("region mapped to unexpected kind: {:?}", u), - None => { + None if generics.parent.is_some() => { if !self.map_missing_regions_to_empty && !self.tainted_by_errors { if let Some(hidden_ty) = self.hidden_ty.take() { unexpected_hidden_region_diagnostic( @@ -829,6 +838,21 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> { } self.tcx.lifetimes.re_empty } + None => { + self.tcx.sess + .struct_span_err( + self.span, + "non-defining existential type use in defining scope" + ) + .span_label( + self.span, + format!("lifetime `{}` is part of concrete type but not used in \ + parameter list of existential type", r), + ) + .emit(); + + self.tcx().global_tcx().mk_region(ty::ReStatic) + }, } } @@ -890,9 +914,59 @@ impl TypeFolder<'tcx> for ReverseMapper<'tcx> { self.tcx.mk_generator(def_id, ty::GeneratorSubsts { substs }, movability) } + ty::Param(..) => { + // Look it up in the substitution list. + match self.map.get(&ty.into()).map(|k| k.unpack()) { + // Found it in the substitution list; replace with the parameter from the + // existential type. + Some(UnpackedKind::Type(t1)) => t1, + Some(u) => panic!("type mapped to unexpected kind: {:?}", u), + None => { + self.tcx.sess + .struct_span_err( + self.span, + &format!("type parameter `{}` is part of concrete type but not \ + used in parameter list for existential type", ty), + ) + .emit(); + + self.tcx().types.err + } + } + } + _ => ty.super_fold_with(self), } } + + fn fold_const(&mut self, ct: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> { + trace!("checking const {:?}", ct); + // Find a const parameter + match ct.val { + ConstValue::Param(..) => { + // Look it up in the substitution list. + match self.map.get(&ct.into()).map(|k| k.unpack()) { + // Found it in the substitution list, replace with the parameter from the + // existential type. + Some(UnpackedKind::Const(c1)) => c1, + Some(u) => panic!("const mapped to unexpected kind: {:?}", u), + None => { + self.tcx.sess + .struct_span_err( + self.span, + &format!("const parameter `{}` is part of concrete type but not \ + used in parameter list for existential type", ct) + ) + .emit(); + + self.tcx().consts.err + } + } + } + + _ => ct, + } + } } struct Instantiator<'a, 'tcx> { diff --git a/src/librustc_typeck/check/writeback.rs b/src/librustc_typeck/check/writeback.rs index 28711e32a4c51..2a33b47c93a61 100644 --- a/src/librustc_typeck/check/writeback.rs +++ b/src/librustc_typeck/check/writeback.rs @@ -9,10 +9,8 @@ use rustc::hir::def_id::{DefId, DefIndex}; use rustc::hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc::infer::InferCtxt; use rustc::ty::adjustment::{Adjust, Adjustment, PointerCast}; -use rustc::ty::fold::{BottomUpFolder, TypeFoldable, TypeFolder}; -use rustc::ty::subst::UnpackedKind; +use rustc::ty::fold::{TypeFoldable, TypeFolder}; use rustc::ty::{self, Ty, TyCtxt}; -use rustc::mir::interpret::ConstValue; use rustc::util::nodemap::DefIdSet; use rustc_data_structures::sync::Lrc; use std::mem; @@ -440,141 +438,20 @@ impl<'cx, 'tcx> WritebackCx<'cx, 'tcx> { debug_assert!(!instantiated_ty.has_escaping_bound_vars()); - let generics = self.tcx().generics_of(def_id); - - let definition_ty = if generics.parent.is_some() { - // `impl Trait` - self.fcx.infer_opaque_definition_from_instantiation( - def_id, - opaque_defn, - instantiated_ty, - ) - } else { - // Prevent: - // * `fn foo() -> Foo` - // * `fn foo() -> Foo` - // from being defining. - - // Also replace all generic params with the ones from the existential type - // definition so that - // ```rust - // existential type Foo: 'static; - // fn foo() -> Foo { .. } - // ``` - // figures out the concrete type with `U`, but the stored type is with `T`. - instantiated_ty.fold_with(&mut BottomUpFolder { - tcx: self.tcx().global_tcx(), - ty_op: |ty| { - trace!("checking type {:?}", ty); - // Find a type parameter. - if let ty::Param(..) = ty.sty { - // Look it up in the substitution list. - assert_eq!(opaque_defn.substs.len(), generics.params.len()); - for (subst, param) in opaque_defn.substs.iter().zip(&generics.params) { - if let UnpackedKind::Type(subst) = subst.unpack() { - if subst == ty { - // Found it in the substitution list; replace with the - // parameter from the existential type. - return self.tcx() - .global_tcx() - .mk_ty_param(param.index, param.name); - } - } - } - self.tcx() - .sess - .struct_span_err( - span, - &format!( - "type parameter `{}` is part of concrete type but not used \ - in parameter list for existential type", - ty, - ), - ) - .emit(); - return self.tcx().types.err; - } - ty - }, - lt_op: |region| { - match region { - // Skip static and bound regions: they don't require substitution. - ty::ReStatic | ty::ReLateBound(..) => region, - _ => { - trace!("checking {:?}", region); - for (subst, p) in opaque_defn.substs.iter().zip(&generics.params) { - if let UnpackedKind::Lifetime(subst) = subst.unpack() { - if subst == region { - // Found it in the substitution list; replace with the - // parameter from the existential type. - let reg = ty::EarlyBoundRegion { - def_id: p.def_id, - index: p.index, - name: p.name, - }; - trace!("replace {:?} with {:?}", region, reg); - return self.tcx() - .global_tcx() - .mk_region(ty::ReEarlyBound(reg)); - } - } - } - trace!("opaque_defn: {:#?}", opaque_defn); - trace!("generics: {:#?}", generics); - self.tcx() - .sess - .struct_span_err( - span, - "non-defining existential type use in defining scope", - ) - .span_label( - span, - format!( - "lifetime `{}` is part of concrete type but not used \ - in parameter list of existential type", - region, - ), - ) - .emit(); - self.tcx().global_tcx().mk_region(ty::ReStatic) - } - } - }, - ct_op: |ct| { - trace!("checking const {:?}", ct); - // Find a const parameter - if let ConstValue::Param(..) = ct.val { - // look it up in the substitution list - assert_eq!(opaque_defn.substs.len(), generics.params.len()); - for (subst, param) in opaque_defn.substs.iter() - .zip(&generics.params) { - if let UnpackedKind::Const(subst) = subst.unpack() { - if subst == ct { - // found it in the substitution list, replace with the - // parameter from the existential type - return self.tcx() - .global_tcx() - .mk_const_param(param.index, param.name, ct.ty); - } - } - } - self.tcx() - .sess - .struct_span_err( - span, - &format!( - "const parameter `{}` is part of concrete type but not \ - used in parameter list for existential type", - ct, - ), - ) - .emit(); - return self.tcx().consts.err; - } - ct - } - }) - }; + // Prevent: + // * `fn foo() -> Foo` + // * `fn foo() -> Foo` + // from being defining. + + // Also replace all generic params with the ones from the existential type + // definition so that + // ```rust + // existential type Foo: 'static; + // fn foo() -> Foo { .. } + // ``` + // figures out the concrete type with `U`, but the stored type is with `T`. + let definition_ty = self.fcx.infer_opaque_definition_from_instantiation( + def_id, opaque_defn, instantiated_ty, span); if let ty::Opaque(defin_ty_def_id, _substs) = definition_ty.sty { if def_id == defin_ty_def_id { diff --git a/src/test/ui/impl-trait/issue-55872-1.rs b/src/test/ui/impl-trait/issue-55872-1.rs new file mode 100644 index 0000000000000..5095b26f8a4e0 --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872-1.rs @@ -0,0 +1,22 @@ +// ignore-tidy-linelength +#![feature(existential_type)] + +pub trait Bar +{ + type E: Copy; + + fn foo() -> Self::E; +} + +impl Bar for S { + existential type E: Copy; + //~^ ERROR the trait bound `S: std::marker::Copy` is not satisfied in `(S, T)` [E0277] + //~^^ ERROR the trait bound `T: std::marker::Copy` is not satisfied in `(S, T)` [E0277] + + fn foo() -> Self::E { + //~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for existential type + (S::default(), T::default()) + } +} + +fn main() {} diff --git a/src/test/ui/impl-trait/issue-55872-1.stderr b/src/test/ui/impl-trait/issue-55872-1.stderr new file mode 100644 index 0000000000000..04b4d2d4a50a5 --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872-1.stderr @@ -0,0 +1,33 @@ +error[E0277]: the trait bound `S: std::marker::Copy` is not satisfied in `(S, T)` + --> $DIR/issue-55872-1.rs:12:5 + | +LL | existential type E: Copy; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ within `(S, T)`, the trait `std::marker::Copy` is not implemented for `S` + | + = help: consider adding a `where S: std::marker::Copy` bound + = note: required because it appears within the type `(S, T)` + = note: the return type of a function must have a statically known size + +error[E0277]: the trait bound `T: std::marker::Copy` is not satisfied in `(S, T)` + --> $DIR/issue-55872-1.rs:12:5 + | +LL | existential type E: Copy; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ within `(S, T)`, the trait `std::marker::Copy` is not implemented for `T` + | + = help: consider adding a `where T: std::marker::Copy` bound + = note: required because it appears within the type `(S, T)` + = note: the return type of a function must have a statically known size + +error: type parameter `T` is part of concrete type but not used in parameter list for existential type + --> $DIR/issue-55872-1.rs:16:37 + | +LL | fn foo() -> Self::E { + | _____________________________________^ +LL | | +LL | | (S::default(), T::default()) +LL | | } + | |_____^ + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/impl-trait/issue-55872-2.rs b/src/test/ui/impl-trait/issue-55872-2.rs new file mode 100644 index 0000000000000..2bdeb14bdc3e6 --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872-2.rs @@ -0,0 +1,20 @@ +// edition:2018 +// ignore-tidy-linelength +#![feature(async_await, existential_type)] + +pub trait Bar { + type E: Copy; + + fn foo() -> Self::E; +} + +impl Bar for S { + existential type E: Copy; + //~^ ERROR the trait bound `impl std::future::Future: std::marker::Copy` is not satisfied [E0277] + fn foo() -> Self::E { + //~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for existential type + async {} + } +} + +fn main() {} diff --git a/src/test/ui/impl-trait/issue-55872-2.stderr b/src/test/ui/impl-trait/issue-55872-2.stderr new file mode 100644 index 0000000000000..2505a82ee23cb --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872-2.stderr @@ -0,0 +1,21 @@ +error[E0277]: the trait bound `impl std::future::Future: std::marker::Copy` is not satisfied + --> $DIR/issue-55872-2.rs:12:5 + | +LL | existential type E: Copy; + | ^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::marker::Copy` is not implemented for `impl std::future::Future` + | + = note: the return type of a function must have a statically known size + +error: type parameter `T` is part of concrete type but not used in parameter list for existential type + --> $DIR/issue-55872-2.rs:14:28 + | +LL | fn foo() -> Self::E { + | ____________________________^ +LL | | +LL | | async {} +LL | | } + | |_____^ + +error: aborting due to 2 previous errors + +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/impl-trait/issue-55872.rs b/src/test/ui/impl-trait/issue-55872.rs new file mode 100644 index 0000000000000..95604545c37f1 --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872.rs @@ -0,0 +1,19 @@ +// ignore-tidy-linelength +#![feature(existential_type)] + +pub trait Bar { + type E: Copy; + + fn foo() -> Self::E; +} + +impl Bar for S { + existential type E: Copy; + + fn foo() -> Self::E { + //~^ ERROR type parameter `T` is part of concrete type but not used in parameter list for existential type + || () + } +} + +fn main() {} diff --git a/src/test/ui/impl-trait/issue-55872.stderr b/src/test/ui/impl-trait/issue-55872.stderr new file mode 100644 index 0000000000000..487f276e317e9 --- /dev/null +++ b/src/test/ui/impl-trait/issue-55872.stderr @@ -0,0 +1,12 @@ +error: type parameter `T` is part of concrete type but not used in parameter list for existential type + --> $DIR/issue-55872.rs:13:28 + | +LL | fn foo() -> Self::E { + | ____________________________^ +LL | | +LL | | || () +LL | | } + | |_____^ + +error: aborting due to previous error + From e808d921ddc0ad81a200934fc4caabc34094afe5 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Fri, 5 Jul 2019 11:38:40 +0200 Subject: [PATCH 2/5] Replace SliceConcatExt trait with inherent methods and SliceConcat helper trait Before this change `SliceConcatExt` was an unstable extension trait with stable methods. It was in the libstd prelude, so that its methods could be used on the stable channel. This replaces it with inherent methods, which can be used without any addition to the prelude. Since the methods are stable and very generic (with for example a return type that depends on the types of parameters), an helper trait is still needed. But now that trait does not need to be in scope for the methods to be used. Removing this depedency on the libstd prelude allows the methods to be used in `#![no_std]` crate that use liballoc, which does not have its own implicitly-imported prelude. --- src/liballoc/prelude/v1.rs | 1 - src/liballoc/slice.rs | 129 ++++++++++++++++++++----------------- src/liballoc/str.rs | 14 ++-- src/libstd/prelude/mod.rs | 3 - src/libstd/prelude/v1.rs | 3 - 5 files changed, 76 insertions(+), 74 deletions(-) diff --git a/src/liballoc/prelude/v1.rs b/src/liballoc/prelude/v1.rs index b6b01395ad632..3cb285bf0492f 100644 --- a/src/liballoc/prelude/v1.rs +++ b/src/liballoc/prelude/v1.rs @@ -6,6 +6,5 @@ #[unstable(feature = "alloc_prelude", issue = "58935")] pub use crate::borrow::ToOwned; #[unstable(feature = "alloc_prelude", issue = "58935")] pub use crate::boxed::Box; -#[unstable(feature = "alloc_prelude", issue = "58935")] pub use crate::slice::SliceConcatExt; #[unstable(feature = "alloc_prelude", issue = "58935")] pub use crate::string::{String, ToString}; #[unstable(feature = "alloc_prelude", issue = "58935")] pub use crate::vec::Vec; diff --git a/src/liballoc/slice.rs b/src/liballoc/slice.rs index f7b0a5e703cc3..bc4ae16798478 100644 --- a/src/liballoc/slice.rs +++ b/src/liballoc/slice.rs @@ -484,6 +484,56 @@ impl [T] { } buf } + + /// Flattens a slice of `T` into a single value `Self::Output`. + /// + /// # Examples + /// + /// ``` + /// assert_eq!(["hello", "world"].concat(), "helloworld"); + /// assert_eq!([[1, 2], [3, 4]].concat(), [1, 2, 3, 4]); + /// ``` + #[stable(feature = "rust1", since = "1.0.0")] + pub fn concat(&self) -> T::Output + where T: SliceConcat + { + SliceConcat::concat(self) + } + + /// Flattens a slice of `T` into a single value `Self::Output`, placing a + /// given separator between each. + /// + /// # Examples + /// + /// ``` + /// assert_eq!(["hello", "world"].join(" "), "hello world"); + /// assert_eq!([[1, 2], [3, 4]].join(&0), [1, 2, 0, 3, 4]); + /// ``` + #[stable(feature = "rename_connect_to_join", since = "1.3.0")] + pub fn join(&self, sep: &Separator) -> T::Output + where T: SliceConcat + { + SliceConcat::join(self, sep) + } + + /// Flattens a slice of `T` into a single value `Self::Output`, placing a + /// given separator between each. + /// + /// # Examples + /// + /// ``` + /// # #![allow(deprecated)] + /// assert_eq!(["hello", "world"].connect(" "), "hello world"); + /// assert_eq!([[1, 2], [3, 4]].connect(&0), [1, 2, 0, 3, 4]); + /// ``` + #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_deprecated(since = "1.3.0", reason = "renamed to join")] + pub fn connect(&self, sep: &Separator) -> T::Output + where T: SliceConcat + { + SliceConcat::join(self, sep) + } + } #[lang = "slice_u8_alloc"] @@ -527,87 +577,46 @@ impl [u8] { //////////////////////////////////////////////////////////////////////////////// // Extension traits for slices over specific kinds of data //////////////////////////////////////////////////////////////////////////////// -#[unstable(feature = "slice_concat_ext", - reason = "trait should not have to exist", - issue = "27747")] -/// An extension trait for concatenating slices -/// -/// While this trait is unstable, the methods are stable. `SliceConcatExt` is -/// included in the [standard library prelude], so you can use [`join()`] and -/// [`concat()`] as if they existed on `[T]` itself. -/// -/// [standard library prelude]: ../../std/prelude/index.html -/// [`join()`]: #tymethod.join -/// [`concat()`]: #tymethod.concat -pub trait SliceConcatExt { - #[unstable(feature = "slice_concat_ext", - reason = "trait should not have to exist", - issue = "27747")] + +/// Helper trait for [`[T]::concat`](../../std/primitive.slice.html#method.concat) +/// and [`[T]::join`](../../std/primitive.slice.html#method.join) +#[unstable(feature = "slice_concat_trait", issue = "27747")] +pub trait SliceConcat: Sized { + #[unstable(feature = "slice_concat_trait", issue = "27747")] /// The resulting type after concatenation type Output; - /// Flattens a slice of `T` into a single value `Self::Output`. - /// - /// # Examples - /// - /// ``` - /// assert_eq!(["hello", "world"].concat(), "helloworld"); - /// assert_eq!([[1, 2], [3, 4]].concat(), [1, 2, 3, 4]); - /// ``` - #[stable(feature = "rust1", since = "1.0.0")] - fn concat(&self) -> Self::Output; - - /// Flattens a slice of `T` into a single value `Self::Output`, placing a - /// given separator between each. - /// - /// # Examples - /// - /// ``` - /// assert_eq!(["hello", "world"].join(" "), "hello world"); - /// assert_eq!([[1, 2], [3, 4]].join(&0), [1, 2, 0, 3, 4]); - /// ``` - #[stable(feature = "rename_connect_to_join", since = "1.3.0")] - fn join(&self, sep: &T) -> Self::Output; + /// Implementation of [`[T]::concat`](../../std/primitive.slice.html#method.concat) + #[unstable(feature = "slice_concat_trait", issue = "27747")] + fn concat(slice: &[Self]) -> Self::Output; - /// Flattens a slice of `T` into a single value `Self::Output`, placing a - /// given separator between each. - /// - /// # Examples - /// - /// ``` - /// # #![allow(deprecated)] - /// assert_eq!(["hello", "world"].connect(" "), "hello world"); - /// assert_eq!([[1, 2], [3, 4]].connect(&0), [1, 2, 0, 3, 4]); - /// ``` - #[stable(feature = "rust1", since = "1.0.0")] - #[rustc_deprecated(since = "1.3.0", reason = "renamed to join")] - fn connect(&self, sep: &T) -> Self::Output { - self.join(sep) - } + /// Implementation of [`[T]::join`](../../std/primitive.slice.html#method.join) + #[unstable(feature = "slice_concat_trait", issue = "27747")] + fn join(slice: &[Self], sep: &Separator) -> Self::Output; } #[unstable(feature = "slice_concat_ext", reason = "trait should not have to exist", issue = "27747")] -impl> SliceConcatExt for [V] { +impl> SliceConcat for V { type Output = Vec; - fn concat(&self) -> Vec { - let size = self.iter().map(|slice| slice.borrow().len()).sum(); + fn concat(slice: &[Self]) -> Vec { + let size = slice.iter().map(|slice| slice.borrow().len()).sum(); let mut result = Vec::with_capacity(size); - for v in self { + for v in slice { result.extend_from_slice(v.borrow()) } result } - fn join(&self, sep: &T) -> Vec { - let mut iter = self.iter(); + fn join(slice: &[Self], sep: &T) -> Vec { + let mut iter = slice.iter(); let first = match iter.next() { Some(first) => first, None => return vec![], }; - let size = self.iter().map(|slice| slice.borrow().len()).sum::() + self.len() - 1; + let size = slice.iter().map(|slice| slice.borrow().len()).sum::() + slice.len() - 1; let mut result = Vec::with_capacity(size); result.extend_from_slice(first.borrow()); diff --git a/src/liballoc/str.rs b/src/liballoc/str.rs index 70a93157c9ee2..37a1046d0942d 100644 --- a/src/liballoc/str.rs +++ b/src/liballoc/str.rs @@ -37,7 +37,7 @@ use core::unicode::conversions; use crate::borrow::ToOwned; use crate::boxed::Box; -use crate::slice::{SliceConcatExt, SliceIndex}; +use crate::slice::{SliceConcat, SliceIndex}; use crate::string::String; use crate::vec::Vec; @@ -74,16 +74,16 @@ pub use core::str::{EscapeDebug, EscapeDefault, EscapeUnicode}; #[unstable(feature = "slice_concat_ext", reason = "trait should not have to exist", issue = "27747")] -impl> SliceConcatExt for [S] { +impl> SliceConcat for S { type Output = String; - fn concat(&self) -> String { - self.join("") + fn concat(slice: &[Self]) -> String { + Self::join(slice, "") } - fn join(&self, sep: &str) -> String { + fn join(slice: &[Self], sep: &str) -> String { unsafe { - String::from_utf8_unchecked( join_generic_copy(self, sep.as_bytes()) ) + String::from_utf8_unchecked( join_generic_copy(slice, sep.as_bytes()) ) } } } @@ -126,7 +126,7 @@ macro_rules! copy_slice_and_advance { // Optimized join implementation that works for both Vec (T: Copy) and String's inner vec // Currently (2018-05-13) there is a bug with type inference and specialization (see issue #36262) -// For this reason SliceConcatExt is not specialized for T: Copy and SliceConcatExt is the +// For this reason SliceConcat is not specialized for T: Copy and SliceConcat is the // only user of this function. It is left in place for the time when that is fixed. // // the bounds for String-join are S: Borrow and for Vec-join Borrow<[T]> diff --git a/src/libstd/prelude/mod.rs b/src/libstd/prelude/mod.rs index 551e982a3c685..3085c3d829653 100644 --- a/src/libstd/prelude/mod.rs +++ b/src/libstd/prelude/mod.rs @@ -71,9 +71,6 @@ //! * [`std::result`]::[`Result`]::{`self`, `Ok`, `Err`}. A type for functions //! that may succeed or fail. Like [`Option`], its variants are exported as //! well. -//! * [`std::slice`]::[`SliceConcatExt`], a trait that exists for technical -//! reasons, but shouldn't have to exist. It provides a few useful methods on -//! slices. //! * [`std::string`]::{[`String`], [`ToString`]}, heap allocated strings. //! * [`std::vec`]::[`Vec`](../vec/struct.Vec.html), a growable, heap-allocated //! vector. diff --git a/src/libstd/prelude/v1.rs b/src/libstd/prelude/v1.rs index ce1e8e3319cf8..a863bebf4a264 100644 --- a/src/libstd/prelude/v1.rs +++ b/src/libstd/prelude/v1.rs @@ -60,9 +60,6 @@ pub use crate::boxed::Box; pub use crate::borrow::ToOwned; #[stable(feature = "rust1", since = "1.0.0")] #[doc(no_inline)] -pub use crate::slice::SliceConcatExt; -#[stable(feature = "rust1", since = "1.0.0")] -#[doc(no_inline)] pub use crate::string::{String, ToString}; #[stable(feature = "rust1", since = "1.0.0")] #[doc(no_inline)] From e98ea52762e7ecf492b3a1880deb78610ef22996 Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Wed, 1 May 2019 17:19:41 +1000 Subject: [PATCH 3/5] add key and value methods to DebugMap --- .../library-features/debug-map-key-value.md | 9 + src/libcore/fmt/builders.rs | 158 ++++++++++++++++-- src/libcore/tests/fmt/builders.rs | 93 ++++++++++- src/libcore/tests/lib.rs | 1 + 4 files changed, 236 insertions(+), 25 deletions(-) create mode 100644 src/doc/unstable-book/src/library-features/debug-map-key-value.md diff --git a/src/doc/unstable-book/src/library-features/debug-map-key-value.md b/src/doc/unstable-book/src/library-features/debug-map-key-value.md new file mode 100644 index 0000000000000..ae839bf2ac32b --- /dev/null +++ b/src/doc/unstable-book/src/library-features/debug-map-key-value.md @@ -0,0 +1,9 @@ +# `debug_map_key_value` + +The tracking issue for this feature is: [#62482] + +[#62482]: https://github.com/rust-lang/rust/issues/62482 + +------------------------ + +Add the methods `key` and `value` to `DebugMap` so that an entry can be formatted across multiple calls without additional buffering. diff --git a/src/libcore/fmt/builders.rs b/src/libcore/fmt/builders.rs index df86da5fc3906..abc97aacbb95a 100644 --- a/src/libcore/fmt/builders.rs +++ b/src/libcore/fmt/builders.rs @@ -1,37 +1,50 @@ use crate::fmt; -struct PadAdapter<'a> { - buf: &'a mut (dyn fmt::Write + 'a), +struct PadAdapter<'buf, 'state> { + buf: &'buf mut (dyn fmt::Write + 'buf), + state: &'state mut PadAdapterState, +} + +struct PadAdapterState { on_newline: bool, } -impl<'a> PadAdapter<'a> { - fn wrap<'b, 'c: 'a+'b>(fmt: &'c mut fmt::Formatter<'_>, slot: &'b mut Option) - -> fmt::Formatter<'b> { +impl Default for PadAdapterState { + fn default() -> Self { + PadAdapterState { + on_newline: true, + } + } +} + +impl<'buf, 'state> PadAdapter<'buf, 'state> { + fn wrap<'slot, 'fmt: 'buf+'slot>(fmt: &'fmt mut fmt::Formatter<'_>, + slot: &'slot mut Option, + state: &'state mut PadAdapterState) -> fmt::Formatter<'slot> { fmt.wrap_buf(move |buf| { *slot = Some(PadAdapter { buf, - on_newline: true, + state, }); slot.as_mut().unwrap() }) } } -impl fmt::Write for PadAdapter<'_> { +impl fmt::Write for PadAdapter<'_, '_> { fn write_str(&mut self, mut s: &str) -> fmt::Result { while !s.is_empty() { - if self.on_newline { + if self.state.on_newline { self.buf.write_str(" ")?; } let split = match s.find('\n') { Some(pos) => { - self.on_newline = true; + self.state.on_newline = true; pos + 1 } None => { - self.on_newline = false; + self.state.on_newline = false; s.len() } }; @@ -133,7 +146,8 @@ impl<'a, 'b: 'a> DebugStruct<'a, 'b> { self.fmt.write_str(" {\n")?; } let mut slot = None; - let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot); + let mut state = Default::default(); + let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut state); writer.write_str(name)?; writer.write_str(": ")?; value.fmt(&mut writer)?; @@ -279,7 +293,8 @@ impl<'a, 'b: 'a> DebugTuple<'a, 'b> { self.fmt.write_str("(\n")?; } let mut slot = None; - let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot); + let mut state = Default::default(); + let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut state); value.fmt(&mut writer)?; writer.write_str(",\n") } else { @@ -349,7 +364,8 @@ impl<'a, 'b: 'a> DebugInner<'a, 'b> { self.fmt.write_str("\n")?; } let mut slot = None; - let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot); + let mut state = Default::default(); + let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut state); entry.fmt(&mut writer)?; writer.write_str(",\n") } else { @@ -676,6 +692,9 @@ pub struct DebugMap<'a, 'b: 'a> { fmt: &'a mut fmt::Formatter<'b>, result: fmt::Result, has_fields: bool, + has_key: bool, + // The state of newlines is tracked between keys and values + state: PadAdapterState, } pub fn debug_map_new<'a, 'b>(fmt: &'a mut fmt::Formatter<'b>) -> DebugMap<'a, 'b> { @@ -684,6 +703,8 @@ pub fn debug_map_new<'a, 'b>(fmt: &'a mut fmt::Formatter<'b>) -> DebugMap<'a, 'b fmt, result, has_fields: false, + has_key: false, + state: Default::default(), } } @@ -712,25 +733,121 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> { /// ``` #[stable(feature = "debug_builders", since = "1.2.0")] pub fn entry(&mut self, key: &dyn fmt::Debug, value: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> { + self.key(key).value(value) + } + + /// Adds the key part of a new entry to the map output. + /// + /// This method, together with `value`, is an alternative to `entry` that + /// can be used when the complete entry isn't known upfront. Prefer the `entry` + /// method when it's possible to use. + /// + /// # Panics + /// + /// `key` must be called before `value` and each call to `key` must be followed + /// by a corresponding call to `value`. Otherwise this method will panic. + /// + /// # Examples + /// + /// ``` + /// use std::fmt; + /// + /// struct Foo(Vec<(String, i32)>); + /// + /// impl fmt::Debug for Foo { + /// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// fmt.debug_map() + /// .key(&"whole").value(&self.0) // We add the "whole" entry. + /// .finish() + /// } + /// } + /// + /// assert_eq!( + /// format!("{:?}", Foo(vec![("A".to_string(), 10), ("B".to_string(), 11)])), + /// "{\"whole\": [(\"A\", 10), (\"B\", 11)]}", + /// ); + /// ``` + #[unstable(feature = "debug_map_key_value", + reason = "recently added", + issue = "62482")] + pub fn key(&mut self, key: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> { + assert!(!self.has_key, "attempted to begin a new map entry \ + without completing the previous one"); + self.result = self.result.and_then(|_| { if self.is_pretty() { if !self.has_fields { self.fmt.write_str("\n")?; } let mut slot = None; - let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot); + self.state = Default::default(); + let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut self.state); key.fmt(&mut writer)?; writer.write_str(": ")?; - value.fmt(&mut writer)?; - writer.write_str(",\n") } else { if self.has_fields { self.fmt.write_str(", ")? } key.fmt(self.fmt)?; self.fmt.write_str(": ")?; - value.fmt(self.fmt) } + + self.has_key = true; + Ok(()) + }); + + self + } + + /// Adds the value part of a new entry to the map output. + /// + /// This method, together with `key`, is an alternative to `entry` that + /// can be used when the complete entry isn't known upfront. Prefer the `entry` + /// method when it's possible to use. + /// + /// # Panics + /// + /// `key` must be called before `value` and each call to `key` must be followed + /// by a corresponding call to `value`. Otherwise this method will panic. + /// + /// # Examples + /// + /// ``` + /// use std::fmt; + /// + /// struct Foo(Vec<(String, i32)>); + /// + /// impl fmt::Debug for Foo { + /// fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + /// fmt.debug_map() + /// .key(&"whole").value(&self.0) // We add the "whole" entry. + /// .finish() + /// } + /// } + /// + /// assert_eq!( + /// format!("{:?}", Foo(vec![("A".to_string(), 10), ("B".to_string(), 11)])), + /// "{\"whole\": [(\"A\", 10), (\"B\", 11)]}", + /// ); + /// ``` + #[unstable(feature = "debug_map_key_value", + reason = "recently added", + issue = "62482")] + pub fn value(&mut self, value: &dyn fmt::Debug) -> &mut DebugMap<'a, 'b> { + assert!(self.has_key, "attempted to format a map value before its key"); + + self.result = self.result.and_then(|_| { + if self.is_pretty() { + let mut slot = None; + let mut writer = PadAdapter::wrap(&mut self.fmt, &mut slot, &mut self.state); + value.fmt(&mut writer)?; + writer.write_str(",\n")?; + } else { + value.fmt(self.fmt)?; + } + + self.has_key = false; + Ok(()) }); self.has_fields = true; @@ -775,6 +892,11 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> { /// Finishes output and returns any error encountered. /// + /// # Panics + /// + /// `key` must be called before `value` and each call to `key` must be followed + /// by a corresponding call to `value`. Otherwise this method will panic. + /// /// # Examples /// /// ``` @@ -797,6 +919,8 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> { /// ``` #[stable(feature = "debug_builders", since = "1.2.0")] pub fn finish(&mut self) -> fmt::Result { + assert!(!self.has_key, "attempted to finish a map with a partial entry"); + self.result.and_then(|_| self.fmt.write_str("}")) } diff --git a/src/libcore/tests/fmt/builders.rs b/src/libcore/tests/fmt/builders.rs index 62fe09c5eb32c..200659b91bb4e 100644 --- a/src/libcore/tests/fmt/builders.rs +++ b/src/libcore/tests/fmt/builders.rs @@ -211,9 +211,9 @@ mod debug_map { #[test] fn test_single() { - struct Foo; + struct Entry; - impl fmt::Debug for Foo { + impl fmt::Debug for Entry { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_map() .entry(&"bar", &true) @@ -221,19 +221,32 @@ mod debug_map { } } - assert_eq!("{\"bar\": true}", format!("{:?}", Foo)); + struct KeyValue; + + impl fmt::Debug for KeyValue { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map() + .key(&"bar").value(&true) + .finish() + } + } + + assert_eq!(format!("{:?}", Entry), format!("{:?}", KeyValue)); + assert_eq!(format!("{:#?}", Entry), format!("{:#?}", KeyValue)); + + assert_eq!("{\"bar\": true}", format!("{:?}", Entry)); assert_eq!( "{ \"bar\": true, }", - format!("{:#?}", Foo)); + format!("{:#?}", Entry)); } #[test] fn test_multiple() { - struct Foo; + struct Entry; - impl fmt::Debug for Foo { + impl fmt::Debug for Entry { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { fmt.debug_map() .entry(&"bar", &true) @@ -242,13 +255,27 @@ mod debug_map { } } - assert_eq!("{\"bar\": true, 10: 10/20}", format!("{:?}", Foo)); + struct KeyValue; + + impl fmt::Debug for KeyValue { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map() + .key(&"bar").value(&true) + .key(&10).value(&format_args!("{}/{}", 10, 20)) + .finish() + } + } + + assert_eq!(format!("{:?}", Entry), format!("{:?}", KeyValue)); + assert_eq!(format!("{:#?}", Entry), format!("{:#?}", KeyValue)); + + assert_eq!("{\"bar\": true, 10: 10/20}", format!("{:?}", Entry)); assert_eq!( "{ \"bar\": true, 10: 10/20, }", - format!("{:#?}", Foo)); + format!("{:#?}", Entry)); } #[test] @@ -291,6 +318,56 @@ mod debug_map { }", format!("{:#?}", Bar)); } + + #[test] + #[should_panic] + fn test_invalid_key_when_entry_is_incomplete() { + struct Foo; + + impl fmt::Debug for Foo { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map() + .key(&"bar") + .key(&"invalid") + .finish() + } + } + + format!("{:?}", Foo); + } + + #[test] + #[should_panic] + fn test_invalid_finish_incomplete_entry() { + struct Foo; + + impl fmt::Debug for Foo { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map() + .key(&"bar") + .finish() + } + } + + format!("{:?}", Foo); + } + + #[test] + #[should_panic] + fn test_invalid_value_before_key() { + struct Foo; + + impl fmt::Debug for Foo { + fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt.debug_map() + .value(&"invalid") + .key(&"bar") + .finish() + } + } + + format!("{:?}", Foo); + } } mod debug_set { diff --git a/src/libcore/tests/lib.rs b/src/libcore/tests/lib.rs index bf072a9243b51..4b48d1225902b 100644 --- a/src/libcore/tests/lib.rs +++ b/src/libcore/tests/lib.rs @@ -3,6 +3,7 @@ #![feature(cell_update)] #![feature(core_private_bignum)] #![feature(core_private_diy_float)] +#![feature(debug_map_key_value)] #![feature(dec2flt)] #![feature(euclidean_division)] #![feature(exact_size_is_empty)] From b06ed52cfd57a971bf71bb8a4dab1a134cd041a3 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Tue, 9 Jul 2019 00:17:42 +0900 Subject: [PATCH 4/5] Remove unused dependencies --- Cargo.lock | 3 --- src/librustc_codegen_ssa/Cargo.toml | 1 - src/librustc_driver/Cargo.toml | 1 - src/librustc_interface/Cargo.toml | 1 - 4 files changed, 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6b5499ec6f351..f2ef5f5aacab6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2985,7 +2985,6 @@ dependencies = [ "num_cpus 1.8.0 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.7.1 (registry+https://github.com/rust-lang/crates.io-index)", "rustc 0.0.0", - "rustc-demangle 0.1.15 (registry+https://github.com/rust-lang/crates.io-index)", "rustc_allocator 0.0.0", "rustc_apfloat 0.0.0", "rustc_codegen_utils 0.0.0", @@ -3064,7 +3063,6 @@ dependencies = [ "rustc_target 0.0.0", "rustc_traits 0.0.0", "rustc_typeck 0.0.0", - "scoped-tls 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "serialize 0.0.0", "smallvec 0.6.10 (registry+https://github.com/rust-lang/crates.io-index)", "syntax 0.0.0", @@ -3128,7 +3126,6 @@ dependencies = [ "rustc_resolve 0.0.0", "rustc_traits 0.0.0", "rustc_typeck 0.0.0", - "scoped-tls 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)", "serialize 0.0.0", "smallvec 0.6.10 (registry+https://github.com/rust-lang/crates.io-index)", "syntax 0.0.0", diff --git a/src/librustc_codegen_ssa/Cargo.toml b/src/librustc_codegen_ssa/Cargo.toml index 343596feed25f..e7ee06df7e12d 100644 --- a/src/librustc_codegen_ssa/Cargo.toml +++ b/src/librustc_codegen_ssa/Cargo.toml @@ -13,7 +13,6 @@ test = false bitflags = "1.0.4" cc = "1.0.1" num_cpus = "1.0" -rustc-demangle = "0.1.15" memmap = "0.6" log = "0.4.5" libc = "0.2.44" diff --git a/src/librustc_driver/Cargo.toml b/src/librustc_driver/Cargo.toml index b28b015db7573..9a8473e1409d1 100644 --- a/src/librustc_driver/Cargo.toml +++ b/src/librustc_driver/Cargo.toml @@ -15,7 +15,6 @@ graphviz = { path = "../libgraphviz" } log = "0.4" env_logger = { version = "0.5", default-features = false } rayon = { version = "0.2.0", package = "rustc-rayon" } -scoped-tls = "1.0" rustc = { path = "../librustc" } rustc_allocator = { path = "../librustc_allocator" } rustc_target = { path = "../librustc_target" } diff --git a/src/librustc_interface/Cargo.toml b/src/librustc_interface/Cargo.toml index 82880d2198712..2712355d5379b 100644 --- a/src/librustc_interface/Cargo.toml +++ b/src/librustc_interface/Cargo.toml @@ -13,7 +13,6 @@ doctest = false log = "0.4" rayon = { version = "0.2.0", package = "rustc-rayon" } smallvec = { version = "0.6.7", features = ["union", "may_dangle"] } -scoped-tls = "1.0" syntax = { path = "../libsyntax" } syntax_ext = { path = "../libsyntax_ext" } syntax_pos = { path = "../libsyntax_pos" } From 70d630fd5a57d436b6a5064840b69920b10a110c Mon Sep 17 00:00:00 2001 From: Ashley Mannix Date: Tue, 9 Jul 2019 08:30:20 +1000 Subject: [PATCH 5/5] add feature to docs --- src/libcore/fmt/builders.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libcore/fmt/builders.rs b/src/libcore/fmt/builders.rs index abc97aacbb95a..cb4e32622ff1f 100644 --- a/src/libcore/fmt/builders.rs +++ b/src/libcore/fmt/builders.rs @@ -750,6 +750,7 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> { /// # Examples /// /// ``` + /// # #![feature(debug_map_key_value)] /// use std::fmt; /// /// struct Foo(Vec<(String, i32)>); @@ -813,6 +814,7 @@ impl<'a, 'b: 'a> DebugMap<'a, 'b> { /// # Examples /// /// ``` + /// # #![feature(debug_map_key_value)] /// use std::fmt; /// /// struct Foo(Vec<(String, i32)>);