From b2b8cba3e625bf0800e2de29cba06a472521b0a4 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 7 Aug 2017 20:50:34 +0000 Subject: make `for_all_relevant_impls` O(1) again A change in #41911 had made `for_all_relevant_impls` do a linear scan over all impls, instead of using an HashMap. Use an HashMap again to avoid quadratic blowup when there is a large number of structs with impls. I think this fixes #43141 completely, but I want better measurements in order to be sure. As a perf patch, please don't roll this up. --- src/librustc/traits/error_reporting.rs | 9 +- src/librustc/traits/select.rs | 6 +- src/librustc/traits/specialize/mod.rs | 3 +- src/librustc/ty/maps.rs | 15 +-- src/librustc/ty/mod.rs | 2 - src/librustc/ty/trait_def.rs | 168 ++++++++++----------------- src/librustc/ty/util.rs | 2 +- src/librustc_lint/builtin.rs | 3 +- src/librustc_mir/transform/qualify_consts.rs | 3 +- src/librustc_typeck/check/method/probe.rs | 4 +- 10 files changed, 77 insertions(+), 138 deletions(-) diff --git a/src/librustc/traits/error_reporting.rs b/src/librustc/traits/error_reporting.rs index c02d1394f6b..65c0a9f8ffd 100644 --- a/src/librustc/traits/error_reporting.rs +++ b/src/librustc/traits/error_reporting.rs @@ -278,8 +278,8 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { let mut self_match_impls = vec![]; let mut fuzzy_match_impls = vec![]; - self.tcx.trait_def(trait_ref.def_id) - .for_each_relevant_impl(self.tcx, trait_self_ty, |def_id| { + self.tcx.for_each_relevant_impl( + trait_ref.def_id, trait_self_ty, |def_id| { let impl_substs = self.fresh_substs_for_item(obligation.cause.span, def_id); let impl_trait_ref = tcx .impl_trait_ref(def_id) @@ -396,10 +396,9 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { trait_ref.skip_binder().self_ty(), true); let mut impl_candidates = Vec::new(); - let trait_def = self.tcx.trait_def(trait_ref.def_id()); match simp { - Some(simp) => trait_def.for_each_impl(self.tcx, |def_id| { + Some(simp) => self.tcx.for_each_impl(trait_ref.def_id(), |def_id| { let imp = self.tcx.impl_trait_ref(def_id).unwrap(); let imp_simp = fast_reject::simplify_type(self.tcx, imp.self_ty(), @@ -411,7 +410,7 @@ impl<'a, 'gcx, 'tcx> InferCtxt<'a, 'gcx, 'tcx> { } impl_candidates.push(imp); }), - None => trait_def.for_each_impl(self.tcx, |def_id| { + None => self.tcx.for_each_impl(trait_ref.def_id(), |def_id| { impl_candidates.push( self.tcx.impl_trait_ref(def_id).unwrap()); }) diff --git a/src/librustc/traits/select.rs b/src/librustc/traits/select.rs index 884f6bdb70b..f046cad82f3 100644 --- a/src/librustc/traits/select.rs +++ b/src/librustc/traits/select.rs @@ -1430,10 +1430,8 @@ impl<'cx, 'gcx, 'tcx> SelectionContext<'cx, 'gcx, 'tcx> { { debug!("assemble_candidates_from_impls(obligation={:?})", obligation); - let def = self.tcx().trait_def(obligation.predicate.def_id()); - - def.for_each_relevant_impl( - self.tcx(), + self.tcx().for_each_relevant_impl( + obligation.predicate.def_id(), obligation.predicate.0.trait_ref.self_ty(), |impl_def_id| { self.probe(|this, snapshot| { /* [1] */ diff --git a/src/librustc/traits/specialize/mod.rs b/src/librustc/traits/specialize/mod.rs index 18734e2dbc3..36b2027a3d3 100644 --- a/src/librustc/traits/specialize/mod.rs +++ b/src/librustc/traits/specialize/mod.rs @@ -300,7 +300,8 @@ pub(super) fn specialization_graph_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx -> Rc { let mut sg = specialization_graph::Graph::new(); - let mut trait_impls: Vec = tcx.trait_impls_of(trait_id).iter().collect(); + let mut trait_impls = Vec::new(); + tcx.for_each_impl(trait_id, |impl_did| trait_impls.push(impl_did)); // The coherence checking implementation seems to rely on impls being // iterated over (roughly) in definition order, so we are sorting by diff --git a/src/librustc/ty/maps.rs b/src/librustc/ty/maps.rs index e94308f3510..e803010383e 100644 --- a/src/librustc/ty/maps.rs +++ b/src/librustc/ty/maps.rs @@ -470,12 +470,6 @@ impl<'tcx> QueryDescription for queries::trait_impls_of<'tcx> { } } -impl<'tcx> QueryDescription for queries::relevant_trait_impls_for<'tcx> { - fn describe(tcx: TyCtxt, (def_id, ty): (DefId, SimplifiedType)) -> String { - format!("relevant impls for: `({}, {:?})`", tcx.item_path_str(def_id), ty) - } -} - impl<'tcx> QueryDescription for queries::is_object_safe<'tcx> { fn describe(tcx: TyCtxt, def_id: DefId) -> String { format!("determine object safety of trait `{}`", tcx.item_path_str(def_id)) @@ -966,10 +960,7 @@ define_maps! { <'tcx> [] const_is_rvalue_promotable_to_static: ConstIsRvaluePromotableToStatic(DefId) -> bool, [] is_mir_available: IsMirAvailable(DefId) -> bool, - [] trait_impls_of: TraitImpls(DefId) -> ty::trait_def::TraitImpls, - // Note that TraitDef::for_each_relevant_impl() will do type simplication for you. - [] relevant_trait_impls_for: relevant_trait_impls_for((DefId, SimplifiedType)) - -> ty::trait_def::TraitImpls, + [] trait_impls_of: TraitImpls(DefId) -> Rc, [] specialization_graph_of: SpecializationGraph(DefId) -> Rc, [] is_object_safe: ObjectSafety(DefId) -> bool, @@ -1045,10 +1036,6 @@ fn crate_variances<'tcx>(_: CrateNum) -> DepConstructor<'tcx> { DepConstructor::CrateVariances } -fn relevant_trait_impls_for<'tcx>((def_id, t): (DefId, SimplifiedType)) -> DepConstructor<'tcx> { - DepConstructor::RelevantTraitImpls(def_id, t) -} - fn is_copy_dep_node<'tcx>(key: ty::ParamEnvAnd<'tcx, Ty<'tcx>>) -> DepConstructor<'tcx> { let def_id = ty::item_path::characteristic_def_id_of_type(key.value) .unwrap_or(DefId::local(CRATE_DEF_INDEX)); diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 3f1302e72e9..8bd05f5d707 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -2530,7 +2530,6 @@ pub fn provide(providers: &mut ty::maps::Providers) { param_env, trait_of_item, trait_impls_of: trait_def::trait_impls_of_provider, - relevant_trait_impls_for: trait_def::relevant_trait_impls_provider, ..*providers }; } @@ -2540,7 +2539,6 @@ pub fn provide_extern(providers: &mut ty::maps::Providers) { adt_sized_constraint, adt_dtorck_constraint, trait_impls_of: trait_def::trait_impls_of_provider, - relevant_trait_impls_for: trait_def::relevant_trait_impls_provider, param_env, ..*providers }; diff --git a/src/librustc/ty/trait_def.rs b/src/librustc/ty/trait_def.rs index ef6bce8a3d9..9990472c6b4 100644 --- a/src/librustc/ty/trait_def.rs +++ b/src/librustc/ty/trait_def.rs @@ -8,14 +8,17 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use hir; use hir::def_id::DefId; use hir::map::DefPathHash; use traits::specialization_graph; use ty::fast_reject; use ty::fold::TypeFoldable; use ty::{Ty, TyCtxt}; + +use rustc_data_structures::fx::FxHashMap; + use std::rc::Rc; -use hir; /// A trait's definition with type information. pub struct TraitDef { @@ -36,60 +39,12 @@ pub struct TraitDef { pub def_path_hash: DefPathHash, } -// We don't store the list of impls in a flat list because each cached list of -// `relevant_impls_for` we would then duplicate all blanket impls. By keeping -// blanket and non-blanket impls separate, we can share the list of blanket -// impls. -#[derive(Clone)] pub struct TraitImpls { - blanket_impls: Rc>, - non_blanket_impls: Rc>, + blanket_impls: Vec, + /// Impls indexed by their simplified self-type, for fast lookup. + non_blanket_impls: FxHashMap>, } -impl TraitImpls { - pub fn iter(&self) -> TraitImplsIter { - TraitImplsIter { - blanket_impls: self.blanket_impls.clone(), - non_blanket_impls: self.non_blanket_impls.clone(), - index: 0 - } - } -} - -#[derive(Clone)] -pub struct TraitImplsIter { - blanket_impls: Rc>, - non_blanket_impls: Rc>, - index: usize, -} - -impl Iterator for TraitImplsIter { - type Item = DefId; - - fn next(&mut self) -> Option { - if self.index < self.blanket_impls.len() { - let bi_index = self.index; - self.index += 1; - Some(self.blanket_impls[bi_index]) - } else { - let nbi_index = self.index - self.blanket_impls.len(); - if nbi_index < self.non_blanket_impls.len() { - self.index += 1; - Some(self.non_blanket_impls[nbi_index]) - } else { - None - } - } - } - - fn size_hint(&self) -> (usize, Option) { - let items_left = (self.blanket_impls.len() + self.non_blanket_impls.len()) - self.index; - (items_left, Some(items_left)) - } -} - -impl ExactSizeIterator for TraitImplsIter {} - impl<'a, 'gcx, 'tcx> TraitDef { pub fn new(def_id: DefId, unsafety: hir::Unsafety, @@ -111,20 +66,36 @@ impl<'a, 'gcx, 'tcx> TraitDef { -> specialization_graph::Ancestors { specialization_graph::ancestors(tcx, self.def_id, of_impl) } +} - pub fn for_each_impl(&self, tcx: TyCtxt<'a, 'gcx, 'tcx>, mut f: F) { - for impl_def_id in tcx.trait_impls_of(self.def_id).iter() { +impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { + pub fn for_each_impl(self, def_id: DefId, mut f: F) { + let impls = self.trait_impls_of(def_id); + + for &impl_def_id in impls.blanket_impls.iter() { f(impl_def_id); } + + for v in impls.non_blanket_impls.values() { + for &impl_def_id in v { + f(impl_def_id); + } + } } /// Iterate over every impl that could possibly match the /// self-type `self_ty`. - pub fn for_each_relevant_impl(&self, - tcx: TyCtxt<'a, 'gcx, 'tcx>, + pub fn for_each_relevant_impl(self, + def_id: DefId, self_ty: Ty<'tcx>, mut f: F) { + let impls = self.trait_impls_of(def_id); + + for &impl_def_id in impls.blanket_impls.iter() { + f(impl_def_id); + } + // simplify_type(.., false) basically replaces type parameters and // projections with infer-variables. This is, of course, done on // the impl trait-ref when it is instantiated, but not on the @@ -137,15 +108,31 @@ impl<'a, 'gcx, 'tcx> TraitDef { // replace `S` with anything - this impl of course can't be // selected, and as there are hundreds of similar impls, // considering them would significantly harm performance. - let relevant_impls = if let Some(simplified_self_ty) = - fast_reject::simplify_type(tcx, self_ty, true) { - tcx.relevant_trait_impls_for((self.def_id, simplified_self_ty)) - } else { - tcx.trait_impls_of(self.def_id) - }; - for impl_def_id in relevant_impls.iter() { - f(impl_def_id); + // This depends on the set of all impls for the trait. That is + // unfortunate. When we get red-green recompilation, we would like + // to have a way of knowing whether the set of relevant impls + // changed. The most naive + // way would be to compute the Vec of relevant impls and see whether + // it differs between compilations. That shouldn't be too slow by + // itself - we do quite a bit of work for each relevant impl anyway. + // + // If we want to be faster, we could have separate queries for + // blanket and non-blanket impls, and compare them separately. + // + // I think we'll cross that bridge when we get to it. + if let Some(simp) = fast_reject::simplify_type(self, self_ty, true) { + if let Some(impls) = impls.non_blanket_impls.get(&simp) { + for &impl_def_id in impls { + f(impl_def_id); + } + } + } else { + for v in impls.non_blanket_impls.values() { + for &impl_def_id in v { + f(impl_def_id); + } + } } } } @@ -153,7 +140,7 @@ impl<'a, 'gcx, 'tcx> TraitDef { // Query provider for `trait_impls_of`. pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, trait_id: DefId) - -> TraitImpls { + -> Rc { let remote_impls = if trait_id.is_local() { // Traits defined in the current crate can't have impls in upstream // crates, so we don't bother querying the cstore. @@ -163,7 +150,7 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, }; let mut blanket_impls = Vec::new(); - let mut non_blanket_impls = Vec::new(); + let mut non_blanket_impls = FxHashMap(); let local_impls = tcx.hir .trait_impls(trait_id) @@ -176,47 +163,20 @@ pub(super) fn trait_impls_of_provider<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, continue } - if fast_reject::simplify_type(tcx, impl_self_ty, false).is_some() { - non_blanket_impls.push(impl_def_id); + if let Some(simplified_self_ty) = + fast_reject::simplify_type(tcx, impl_self_ty, false) + { + non_blanket_impls + .entry(simplified_self_ty) + .or_insert(vec![]) + .push(impl_def_id); } else { blanket_impls.push(impl_def_id); } } - TraitImpls { - blanket_impls: Rc::new(blanket_impls), - non_blanket_impls: Rc::new(non_blanket_impls), - } -} - -// Query provider for `relevant_trait_impls_for`. -pub(super) fn relevant_trait_impls_provider<'a, 'tcx>( - tcx: TyCtxt<'a, 'tcx, 'tcx>, - (trait_id, self_ty): (DefId, fast_reject::SimplifiedType)) - -> TraitImpls -{ - let all_trait_impls = tcx.trait_impls_of(trait_id); - - let relevant: Vec = all_trait_impls - .non_blanket_impls - .iter() - .cloned() - .filter(|&impl_def_id| { - let impl_self_ty = tcx.type_of(impl_def_id); - let impl_simple_self_ty = fast_reject::simplify_type(tcx, - impl_self_ty, - false).unwrap(); - impl_simple_self_ty == self_ty - }) - .collect(); - - if all_trait_impls.non_blanket_impls.len() == relevant.len() { - // If we didn't filter anything out, re-use the existing vec. - all_trait_impls - } else { - TraitImpls { - blanket_impls: all_trait_impls.blanket_impls.clone(), - non_blanket_impls: Rc::new(relevant), - } - } + Rc::new(TraitImpls { + blanket_impls: blanket_impls, + non_blanket_impls: non_blanket_impls, + }) } diff --git a/src/librustc/ty/util.rs b/src/librustc/ty/util.rs index df4bbad3859..d1c73831545 100644 --- a/src/librustc/ty/util.rs +++ b/src/librustc/ty/util.rs @@ -422,7 +422,7 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { let mut dtor_did = None; let ty = self.type_of(adt_did); - self.trait_def(drop_trait).for_each_relevant_impl(self, ty, |impl_did| { + self.for_each_relevant_impl(drop_trait, ty, |impl_did| { if let Some(item) = self.associated_items(impl_did).next() { if let Ok(()) = validate(self, impl_did) { dtor_did = Some(item.def_id); diff --git a/src/librustc_lint/builtin.rs b/src/librustc_lint/builtin.rs index 02d68a41b4c..6726a6fa1ff 100644 --- a/src/librustc_lint/builtin.rs +++ b/src/librustc_lint/builtin.rs @@ -542,9 +542,8 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MissingDebugImplementations { }; if self.impling_types.is_none() { - let debug_def = cx.tcx.trait_def(debug); let mut impls = NodeSet(); - debug_def.for_each_impl(cx.tcx, |d| { + cx.tcx.for_each_impl(debug, |d| { if let Some(ty_def) = cx.tcx.type_of(d).ty_to_def_id() { if let Some(node_id) = cx.tcx.hir.as_local_node_id(ty_def) { impls.insert(node_id); diff --git a/src/librustc_mir/transform/qualify_consts.rs b/src/librustc_mir/transform/qualify_consts.rs index 68b687a2e61..502c64a209b 100644 --- a/src/librustc_mir/transform/qualify_consts.rs +++ b/src/librustc_mir/transform/qualify_consts.rs @@ -244,8 +244,7 @@ impl<'a, 'tcx> Qualifier<'a, 'tcx, 'tcx> { let mut span = None; self.tcx - .trait_def(drop_trait_id) - .for_each_relevant_impl(self.tcx, self.mir.return_ty, |impl_did| { + .for_each_relevant_impl(drop_trait_id, self.mir.return_ty, |impl_did| { self.tcx.hir .as_local_node_id(impl_did) .and_then(|impl_node_id| self.tcx.hir.find(impl_node_id)) diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index dfc5cd00b6e..558cb1add9c 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -718,10 +718,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { import_id: Option, trait_def_id: DefId, item: ty::AssociatedItem) { - let trait_def = self.tcx.trait_def(trait_def_id); - // FIXME(arielb1): can we use for_each_relevant_impl here? - trait_def.for_each_impl(self.tcx, |impl_def_id| { + self.tcx.for_each_impl(trait_def_id, |impl_def_id| { debug!("assemble_extension_candidates_for_trait_impl: trait_def_id={:?} \ impl_def_id={:?}", trait_def_id, -- cgit 1.4.1-3-g733a5 From f0cd69592b49821f2f1a1b9f5cf5416ef120b6a9 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 12 Aug 2017 15:31:37 -0700 Subject: std: Respect formatting flags for str-like OsStr Historically many `Display` and `Debug` implementations for `OsStr`-like abstractions have gone through `String::from_utf8_lossy`, but this was updated in #42613 to use an internal `Utf8Lossy` abstraction instead. This had the unfortunate side effect of causing a regression (#43765) in code which relied on these `fmt` trait implementations respecting the various formatting flags specified. This commit opportunistically adds back interpretation of formatting trait flags in the "common case" where where `OsStr`-like "thing" is all valid utf-8 and can delegate to the formatting implementation for `str`. This doesn't entirely solve the regression as non-utf8 paths will format differently than they did before still (in that they will not respect formatting flags), but this should solve the regression for all "real world" use cases of paths and such. The door's also still open for handling these flags in the future! Closes #43765 --- src/libstd/path.rs | 6 ++++++ src/libstd/sys_common/wtf8.rs | 10 +++++++--- src/libstd_unicode/lib.rs | 1 - src/libstd_unicode/lossy.rs | 18 ++++++++++++++++-- 4 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/libstd/path.rs b/src/libstd/path.rs index a1a1825ffbf..62ffe746f3b 100644 --- a/src/libstd/path.rs +++ b/src/libstd/path.rs @@ -3953,4 +3953,10 @@ mod tests { assert_eq!(path, path_buf); assert!(path_buf.into_os_string().capacity() >= 15); } + + #[test] + fn display_format_flags() { + assert_eq!(format!("a{:#<5}b", Path::new("").display()), "a#####b"); + assert_eq!(format!("a{:#<5}b", Path::new("a").display()), "aa####b"); + } } diff --git a/src/libstd/sys_common/wtf8.rs b/src/libstd/sys_common/wtf8.rs index 4e4a6e77d12..b89a73cd28a 100644 --- a/src/libstd/sys_common/wtf8.rs +++ b/src/libstd/sys_common/wtf8.rs @@ -452,10 +452,14 @@ impl fmt::Display for Wtf8 { pos = surrogate_pos + 3; }, None => { - formatter.write_str(unsafe { + let s = unsafe { str::from_utf8_unchecked(&wtf8_bytes[pos..]) - })?; - return Ok(()); + }; + if pos == 0 { + return s.fmt(formatter) + } else { + return formatter.write_str(s) + } } } } diff --git a/src/libstd_unicode/lib.rs b/src/libstd_unicode/lib.rs index 698210e83f3..d568baa2cd7 100644 --- a/src/libstd_unicode/lib.rs +++ b/src/libstd_unicode/lib.rs @@ -34,7 +34,6 @@ #![feature(core_char_ext)] #![feature(str_internals)] -#![feature(core_intrinsics)] #![feature(decode_utf8)] #![feature(fused)] #![feature(fn_traits)] diff --git a/src/libstd_unicode/lossy.rs b/src/libstd_unicode/lossy.rs index b914cbcf4b8..253dcb6a159 100644 --- a/src/libstd_unicode/lossy.rs +++ b/src/libstd_unicode/lossy.rs @@ -12,7 +12,7 @@ use core::str as core_str; use core::fmt; use core::fmt::Write; use char; -use core::intrinsics; +use core::mem; /// Lossy UTF-8 string. @@ -27,7 +27,7 @@ impl Utf8Lossy { } pub fn from_bytes(bytes: &[u8]) -> &Utf8Lossy { - unsafe { intrinsics::transmute(bytes) } + unsafe { mem::transmute(bytes) } } pub fn chunks(&self) -> Utf8LossyChunksIter { @@ -153,7 +153,21 @@ impl<'a> Iterator for Utf8LossyChunksIter<'a> { impl fmt::Display for Utf8Lossy { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + // If we're the empty string then our iterator won't actually yield + // anything, so perform the formatting manually + if self.bytes.len() == 0 { + return "".fmt(f) + } + for Utf8LossyChunk { valid, broken } in self.chunks() { + // If we successfully decoded the whole chunk as a valid string then + // we can return a direct formatting of the string which will also + // respect various formatting flags if possible. + if valid.len() == self.bytes.len() { + assert!(broken.is_empty()); + return valid.fmt(f) + } + f.write_str(valid)?; if !broken.is_empty() { f.write_char(char::REPLACEMENT_CHARACTER)?; -- cgit 1.4.1-3-g733a5 From 5bb0cac84891de785ce803ca2bd36a55cd9775d9 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Sun, 13 Aug 2017 16:59:54 +0300 Subject: ast_validation: forbid "nonstandard" literal patterns Since #42886, macros can create "nonstandard" PatKind::Lit patterns, that contain path expressions instead of the usual literal expr. These can cause trouble, including ICEs. We *could* map these nonstandard patterns to PatKind::Path patterns during HIR lowering, but that would be much effort for little gain, and I think is too risky for beta. So let's just forbid them during AST validation. Fixes #43250. --- src/librustc_passes/ast_validation.rs | 27 +++++++++++++++++++++------ src/test/compile-fail/issue-43250.rs | 23 +++++++++++++++++++++++ 2 files changed, 44 insertions(+), 6 deletions(-) create mode 100644 src/test/compile-fail/issue-43250.rs diff --git a/src/librustc_passes/ast_validation.rs b/src/librustc_passes/ast_validation.rs index 72c7b92fe6e..66736507abc 100644 --- a/src/librustc_passes/ast_validation.rs +++ b/src/librustc_passes/ast_validation.rs @@ -94,10 +94,25 @@ impl<'a> AstValidator<'a> { } } - /// matches '-' lit | lit (cf. parser::Parser::parse_pat_literal_maybe_minus) - fn check_expr_within_pat(&self, expr: &Expr) { + /// matches '-' lit | lit (cf. parser::Parser::parse_pat_literal_maybe_minus), + /// or path for ranges. + /// + /// FIXME: do we want to allow expr -> pattern conversion to create path expressions? + /// That means making this work: + /// + /// ```rust,ignore (FIXME) + /// struct S; + /// macro_rules! m { + /// ($a:expr) => { + /// let $a = S; + /// } + /// } + /// m!(S); + /// ``` + fn check_expr_within_pat(&self, expr: &Expr, allow_paths: bool) { match expr.node { - ExprKind::Lit(..) | ExprKind::Path(..) => {} + ExprKind::Lit(..) => {} + ExprKind::Path(..) if allow_paths => {} ExprKind::Unary(UnOp::Neg, ref inner) if match inner.node { ExprKind::Lit(_) => true, _ => false } => {} _ => self.err_handler().span_err(expr.span, "arbitrary expressions aren't allowed \ @@ -340,11 +355,11 @@ impl<'a> Visitor<'a> for AstValidator<'a> { fn visit_pat(&mut self, pat: &'a Pat) { match pat.node { PatKind::Lit(ref expr) => { - self.check_expr_within_pat(expr); + self.check_expr_within_pat(expr, false); } PatKind::Range(ref start, ref end, _) => { - self.check_expr_within_pat(start); - self.check_expr_within_pat(end); + self.check_expr_within_pat(start, true); + self.check_expr_within_pat(end, true); } _ => {} } diff --git a/src/test/compile-fail/issue-43250.rs b/src/test/compile-fail/issue-43250.rs new file mode 100644 index 00000000000..e1d34f339dc --- /dev/null +++ b/src/test/compile-fail/issue-43250.rs @@ -0,0 +1,23 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + let mut y; + const C: u32 = 0; + macro_rules! m { + ($a:expr) => { + let $a = 0; + } + } + m!(y); + //~^ ERROR arbitrary expressions aren't allowed in patterns + m!(C); + //~^ ERROR arbitrary expressions aren't allowed in patterns +} -- cgit 1.4.1-3-g733a5 From bd20a035e46739c64ccd616ce8ad9926a86a269f Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 21 Aug 2017 14:11:57 +0300 Subject: register fn-ptr coercion obligations out of a snapshot Fixes #43923. --- src/librustc_typeck/check/coercion.rs | 3 +-- src/test/run-pass/issue-43923.rs | 19 +++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100644 src/test/run-pass/issue-43923.rs diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index 968e893b9a0..e4dea7d7983 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -807,8 +807,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let lub_ty = self.commit_if_ok(|_| { self.at(cause, self.param_env) .lub(prev_ty, new_ty) - .map(|ok| self.register_infer_ok_obligations(ok)) - }); + }).map(|ok| self.register_infer_ok_obligations(ok)); if lub_ty.is_ok() { // We have a LUB of prev_ty and new_ty, just return it. diff --git a/src/test/run-pass/issue-43923.rs b/src/test/run-pass/issue-43923.rs new file mode 100644 index 00000000000..e1992e4fc50 --- /dev/null +++ b/src/test/run-pass/issue-43923.rs @@ -0,0 +1,19 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +struct A { ptr: T } + +fn foo(x: &A<[T]>) {} + +fn main() { + let a = foo; + let b = A { ptr: [a, a, a] }; + a(&A { ptr: [()] }); +} -- cgit 1.4.1-3-g733a5 From 4b6a82150bf3093c9dab12038b3e749cb8a6d1fd Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Mon, 21 Aug 2017 14:26:33 +0300 Subject: fix other cases of registering obligations in a snapshot No test cases for these ones, but they would all ICE if they ever run with a non-empty set of obligations. --- src/librustc_typeck/check/coercion.rs | 6 ++---- src/librustc_typeck/check/regionck.rs | 10 +++++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index e4dea7d7983..f18d52ba40d 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -883,8 +883,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return self.commit_if_ok(|_| { self.at(cause, self.param_env) .lub(prev_ty, new_ty) - .map(|ok| self.register_infer_ok_obligations(ok)) - }); + }).map(|ok| self.register_infer_ok_obligations(ok)); } } @@ -897,8 +896,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { self.commit_if_ok(|_| { self.at(cause, self.param_env) .lub(prev_ty, new_ty) - .map(|ok| self.register_infer_ok_obligations(ok)) - }) + }).map(|ok| self.register_infer_ok_obligations(ok)) } } Ok(ok) => { diff --git a/src/librustc_typeck/check/regionck.rs b/src/librustc_typeck/check/regionck.rs index 82207428efc..3437aec892a 100644 --- a/src/librustc_typeck/check/regionck.rs +++ b/src/librustc_typeck/check/regionck.rs @@ -1805,12 +1805,12 @@ impl<'a, 'gcx, 'tcx> RegionCtxt<'a, 'gcx, 'tcx> { // check whether this predicate applies to our current projection let cause = self.fcx.misc(span); match self.at(&cause, self.fcx.param_env).eq(outlives.0, ty) { - Ok(ok) => { - self.register_infer_ok_obligations(ok); - Ok(outlives.1) - } - Err(_) => { Err(()) } + Ok(ok) => Ok((ok, outlives.1)), + Err(_) => Err(()) } + }).map(|(ok, result)| { + self.register_infer_ok_obligations(ok); + result }); debug!("projection_bounds: region_result={:?}", -- cgit 1.4.1-3-g733a5 From 76a1144cd8f7e195126ab20bc7cb218058b6d0a6 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 22 Aug 2017 14:36:49 -0700 Subject: std: Mark allocation functions as nounwind This commit flags all allocation-related functions in liballoc as "this can't unwind" which should largely resolve the size-related issues found on #42808. The documentation on the trait was updated with such a restriction (they can't panic) as well as some other words about the relative instability about implementing a bullet-proof allocator. Closes #42808 --- src/liballoc/allocator.rs | 21 +++++++++++++++++++++ src/liballoc/heap.rs | 11 +++++++++++ src/librustc_trans/attributes.rs | 2 ++ src/test/codegen/dealloc-no-unwind.rs | 32 ++++++++++++++++++++++++++++++++ 4 files changed, 66 insertions(+) create mode 100644 src/test/codegen/dealloc-no-unwind.rs diff --git a/src/liballoc/allocator.rs b/src/liballoc/allocator.rs index 23a005cbaf7..7c9c239560d 100644 --- a/src/liballoc/allocator.rs +++ b/src/liballoc/allocator.rs @@ -452,6 +452,27 @@ impl fmt::Display for CannotReallocInPlace { /// * if a layout `k` fits a memory block (denoted by `ptr`) /// currently allocated via an allocator `a`, then it is legal to /// use that layout to deallocate it, i.e. `a.dealloc(ptr, k);`. +/// +/// # Unsafety +/// +/// The `Alloc` trait is an `unsafe` trait for a number of reasons, and +/// implementors must ensure that they adhere to these contracts: +/// +/// * Pointers returned from allocation functions must point to valid memory and +/// retain their validity until at least the instance of `Alloc` is dropped +/// itself. +/// +/// * Implementations of `Alloc` are not currently memory safe if they unwind. +/// This restriction may be lifted in the future, but currently an panic from +/// any of these functions may lead to memory unsafety. +/// +/// * `Layout` queries and calculations in general must be correct. Callers of +/// this trait are allowed to rely on the contracts defined on each method, +/// and implementors must ensure such contracts remain true. +/// +/// Note that this list may get tweaked over time as clarifications are made in +/// the future. Additionally global allocators may gain unique requirements for +/// how to safely implement one in the future as well. pub unsafe trait Alloc { // (Note: existing allocators have unspecified but well-defined diff --git a/src/liballoc/heap.rs b/src/liballoc/heap.rs index 1d959ac5bf6..feab1feadfb 100644 --- a/src/liballoc/heap.rs +++ b/src/liballoc/heap.rs @@ -27,23 +27,32 @@ pub mod __core { extern "Rust" { #[allocator] + #[allocator_nounwind] fn __rust_alloc(size: usize, align: usize, err: *mut u8) -> *mut u8; + #[cold] + #[allocator_nounwind] fn __rust_oom(err: *const u8) -> !; + #[allocator_nounwind] fn __rust_dealloc(ptr: *mut u8, size: usize, align: usize); + #[allocator_nounwind] fn __rust_usable_size(layout: *const u8, min: *mut usize, max: *mut usize); + #[allocator_nounwind] fn __rust_realloc(ptr: *mut u8, old_size: usize, old_align: usize, new_size: usize, new_align: usize, err: *mut u8) -> *mut u8; + #[allocator_nounwind] fn __rust_alloc_zeroed(size: usize, align: usize, err: *mut u8) -> *mut u8; + #[allocator_nounwind] fn __rust_alloc_excess(size: usize, align: usize, excess: *mut usize, err: *mut u8) -> *mut u8; + #[allocator_nounwind] fn __rust_realloc_excess(ptr: *mut u8, old_size: usize, old_align: usize, @@ -51,11 +60,13 @@ extern "Rust" { new_align: usize, excess: *mut usize, err: *mut u8) -> *mut u8; + #[allocator_nounwind] fn __rust_grow_in_place(ptr: *mut u8, old_size: usize, old_align: usize, new_size: usize, new_align: usize) -> u8; + #[allocator_nounwind] fn __rust_shrink_in_place(ptr: *mut u8, old_size: usize, old_align: usize, diff --git a/src/librustc_trans/attributes.rs b/src/librustc_trans/attributes.rs index cbad43066e4..839614f4a29 100644 --- a/src/librustc_trans/attributes.rs +++ b/src/librustc_trans/attributes.rs @@ -119,6 +119,8 @@ pub fn from_fn_attrs(ccx: &CrateContext, attrs: &[ast::Attribute], llfn: ValueRe llvm::AttributePlace::ReturnValue(), llfn); } else if attr.check_name("unwind") { unwind(llfn, true); + } else if attr.check_name("allocator_nounwind") { + unwind(llfn, false); } } if !target_features.is_empty() { diff --git a/src/test/codegen/dealloc-no-unwind.rs b/src/test/codegen/dealloc-no-unwind.rs new file mode 100644 index 00000000000..551b66e103a --- /dev/null +++ b/src/test/codegen/dealloc-no-unwind.rs @@ -0,0 +1,32 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +// +// no-system-llvm +// compile-flags: -O + +#![crate_type="lib"] + +struct A; + +impl Drop for A { + fn drop(&mut self) { + extern { fn foo(); } + unsafe { foo(); } + } +} + +#[no_mangle] +pub fn a(a: Box) { + // CHECK-LABEL: define void @a + // CHECK: call void @__rust_dealloc + // CHECK-NEXT: call void @foo + let _a = A; + drop(a); +} -- cgit 1.4.1-3-g733a5 From c5beaac564f3a14b5a8390131706959d49499abc Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 23 Aug 2017 07:54:56 -0700 Subject: Bump to beta .3 --- src/bootstrap/channel.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/bootstrap/channel.rs b/src/bootstrap/channel.rs index aae02761708..16a7aa8b0ae 100644 --- a/src/bootstrap/channel.rs +++ b/src/bootstrap/channel.rs @@ -28,7 +28,7 @@ pub const CFG_RELEASE_NUM: &str = "1.20.0"; // An optional number to put after the label, e.g. '.2' -> '-beta.2' // Be sure to make this starts with a dot to conform to semver pre-release // versions (section 9) -pub const CFG_PRERELEASE_VERSION: &str = ".2"; +pub const CFG_PRERELEASE_VERSION: &str = ".3"; pub struct GitInfo { inner: Option, -- cgit 1.4.1-3-g733a5 From 7c076fc82a4197759044dd912c37c7fc9c33caf3 Mon Sep 17 00:00:00 2001 From: Jeffrey Seyfried Date: Thu, 17 Aug 2017 11:44:28 -0700 Subject: Ensure that generic arguments don't end up in attribute paths. --- src/libsyntax/parse/parser.rs | 8 +++++++- src/test/compile-fail/issue-43424.rs | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 src/test/compile-fail/issue-43424.rs diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 90d9ae383a4..1a1f35505d0 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1751,7 +1751,13 @@ impl<'a> Parser<'a> { fn parse_path_common(&mut self, mode: PathStyle, parse_generics: bool) -> PResult<'a, ast::Path> { - maybe_whole!(self, NtPath, |x| x); + maybe_whole!(self, NtPath, |path| { + if style == PathStyle::Mod && + path.segments.iter().any(|segment| segment.parameters.is_some()) { + self.diagnostic().span_err(path.span, "unexpected generic arguments in path"); + } + path + }); let lo = self.meta_var_span.unwrap_or(self.span); let is_global = self.eat(&token::ModSep); diff --git a/src/test/compile-fail/issue-43424.rs b/src/test/compile-fail/issue-43424.rs new file mode 100644 index 00000000000..431fc8a5aa2 --- /dev/null +++ b/src/test/compile-fail/issue-43424.rs @@ -0,0 +1,22 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +#![allow(unused)] + +macro_rules! m { + ($attr_path: path) => { + #[$attr_path] + fn f() {} + } +} + +m!(inline); //~ ERROR: unexpected generic arguments in path + +fn main() {} -- cgit 1.4.1-3-g733a5 From 0e9969c81c6df7b13b75638a7c3635d0a4515c36 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Wed, 23 Aug 2017 08:47:50 -0700 Subject: Fix tests and fallout --- src/libsyntax/parse/parser.rs | 2 +- src/test/compile-fail/import-ty-params.rs | 2 +- src/test/compile-fail/privacy/restricted/ty-params.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 1a1f35505d0..573e01eab2a 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -1752,7 +1752,7 @@ impl<'a> Parser<'a> { -> PResult<'a, ast::Path> { maybe_whole!(self, NtPath, |path| { - if style == PathStyle::Mod && + if mode == PathStyle::Mod && path.segments.iter().any(|segment| segment.parameters.is_some()) { self.diagnostic().span_err(path.span, "unexpected generic arguments in path"); } diff --git a/src/test/compile-fail/import-ty-params.rs b/src/test/compile-fail/import-ty-params.rs index 7344f31535f..a345523ded5 100644 --- a/src/test/compile-fail/import-ty-params.rs +++ b/src/test/compile-fail/import-ty-params.rs @@ -20,6 +20,6 @@ macro_rules! import { ($p: path) => (use $p;); } -import! { a::b::c::S } //~ERROR type or lifetime parameters in import path +import! { a::b::c::S } //~ERROR unexpected generic arguments in path fn main() {} diff --git a/src/test/compile-fail/privacy/restricted/ty-params.rs b/src/test/compile-fail/privacy/restricted/ty-params.rs index c83a4e56852..5708d319984 100644 --- a/src/test/compile-fail/privacy/restricted/ty-params.rs +++ b/src/test/compile-fail/privacy/restricted/ty-params.rs @@ -13,7 +13,7 @@ macro_rules! m { } struct S(T); -m!{ S } //~ ERROR type or lifetime parameters in visibility path +m!{ S } //~ ERROR unexpected generic arguments in path //~^ ERROR expected module, found struct `S` fn main() {} -- cgit 1.4.1-3-g733a5