From 7c4c96c095fc5aec6e30a3f349bcf9a9e4c4e8b9 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 15 Sep 2020 09:37:19 -0500 Subject: [PATCH 1/9] Only visit types once when walking the type tree This fixes #72408. Nested closures were resulting in exponential compilation time. As a performance optimization this change introduces MiniSet, which is a simple small storage optimized set. --- Cargo.lock | 1 + src/librustc_infer/infer/outlives/verify.rs | 40 +++++++--- src/librustc_middle/Cargo.toml | 1 + src/librustc_middle/ty/outlives.rs | 33 +++++--- src/librustc_middle/ty/walk.rs | 80 +++++++++++++++++-- ...issue-72408-nested-closures-exponential.rs | 59 ++++++++++++++ src/test/ui/issues/issue-22638.rs | 2 +- src/test/ui/issues/issue-22638.stderr | 12 ++- src/test/ui/type_length_limit.rs | 2 +- src/test/ui/type_length_limit.stderr | 2 +- 10 files changed, 193 insertions(+), 39 deletions(-) create mode 100644 src/test/ui/closures/issue-72408-nested-closures-exponential.rs diff --git a/Cargo.lock b/Cargo.lock index 1bbae2cbd80c9..eacc28a756765 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3738,6 +3738,7 @@ dependencies = [ name = "rustc_middle" version = "0.0.0" dependencies = [ + "arrayvec", "bitflags", "byteorder", "chalk-ir", diff --git a/src/librustc_infer/infer/outlives/verify.rs b/src/librustc_infer/infer/outlives/verify.rs index 8f20b5743df4f..5a8368700b7c4 100644 --- a/src/librustc_infer/infer/outlives/verify.rs +++ b/src/librustc_infer/infer/outlives/verify.rs @@ -3,6 +3,7 @@ use crate::infer::{GenericKind, VerifyBound}; use rustc_data_structures::captures::Captures; use rustc_hir::def_id::DefId; use rustc_middle::ty::subst::{GenericArg, GenericArgKind, Subst}; +use rustc_middle::ty::walk::MiniSet; use rustc_middle::ty::{self, Ty, TyCtxt}; /// The `TypeOutlives` struct has the job of "lowering" a `T: 'a` @@ -31,16 +32,23 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { /// Returns a "verify bound" that encodes what we know about /// `generic` and the regions it outlives. pub fn generic_bound(&self, generic: GenericKind<'tcx>) -> VerifyBound<'tcx> { + let mut visited = MiniSet::new(); match generic { GenericKind::Param(param_ty) => self.param_bound(param_ty), - GenericKind::Projection(projection_ty) => self.projection_bound(projection_ty), + GenericKind::Projection(projection_ty) => { + self.projection_bound(projection_ty, &mut visited) + } } } - fn type_bound(&self, ty: Ty<'tcx>) -> VerifyBound<'tcx> { + fn type_bound( + &self, + ty: Ty<'tcx>, + visited: &mut MiniSet>, + ) -> VerifyBound<'tcx> { match ty.kind { ty::Param(p) => self.param_bound(p), - ty::Projection(data) => self.projection_bound(data), + ty::Projection(data) => self.projection_bound(data, visited), ty::FnDef(_, substs) => { // HACK(eddyb) ignore lifetimes found shallowly in `substs`. // This is inconsistent with `ty::Adt` (including all substs), @@ -50,9 +58,9 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { let mut bounds = substs .iter() .filter_map(|child| match child.unpack() { - GenericArgKind::Type(ty) => Some(self.type_bound(ty)), + GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)), GenericArgKind::Lifetime(_) => None, - GenericArgKind::Const(_) => Some(self.recursive_bound(child)), + GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)), }) .filter(|bound| { // Remove bounds that must hold, since they are not interesting. @@ -66,7 +74,7 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { ), } } - _ => self.recursive_bound(ty.into()), + _ => self.recursive_bound(ty.into(), visited), } } @@ -137,7 +145,11 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { self.declared_projection_bounds_from_trait(projection_ty) } - pub fn projection_bound(&self, projection_ty: ty::ProjectionTy<'tcx>) -> VerifyBound<'tcx> { + pub fn projection_bound( + &self, + projection_ty: ty::ProjectionTy<'tcx>, + visited: &mut MiniSet>, + ) -> VerifyBound<'tcx> { debug!("projection_bound(projection_ty={:?})", projection_ty); let projection_ty_as_ty = @@ -166,21 +178,25 @@ impl<'cx, 'tcx> VerifyBoundCx<'cx, 'tcx> { // see the extensive comment in projection_must_outlive let ty = self.tcx.mk_projection(projection_ty.item_def_id, projection_ty.substs); - let recursive_bound = self.recursive_bound(ty.into()); + let recursive_bound = self.recursive_bound(ty.into(), visited); VerifyBound::AnyBound(env_bounds.chain(trait_bounds).collect()).or(recursive_bound) } - fn recursive_bound(&self, parent: GenericArg<'tcx>) -> VerifyBound<'tcx> { + fn recursive_bound( + &self, + parent: GenericArg<'tcx>, + visited: &mut MiniSet>, + ) -> VerifyBound<'tcx> { let mut bounds = parent - .walk_shallow() + .walk_shallow(visited) .filter_map(|child| match child.unpack() { - GenericArgKind::Type(ty) => Some(self.type_bound(ty)), + GenericArgKind::Type(ty) => Some(self.type_bound(ty, visited)), GenericArgKind::Lifetime(lt) => { // Ignore late-bound regions. if !lt.is_late_bound() { Some(VerifyBound::OutlivedBy(lt)) } else { None } } - GenericArgKind::Const(_) => Some(self.recursive_bound(child)), + GenericArgKind::Const(_) => Some(self.recursive_bound(child, visited)), }) .filter(|bound| { // Remove bounds that must hold, since they are not interesting. diff --git a/src/librustc_middle/Cargo.toml b/src/librustc_middle/Cargo.toml index 311126361bc5b..ae09420d4a303 100644 --- a/src/librustc_middle/Cargo.toml +++ b/src/librustc_middle/Cargo.toml @@ -31,5 +31,6 @@ rustc_span = { path = "../librustc_span" } byteorder = { version = "1.3" } chalk-ir = "0.14.0" smallvec = { version = "1.0", features = ["union", "may_dangle"] } +arrayvec = { version = "0.5.1", default-features = false } measureme = "0.7.1" rustc_session = { path = "../librustc_session" } diff --git a/src/librustc_middle/ty/outlives.rs b/src/librustc_middle/ty/outlives.rs index 1a8693b8df711..07a0bcc0c4cf9 100644 --- a/src/librustc_middle/ty/outlives.rs +++ b/src/librustc_middle/ty/outlives.rs @@ -3,6 +3,7 @@ // RFC for reference. use crate::ty::subst::{GenericArg, GenericArgKind}; +use crate::ty::walk::MiniSet; use crate::ty::{self, Ty, TyCtxt, TypeFoldable}; use smallvec::SmallVec; @@ -50,12 +51,18 @@ impl<'tcx> TyCtxt<'tcx> { /// Push onto `out` all the things that must outlive `'a` for the condition /// `ty0: 'a` to hold. Note that `ty0` must be a **fully resolved type**. pub fn push_outlives_components(self, ty0: Ty<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>) { - compute_components(self, ty0, out); + let mut visited = MiniSet::new(); + compute_components(self, ty0, out, &mut visited); debug!("components({:?}) = {:?}", ty0, out); } } -fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>) { +fn compute_components( + tcx: TyCtxt<'tcx>, + ty: Ty<'tcx>, + out: &mut SmallVec<[Component<'tcx>; 4]>, + visited: &mut MiniSet>, +) { // Descend through the types, looking for the various "base" // components and collecting them into `out`. This is not written // with `collect()` because of the need to sometimes skip subtrees @@ -73,11 +80,11 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo for child in substs { match child.unpack() { GenericArgKind::Type(ty) => { - compute_components(tcx, ty, out); + compute_components(tcx, ty, out, visited); } GenericArgKind::Lifetime(_) => {} GenericArgKind::Const(_) => { - compute_components_recursive(tcx, child, out); + compute_components_recursive(tcx, child, out, visited); } } } @@ -85,19 +92,19 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo ty::Array(element, _) => { // Don't look into the len const as it doesn't affect regions - compute_components(tcx, element, out); + compute_components(tcx, element, out, visited); } ty::Closure(_, ref substs) => { for upvar_ty in substs.as_closure().upvar_tys() { - compute_components(tcx, upvar_ty, out); + compute_components(tcx, upvar_ty, out, visited); } } ty::Generator(_, ref substs, _) => { // Same as the closure case for upvar_ty in substs.as_generator().upvar_tys() { - compute_components(tcx, upvar_ty, out); + compute_components(tcx, upvar_ty, out, visited); } // We ignore regions in the generator interior as we don't @@ -135,7 +142,8 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo // OutlivesProjectionComponents. Continue walking // through and constrain Pi. let mut subcomponents = smallvec![]; - compute_components_recursive(tcx, ty.into(), &mut subcomponents); + let mut subvisited = MiniSet::new(); + compute_components_recursive(tcx, ty.into(), &mut subcomponents, &mut subvisited); out.push(Component::EscapingProjection(subcomponents.into_iter().collect())); } } @@ -177,7 +185,7 @@ fn compute_components(tcx: TyCtxt<'tcx>, ty: Ty<'tcx>, out: &mut SmallVec<[Compo // the "bound regions list". In our representation, no such // list is maintained explicitly, because bound regions // themselves can be readily identified. - compute_components_recursive(tcx, ty.into(), out); + compute_components_recursive(tcx, ty.into(), out, visited); } } } @@ -186,11 +194,12 @@ fn compute_components_recursive( tcx: TyCtxt<'tcx>, parent: GenericArg<'tcx>, out: &mut SmallVec<[Component<'tcx>; 4]>, + visited: &mut MiniSet>, ) { - for child in parent.walk_shallow() { + for child in parent.walk_shallow(visited) { match child.unpack() { GenericArgKind::Type(ty) => { - compute_components(tcx, ty, out); + compute_components(tcx, ty, out, visited); } GenericArgKind::Lifetime(lt) => { // Ignore late-bound regions. @@ -199,7 +208,7 @@ fn compute_components_recursive( } } GenericArgKind::Const(_) => { - compute_components_recursive(tcx, child, out); + compute_components_recursive(tcx, child, out, visited); } } } diff --git a/src/librustc_middle/ty/walk.rs b/src/librustc_middle/ty/walk.rs index 82c649b8f543b..024f655eb6522 100644 --- a/src/librustc_middle/ty/walk.rs +++ b/src/librustc_middle/ty/walk.rs @@ -3,7 +3,50 @@ use crate::ty; use crate::ty::subst::{GenericArg, GenericArgKind}; +use arrayvec::ArrayVec; +use rustc_data_structures::fx::FxHashSet; use smallvec::{self, SmallVec}; +use std::hash::Hash; + +/// Small-storage-optimized implementation of a set +/// made specifically for walking type tree. +/// +/// Stores elements in a small array up to a certain length +/// and switches to `HashSet` when that length is exceeded. +pub enum MiniSet { + Array(ArrayVec<[T; 8]>), + Set(FxHashSet), +} + +impl MiniSet { + /// Creates an empty `MiniSet`. + pub fn new() -> Self { + MiniSet::Array(ArrayVec::new()) + } + + /// Adds a value to the set. + /// + /// If the set did not have this value present, true is returned. + /// + /// If the set did have this value present, false is returned. + pub fn insert(&mut self, elem: T) -> bool { + match self { + MiniSet::Array(array) => { + if array.iter().any(|e| *e == elem) { + false + } else { + if array.try_push(elem).is_err() { + let mut set: FxHashSet = array.iter().copied().collect(); + set.insert(elem); + *self = MiniSet::Set(set); + } + true + } + } + MiniSet::Set(set) => set.insert(elem), + } + } +} // The TypeWalker's stack is hot enough that it's worth going to some effort to // avoid heap allocations. @@ -12,11 +55,20 @@ type TypeWalkerStack<'tcx> = SmallVec<[GenericArg<'tcx>; 8]>; pub struct TypeWalker<'tcx> { stack: TypeWalkerStack<'tcx>, last_subtree: usize, + visited: MiniSet>, } +/// An iterator for walking the type tree. +/// +/// It's very easy to produce a deeply +/// nested type tree with a lot of +/// identical subtrees. In order to work efficiently +/// in this situation walker only visits each type once. +/// It maintains a set of visited types and +/// skips any types that are already there. impl<'tcx> TypeWalker<'tcx> { - pub fn new(root: GenericArg<'tcx>) -> TypeWalker<'tcx> { - TypeWalker { stack: smallvec![root], last_subtree: 1 } + pub fn new(root: GenericArg<'tcx>) -> Self { + Self { stack: smallvec![root], last_subtree: 1, visited: MiniSet::new() } } /// Skips the subtree corresponding to the last type @@ -41,11 +93,15 @@ impl<'tcx> Iterator for TypeWalker<'tcx> { fn next(&mut self) -> Option> { debug!("next(): stack={:?}", self.stack); - let next = self.stack.pop()?; - self.last_subtree = self.stack.len(); - push_inner(&mut self.stack, next); - debug!("next: stack={:?}", self.stack); - Some(next) + loop { + let next = self.stack.pop()?; + self.last_subtree = self.stack.len(); + if self.visited.insert(next) { + push_inner(&mut self.stack, next); + debug!("next: stack={:?}", self.stack); + return Some(next); + } + } } } @@ -67,9 +123,17 @@ impl GenericArg<'tcx> { /// Iterator that walks the immediate children of `self`. Hence /// `Foo, u32>` yields the sequence `[Bar, u32]` /// (but not `i32`, like `walk`). - pub fn walk_shallow(self) -> impl Iterator> { + /// + /// Iterator only walks items once. + /// It accepts visited set, updates it with all visited types + /// and skips any types that are already there. + pub fn walk_shallow( + self, + visited: &mut MiniSet>, + ) -> impl Iterator> { let mut stack = SmallVec::new(); push_inner(&mut stack, self); + stack.retain(|a| visited.insert(*a)); stack.into_iter() } } diff --git a/src/test/ui/closures/issue-72408-nested-closures-exponential.rs b/src/test/ui/closures/issue-72408-nested-closures-exponential.rs new file mode 100644 index 0000000000000..2d6ba936572d5 --- /dev/null +++ b/src/test/ui/closures/issue-72408-nested-closures-exponential.rs @@ -0,0 +1,59 @@ +// build-pass + +// Closures include captured types twice in a type tree. +// +// Wrapping one closure with another leads to doubling +// the amount of types in the type tree. +// +// This test ensures that rust can handle +// deeply nested type trees with a lot +// of duplicated subtrees. + +fn dup(f: impl Fn(i32) -> i32) -> impl Fn(i32) -> i32 { + move |a| f(a * 2) +} + +fn main() { + let f = |a| a; + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + // Compiler dies around here if it tries + // to walk the tree exhaustively. + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + let f = dup(f); + + println!("Type size was at least {}", f(1)); +} diff --git a/src/test/ui/issues/issue-22638.rs b/src/test/ui/issues/issue-22638.rs index 72c16fddb4b12..89137538425bf 100644 --- a/src/test/ui/issues/issue-22638.rs +++ b/src/test/ui/issues/issue-22638.rs @@ -51,9 +51,9 @@ struct D (Box); impl D { pub fn matches(&self, f: &F) { - //~^ ERROR reached the type-length limit while instantiating `D::matches::<[closure let &D(ref a) = self; a.matches(f) + //~^ ERROR reached the recursion limit while instantiating `A::matches::<[closure } } diff --git a/src/test/ui/issues/issue-22638.stderr b/src/test/ui/issues/issue-22638.stderr index b0df46b11fadb..c4255b95b704e 100644 --- a/src/test/ui/issues/issue-22638.stderr +++ b/src/test/ui/issues/issue-22638.stderr @@ -1,10 +1,14 @@ -error: reached the type-length limit while instantiating `D::matches::$CLOSURE` - --> $DIR/issue-22638.rs:53:5 +error: reached the recursion limit while instantiating `A::matches::$CLOSURE` + --> $DIR/issue-22638.rs:55:9 + | +LL | a.matches(f) + | ^^^^^^^^^^^^ + | +note: `A::matches` defined here + --> $DIR/issue-22638.rs:14:5 | LL | pub fn matches(&self, f: &F) { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: consider adding a `#![type_length_limit="30408681"]` attribute to your crate error: aborting due to previous error diff --git a/src/test/ui/type_length_limit.rs b/src/test/ui/type_length_limit.rs index 1f1c8ad962690..921cded5037b6 100644 --- a/src/test/ui/type_length_limit.rs +++ b/src/test/ui/type_length_limit.rs @@ -4,7 +4,7 @@ // Test that the type length limit can be changed. #![allow(dead_code)] -#![type_length_limit="256"] +#![type_length_limit="4"] macro_rules! link { ($id:ident, $t:ty) => { diff --git a/src/test/ui/type_length_limit.stderr b/src/test/ui/type_length_limit.stderr index 0d90f06076ab2..977a8dd4329c7 100644 --- a/src/test/ui/type_length_limit.stderr +++ b/src/test/ui/type_length_limit.stderr @@ -4,7 +4,7 @@ error: reached the type-length limit while instantiating `std::mem::drop::(_x: T) {} | ^^^^^^^^^^^^^^^^^^^^^^^^ | - = note: consider adding a `#![type_length_limit="1094"]` attribute to your crate + = note: consider adding a `#![type_length_limit="8"]` attribute to your crate error: aborting due to previous error From 87022d00b21249d5ef2b262e948a024ef94385d7 Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Tue, 15 Sep 2020 18:22:24 -0500 Subject: [PATCH 2/9] Better handling for exponential-sized types in misc places Mostly to fix ui/issues/issue-37311-type-length-limit/issue-37311.rs. Most parts of the compiler can handle deeply nested types with a lot of duplicates just fine, but some parts still attempt to naively traverse type tree. Before such problems were caught by type length limit check, but now these places will have to be changed to handle duplicated types gracefully. --- src/librustc_infer/infer/combine.rs | 15 +++++- src/librustc_middle/ty/print/mod.rs | 30 +++++++++--- src/librustc_middle/ty/print/pretty.rs | 12 ++++- src/librustc_mir/monomorphize/collector.rs | 47 +++++++++++-------- .../ui/infinite/infinite-instantiation.stderr | 2 +- .../issue-37311.rs | 4 +- .../issue-37311.stderr | 10 ++-- src/test/ui/issues/issue-67552.stderr | 2 +- src/test/ui/issues/issue-8727.stderr | 2 +- ...-38591-non-regular-dropck-recursion.stderr | 2 +- src/test/ui/recursion/recursion.stderr | 2 +- src/test/ui/type_length_limit.stderr | 2 +- 12 files changed, 89 insertions(+), 41 deletions(-) diff --git a/src/librustc_infer/infer/combine.rs b/src/librustc_infer/infer/combine.rs index 3a54a6475301e..53d1f41458e85 100644 --- a/src/librustc_infer/infer/combine.rs +++ b/src/librustc_infer/infer/combine.rs @@ -31,6 +31,7 @@ use super::unify_key::replace_if_possible; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; use super::{InferCtxt, MiscVariable, TypeTrace}; +use rustc_data_structures::fx::FxHashMap; use crate::traits::{Obligation, PredicateObligations}; @@ -379,6 +380,7 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> { needs_wf: false, root_ty: ty, param_env: self.param_env, + cache: FxHashMap::default(), }; let ty = match generalize.relate(ty, ty) { @@ -438,6 +440,8 @@ struct Generalizer<'cx, 'tcx> { root_ty: Ty<'tcx>, param_env: ty::ParamEnv<'tcx>, + + cache: FxHashMap<(Ty<'tcx>, Ty<'tcx>), RelateResult<'tcx, Ty<'tcx>>>, } /// Result from a generalization operation. This includes @@ -535,13 +539,17 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { fn tys(&mut self, t: Ty<'tcx>, t2: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { assert_eq!(t, t2); // we are abusing TypeRelation here; both LHS and RHS ought to be == + let cache_key = (t, t2); + if let Some(result) = self.cache.get(&cache_key) { + return result.clone(); + } debug!("generalize: t={:?}", t); // Check to see whether the type we are generalizing references // any other type variable related to `vid` via // subtyping. This is basically our "occurs check", preventing // us from creating infinitely sized types. - match t.kind { + let result = match t.kind { ty::Infer(ty::TyVar(vid)) => { let vid = self.infcx.inner.borrow_mut().type_variables().root_var(vid); let sub_vid = self.infcx.inner.borrow_mut().type_variables().sub_root_var(vid); @@ -598,7 +606,10 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { Ok(t) } _ => relate::super_relate_tys(self, t, t), - } + }; + + self.cache.insert(cache_key, result.clone()); + return result; } fn regions( diff --git a/src/librustc_middle/ty/print/mod.rs b/src/librustc_middle/ty/print/mod.rs index 6c8f23c139f6e..bd3236bbb23f3 100644 --- a/src/librustc_middle/ty/print/mod.rs +++ b/src/librustc_middle/ty/print/mod.rs @@ -4,6 +4,7 @@ use crate::ty::{self, DefIdTree, Ty, TyCtxt}; use rustc_data_structures::fx::FxHashSet; use rustc_hir::def_id::{CrateNum, DefId}; use rustc_hir::definitions::{DefPathData, DisambiguatedDefPathData}; +use rustc_middle::ty::walk::MiniSet; // `pretty` is a separate module only for organization. mod pretty; @@ -265,21 +266,33 @@ pub trait Printer<'tcx>: Sized { /// function tries to find a "characteristic `DefId`" for a /// type. It's just a heuristic so it makes some questionable /// decisions and we may want to adjust it later. -pub fn characteristic_def_id_of_type(ty: Ty<'_>) -> Option { +/// +/// Visited set is needed in to avoid full iteration over +/// deeply nested tuples that have no DefId. +fn characteristic_def_id_of_type_cached<'a>( + ty: Ty<'a>, + visited: &mut MiniSet>, +) -> Option { match ty.kind { ty::Adt(adt_def, _) => Some(adt_def.did), ty::Dynamic(data, ..) => data.principal_def_id(), - ty::Array(subty, _) | ty::Slice(subty) => characteristic_def_id_of_type(subty), + ty::Array(subty, _) | ty::Slice(subty) => { + characteristic_def_id_of_type_cached(subty, visited) + } - ty::RawPtr(mt) => characteristic_def_id_of_type(mt.ty), + ty::RawPtr(mt) => characteristic_def_id_of_type_cached(mt.ty, visited), - ty::Ref(_, ty, _) => characteristic_def_id_of_type(ty), + ty::Ref(_, ty, _) => characteristic_def_id_of_type_cached(ty, visited), - ty::Tuple(ref tys) => { - tys.iter().find_map(|ty| characteristic_def_id_of_type(ty.expect_ty())) - } + ty::Tuple(ref tys) => tys.iter().find_map(|ty| { + let ty = ty.expect_ty(); + if visited.insert(ty) { + return characteristic_def_id_of_type_cached(ty, visited); + } + return None; + }), ty::FnDef(def_id, _) | ty::Closure(def_id, _) @@ -304,6 +317,9 @@ pub fn characteristic_def_id_of_type(ty: Ty<'_>) -> Option { | ty::Float(_) => None, } } +pub fn characteristic_def_id_of_type(ty: Ty<'_>) -> Option { + characteristic_def_id_of_type_cached(ty, &mut MiniSet::new()) +} impl<'tcx, P: Printer<'tcx>> Print<'tcx, P> for ty::RegionKind { type Output = P::Region; diff --git a/src/librustc_middle/ty/print/pretty.rs b/src/librustc_middle/ty/print/pretty.rs index 999a1d52a26a2..a29e0b0000130 100644 --- a/src/librustc_middle/ty/print/pretty.rs +++ b/src/librustc_middle/ty/print/pretty.rs @@ -1226,6 +1226,7 @@ pub struct FmtPrinterData<'a, 'tcx, F> { used_region_names: FxHashSet, region_index: usize, binder_depth: usize, + printed_type_count: usize, pub region_highlight_mode: RegionHighlightMode, @@ -1256,6 +1257,7 @@ impl FmtPrinter<'a, 'tcx, F> { used_region_names: Default::default(), region_index: 0, binder_depth: 0, + printed_type_count: 0, region_highlight_mode: RegionHighlightMode::default(), name_resolver: None, })) @@ -1368,8 +1370,14 @@ impl Printer<'tcx> for FmtPrinter<'_, 'tcx, F> { self.pretty_print_region(region) } - fn print_type(self, ty: Ty<'tcx>) -> Result { - self.pretty_print_type(ty) + fn print_type(mut self, ty: Ty<'tcx>) -> Result { + if self.tcx.sess.type_length_limit().value_within_limit(self.printed_type_count) { + self.printed_type_count += 1; + self.pretty_print_type(ty) + } else { + write!(self, "...")?; + Ok(self) + } } fn print_dyn_existential( diff --git a/src/librustc_mir/monomorphize/collector.rs b/src/librustc_mir/monomorphize/collector.rs index d379f4ef428a6..1afa720f69e77 100644 --- a/src/librustc_mir/monomorphize/collector.rs +++ b/src/librustc_mir/monomorphize/collector.rs @@ -419,6 +419,29 @@ fn record_accesses<'a, 'tcx: 'a>( inlining_map.lock_mut().record_accesses(caller, &accesses); } +// Shrinks string by keeping prefix and suffix of given sizes. +fn shrink(s: String, before: usize, after: usize) -> String { + // An iterator of all byte positions including the end of the string. + let positions = || s.char_indices().map(|(i, _)| i).chain(iter::once(s.len())); + + let shrunk = format!( + "{before}...{after}", + before = &s[..positions().nth(before).unwrap_or(s.len())], + after = &s[positions().rev().nth(after).unwrap_or(0)..], + ); + + // Only use the shrunk version if it's really shorter. + // This also avoids the case where before and after slices overlap. + if shrunk.len() < s.len() { shrunk } else { s } +} + +// Format instance name that is already known to be too long for rustc. +// Show only the first and last 32 characters to avoid blasting +// the user's terminal with thousands of lines of type-name. +fn shrunk_instance_name(instance: &Instance<'tcx>) -> String { + shrink(instance.to_string(), 32, 32) +} + fn check_recursion_limit<'tcx>( tcx: TyCtxt<'tcx>, instance: Instance<'tcx>, @@ -441,7 +464,10 @@ fn check_recursion_limit<'tcx>( // more than the recursion limit is assumed to be causing an // infinite expansion. if !tcx.sess.recursion_limit().value_within_limit(adjusted_recursion_depth) { - let error = format!("reached the recursion limit while instantiating `{}`", instance); + let error = format!( + "reached the recursion limit while instantiating `{}`", + shrunk_instance_name(&instance), + ); let mut err = tcx.sess.struct_span_fatal(span, &error); err.span_note( tcx.def_span(def_id), @@ -475,26 +501,9 @@ fn check_type_length_limit<'tcx>(tcx: TyCtxt<'tcx>, instance: Instance<'tcx>) { // // Bail out in these cases to avoid that bad user experience. if !tcx.sess.type_length_limit().value_within_limit(type_length) { - // The instance name is already known to be too long for rustc. - // Show only the first and last 32 characters to avoid blasting - // the user's terminal with thousands of lines of type-name. - let shrink = |s: String, before: usize, after: usize| { - // An iterator of all byte positions including the end of the string. - let positions = || s.char_indices().map(|(i, _)| i).chain(iter::once(s.len())); - - let shrunk = format!( - "{before}...{after}", - before = &s[..positions().nth(before).unwrap_or(s.len())], - after = &s[positions().rev().nth(after).unwrap_or(0)..], - ); - - // Only use the shrunk version if it's really shorter. - // This also avoids the case where before and after slices overlap. - if shrunk.len() < s.len() { shrunk } else { s } - }; let msg = format!( "reached the type-length limit while instantiating `{}`", - shrink(instance.to_string(), 32, 32) + shrunk_instance_name(&instance), ); let mut diag = tcx.sess.struct_span_fatal(tcx.def_span(instance.def_id()), &msg); diag.note(&format!( diff --git a/src/test/ui/infinite/infinite-instantiation.stderr b/src/test/ui/infinite/infinite-instantiation.stderr index eb07d8905d609..72683629694f3 100644 --- a/src/test/ui/infinite/infinite-instantiation.stderr +++ b/src/test/ui/infinite/infinite-instantiation.stderr @@ -1,4 +1,4 @@ -error: reached the recursion limit while instantiating `function::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` +error: reached the recursion limit while instantiating `function::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` --> $DIR/infinite-instantiation.rs:21:9 | LL | function(counter - 1, t.to_option()); diff --git a/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.rs b/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.rs index fec4b17153609..d3d5863ddb3c9 100644 --- a/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.rs +++ b/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.rs @@ -12,8 +12,8 @@ trait Foo { impl Foo for T { #[allow(unconditional_recursion)] - fn recurse(&self) { //~ ERROR reached the type-length limit - (self, self).recurse(); + fn recurse(&self) { + (self, self).recurse(); //~ ERROR reached the recursion limit } } diff --git a/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.stderr b/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.stderr index 6229d90d4b477..a94f190d6b25d 100644 --- a/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.stderr +++ b/src/test/ui/issues/issue-37311-type-length-limit/issue-37311.stderr @@ -1,10 +1,14 @@ -error: reached the type-length limit while instantiating `<(&(&(&(&(&(&(&(&(&(&(&(&(&(&(&(...))))))))))))))) as Foo>::recurse` +error: reached the recursion limit while instantiating `<(&(&(&(&(&(&(&(&(&(&(&(&(&(&(&(.....), ...), ...) as Foo>::recurse` + --> $DIR/issue-37311.rs:16:9 + | +LL | (self, self).recurse(); + | ^^^^^^^^^^^^^^^^^^^^^^ + | +note: `::recurse` defined here --> $DIR/issue-37311.rs:15:5 | LL | fn recurse(&self) { | ^^^^^^^^^^^^^^^^^ - | - = note: consider adding a `#![type_length_limit="2097149"]` attribute to your crate error: aborting due to previous error diff --git a/src/test/ui/issues/issue-67552.stderr b/src/test/ui/issues/issue-67552.stderr index 8243e52039d48..f3e73399b57ce 100644 --- a/src/test/ui/issues/issue-67552.stderr +++ b/src/test/ui/issues/issue-67552.stderr @@ -1,4 +1,4 @@ -error: reached the recursion limit while instantiating `rec::<&mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut &mut Empty>` +error: reached the recursion limit while instantiating `rec::<&mut &mut &mut &mut &mut &... &mut &mut &mut &mut &mut Empty>` --> $DIR/issue-67552.rs:27:9 | LL | rec(identity(&mut it)) diff --git a/src/test/ui/issues/issue-8727.stderr b/src/test/ui/issues/issue-8727.stderr index 59008151f1a5b..8d26c566d4193 100644 --- a/src/test/ui/issues/issue-8727.stderr +++ b/src/test/ui/issues/issue-8727.stderr @@ -9,7 +9,7 @@ LL | generic::>(); = note: `#[warn(unconditional_recursion)]` on by default = help: a `loop` may express intention better if this is on purpose -error: reached the recursion limit while instantiating `generic::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` +error: reached the recursion limit while instantiating `generic::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` --> $DIR/issue-8727.rs:7:5 | LL | generic::>(); diff --git a/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr b/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr index ba5e8a9e39f72..72547fe79fdee 100644 --- a/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr +++ b/src/test/ui/recursion/issue-38591-non-regular-dropck-recursion.stderr @@ -1,4 +1,4 @@ -error: reached the recursion limit while instantiating `std::intrinsics::drop_in_place::> - shim(Some(S))` +error: reached the recursion limit while instantiating `std::intrinsics::drop_in_place::...)))))))))))))))))))))))))))))>))` --> $SRC_DIR/core/src/ptr/mod.rs:LL:COL | LL | / pub unsafe fn drop_in_place(to_drop: *mut T) { diff --git a/src/test/ui/recursion/recursion.stderr b/src/test/ui/recursion/recursion.stderr index db4c99eeb8b16..085bf82ef8b93 100644 --- a/src/test/ui/recursion/recursion.stderr +++ b/src/test/ui/recursion/recursion.stderr @@ -1,4 +1,4 @@ -error: reached the recursion limit while instantiating `test::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` +error: reached the recursion limit while instantiating `test::>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>` --> $DIR/recursion.rs:17:11 | LL | _ => {test (n-1, i+1, Cons {head:2*i+1, tail:first}, Cons{head:i*i, tail:second})} diff --git a/src/test/ui/type_length_limit.stderr b/src/test/ui/type_length_limit.stderr index 977a8dd4329c7..83da193eb04d2 100644 --- a/src/test/ui/type_length_limit.stderr +++ b/src/test/ui/type_length_limit.stderr @@ -1,4 +1,4 @@ -error: reached the type-length limit while instantiating `std::mem::drop::>` +error: reached the type-length limit while instantiating `std::mem::drop::>` --> $SRC_DIR/core/src/mem/mod.rs:LL:COL | LL | pub fn drop(_x: T) {} From 507dd1d918ca19cf17eddf751685fda8249dface Mon Sep 17 00:00:00 2001 From: Valerii Lashmanov Date: Thu, 17 Sep 2020 20:43:29 -0500 Subject: [PATCH 3/9] Intorduced MiniMap - a tiny small storage optimized map implementation This makes everything about 1% faster in rustc-perf, mostly negating performance hit of previous commit. --- Cargo.lock | 1 + src/librustc_infer/Cargo.toml | 1 + src/librustc_infer/infer/combine.rs | 63 ++++++++++++++++++++++++++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index eacc28a756765..1b6c690362461 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3599,6 +3599,7 @@ dependencies = [ name = "rustc_infer" version = "0.0.0" dependencies = [ + "arrayvec", "rustc_ast", "rustc_data_structures", "rustc_errors", diff --git a/src/librustc_infer/Cargo.toml b/src/librustc_infer/Cargo.toml index e1698d66323c3..a0fcc881e4b84 100644 --- a/src/librustc_infer/Cargo.toml +++ b/src/librustc_infer/Cargo.toml @@ -23,4 +23,5 @@ rustc_serialize = { path = "../librustc_serialize" } rustc_span = { path = "../librustc_span" } rustc_target = { path = "../librustc_target" } smallvec = { version = "1.0", features = ["union", "may_dangle"] } +arrayvec = { version = "0.5.1", default-features = false } rustc_ast = { path = "../librustc_ast" } diff --git a/src/librustc_infer/infer/combine.rs b/src/librustc_infer/infer/combine.rs index 53d1f41458e85..3a2ba8f00a67e 100644 --- a/src/librustc_infer/infer/combine.rs +++ b/src/librustc_infer/infer/combine.rs @@ -31,7 +31,9 @@ use super::unify_key::replace_if_possible; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; use super::{InferCtxt, MiscVariable, TypeTrace}; +use arrayvec::ArrayVec; use rustc_data_structures::fx::FxHashMap; +use std::hash::Hash; use crate::traits::{Obligation, PredicateObligations}; @@ -45,6 +47,63 @@ use rustc_middle::ty::{self, InferConst, ToPredicate, Ty, TyCtxt, TypeFoldable}; use rustc_middle::ty::{IntType, UintType}; use rustc_span::DUMMY_SP; +/// Small-storage-optimized implementation of a map +/// made specifically for caching results. +/// +/// Stores elements in a small array up to a certain length +/// and switches to `HashMap` when that length is exceeded. +enum MiniMap { + Array(ArrayVec<[(K, V); 8]>), + Map(FxHashMap), +} + +impl MiniMap { + /// Creates an empty `MiniMap`. + pub fn new() -> Self { + MiniMap::Array(ArrayVec::new()) + } + + /// Inserts or updates value in the map. + pub fn insert(&mut self, key: K, value: V) { + match self { + MiniMap::Array(array) => { + for pair in array.iter_mut() { + if pair.0 == key { + pair.1 = value; + return; + } + } + if let Err(error) = array.try_push((key, value)) { + let mut map: FxHashMap = array.drain(..).collect(); + let (key, value) = error.element(); + map.insert(key, value); + *self = MiniMap::Map(map); + } + } + MiniMap::Map(map) => { + map.insert(key, value); + } + } + } + + /// Return value by key if any. + pub fn get(&self, key: &K) -> Option<&V> { + match self { + MiniMap::Array(array) => { + for pair in array { + if pair.0 == *key { + return Some(&pair.1); + } + } + return None; + } + MiniMap::Map(map) => { + return map.get(key); + } + } + } +} + #[derive(Clone)] pub struct CombineFields<'infcx, 'tcx> { pub infcx: &'infcx InferCtxt<'infcx, 'tcx>, @@ -380,7 +439,7 @@ impl<'infcx, 'tcx> CombineFields<'infcx, 'tcx> { needs_wf: false, root_ty: ty, param_env: self.param_env, - cache: FxHashMap::default(), + cache: MiniMap::new(), }; let ty = match generalize.relate(ty, ty) { @@ -441,7 +500,7 @@ struct Generalizer<'cx, 'tcx> { param_env: ty::ParamEnv<'tcx>, - cache: FxHashMap<(Ty<'tcx>, Ty<'tcx>), RelateResult<'tcx, Ty<'tcx>>>, + cache: MiniMap<(Ty<'tcx>, Ty<'tcx>), RelateResult<'tcx, Ty<'tcx>>>, } /// Result from a generalization operation. This includes From b0db5342998d49ef2536f52fe32d721da2c769e6 Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Fri, 18 Sep 2020 06:07:19 +0000 Subject: [PATCH 4/9] Remove redundancy in cache key --- src/librustc_infer/infer/combine.rs | 7 +++---- src/librustc_middle/ty/print/mod.rs | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/librustc_infer/infer/combine.rs b/src/librustc_infer/infer/combine.rs index 3a2ba8f00a67e..1f70490e54468 100644 --- a/src/librustc_infer/infer/combine.rs +++ b/src/librustc_infer/infer/combine.rs @@ -500,7 +500,7 @@ struct Generalizer<'cx, 'tcx> { param_env: ty::ParamEnv<'tcx>, - cache: MiniMap<(Ty<'tcx>, Ty<'tcx>), RelateResult<'tcx, Ty<'tcx>>>, + cache: MiniMap, RelateResult<'tcx, Ty<'tcx>>>, } /// Result from a generalization operation. This includes @@ -598,8 +598,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { fn tys(&mut self, t: Ty<'tcx>, t2: Ty<'tcx>) -> RelateResult<'tcx, Ty<'tcx>> { assert_eq!(t, t2); // we are abusing TypeRelation here; both LHS and RHS ought to be == - let cache_key = (t, t2); - if let Some(result) = self.cache.get(&cache_key) { + if let Some(result) = self.cache.get(&t) { return result.clone(); } debug!("generalize: t={:?}", t); @@ -667,7 +666,7 @@ impl TypeRelation<'tcx> for Generalizer<'_, 'tcx> { _ => relate::super_relate_tys(self, t, t), }; - self.cache.insert(cache_key, result.clone()); + self.cache.insert(t, result.clone()); return result; } diff --git a/src/librustc_middle/ty/print/mod.rs b/src/librustc_middle/ty/print/mod.rs index bd3236bbb23f3..981e013683ba3 100644 --- a/src/librustc_middle/ty/print/mod.rs +++ b/src/librustc_middle/ty/print/mod.rs @@ -267,7 +267,7 @@ pub trait Printer<'tcx>: Sized { /// type. It's just a heuristic so it makes some questionable /// decisions and we may want to adjust it later. /// -/// Visited set is needed in to avoid full iteration over +/// Visited set is needed to avoid full iteration over /// deeply nested tuples that have no DefId. fn characteristic_def_id_of_type_cached<'a>( ty: Ty<'a>, From 7dca7cc04a42b9ff33beac0403d903253d6a203c Mon Sep 17 00:00:00 2001 From: Bastian Kauschke Date: Sat, 19 Sep 2020 17:27:13 +0200 Subject: [PATCH 5/9] cache types during normalization --- Cargo.lock | 1 + src/librustc_data_structures/Cargo.toml | 1 + src/librustc_data_structures/lib.rs | 12 ++-- src/librustc_data_structures/mini_map.rs | 61 +++++++++++++++++++ src/librustc_index/Cargo.toml | 2 +- src/librustc_infer/infer/combine.rs | 61 +------------------ .../traits/query/normalize.rs | 13 +++- 7 files changed, 83 insertions(+), 68 deletions(-) create mode 100644 src/librustc_data_structures/mini_map.rs diff --git a/Cargo.lock b/Cargo.lock index 1b6c690362461..d5466d57e771a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3424,6 +3424,7 @@ dependencies = [ name = "rustc_data_structures" version = "0.0.0" dependencies = [ + "arrayvec", "bitflags", "cfg-if", "crossbeam-utils 0.7.2", diff --git a/src/librustc_data_structures/Cargo.toml b/src/librustc_data_structures/Cargo.toml index 988bb733f9fb7..590506512021b 100644 --- a/src/librustc_data_structures/Cargo.toml +++ b/src/librustc_data_structures/Cargo.toml @@ -10,6 +10,7 @@ path = "lib.rs" doctest = false [dependencies] +arrayvec = { version = "0.5.1", default-features = false } ena = "0.14" indexmap = "1.5.1" tracing = "0.1" diff --git a/src/librustc_data_structures/lib.rs b/src/librustc_data_structures/lib.rs index af4a7bd18813e..cd25ba2ac6910 100644 --- a/src/librustc_data_structures/lib.rs +++ b/src/librustc_data_structures/lib.rs @@ -85,24 +85,26 @@ pub mod sorted_map; pub mod stable_set; #[macro_use] pub mod stable_hasher; +mod atomic_ref; +pub mod fingerprint; +pub mod profiling; pub mod sharded; pub mod stack; pub mod sync; pub mod thin_vec; pub mod tiny_list; pub mod transitive_relation; -pub use ena::undo_log; -pub use ena::unify; -mod atomic_ref; -pub mod fingerprint; -pub mod profiling; pub mod vec_linked_list; pub mod work_queue; pub use atomic_ref::AtomicRef; pub mod frozen; +pub mod mini_map; pub mod tagged_ptr; pub mod temp_dir; +pub use ena::undo_log; +pub use ena::unify; + pub struct OnDrop(pub F); impl OnDrop { diff --git a/src/librustc_data_structures/mini_map.rs b/src/librustc_data_structures/mini_map.rs new file mode 100644 index 0000000000000..cd3e949d3831a --- /dev/null +++ b/src/librustc_data_structures/mini_map.rs @@ -0,0 +1,61 @@ +use crate::fx::FxHashMap; +use arrayvec::ArrayVec; + +use std::hash::Hash; + +/// Small-storage-optimized implementation of a map +/// made specifically for caching results. +/// +/// Stores elements in a small array up to a certain length +/// and switches to `HashMap` when that length is exceeded. +pub enum MiniMap { + Array(ArrayVec<[(K, V); 8]>), + Map(FxHashMap), +} + +impl MiniMap { + /// Creates an empty `MiniMap`. + pub fn new() -> Self { + MiniMap::Array(ArrayVec::new()) + } + + /// Inserts or updates value in the map. + pub fn insert(&mut self, key: K, value: V) { + match self { + MiniMap::Array(array) => { + for pair in array.iter_mut() { + if pair.0 == key { + pair.1 = value; + return; + } + } + if let Err(error) = array.try_push((key, value)) { + let mut map: FxHashMap = array.drain(..).collect(); + let (key, value) = error.element(); + map.insert(key, value); + *self = MiniMap::Map(map); + } + } + MiniMap::Map(map) => { + map.insert(key, value); + } + } + } + + /// Return value by key if any. + pub fn get(&self, key: &K) -> Option<&V> { + match self { + MiniMap::Array(array) => { + for pair in array { + if pair.0 == *key { + return Some(&pair.1); + } + } + return None; + } + MiniMap::Map(map) => { + return map.get(key); + } + } + } +} diff --git a/src/librustc_index/Cargo.toml b/src/librustc_index/Cargo.toml index 8aaf1cb9cbc58..061f440ef16e1 100644 --- a/src/librustc_index/Cargo.toml +++ b/src/librustc_index/Cargo.toml @@ -10,6 +10,6 @@ path = "lib.rs" doctest = false [dependencies] -arrayvec = "0.5.1" +arrayvec = { version = "0.5.1", default-features = false } rustc_serialize = { path = "../librustc_serialize" } rustc_macros = { path = "../librustc_macros" } diff --git a/src/librustc_infer/infer/combine.rs b/src/librustc_infer/infer/combine.rs index 1f70490e54468..c89d8ced50db7 100644 --- a/src/librustc_infer/infer/combine.rs +++ b/src/librustc_infer/infer/combine.rs @@ -31,13 +31,11 @@ use super::unify_key::replace_if_possible; use super::unify_key::{ConstVarValue, ConstVariableValue}; use super::unify_key::{ConstVariableOrigin, ConstVariableOriginKind}; use super::{InferCtxt, MiscVariable, TypeTrace}; -use arrayvec::ArrayVec; -use rustc_data_structures::fx::FxHashMap; -use std::hash::Hash; use crate::traits::{Obligation, PredicateObligations}; use rustc_ast as ast; +use rustc_data_structures::mini_map::MiniMap; use rustc_hir::def_id::DefId; use rustc_middle::traits::ObligationCause; use rustc_middle::ty::error::TypeError; @@ -47,63 +45,6 @@ use rustc_middle::ty::{self, InferConst, ToPredicate, Ty, TyCtxt, TypeFoldable}; use rustc_middle::ty::{IntType, UintType}; use rustc_span::DUMMY_SP; -/// Small-storage-optimized implementation of a map -/// made specifically for caching results. -/// -/// Stores elements in a small array up to a certain length -/// and switches to `HashMap` when that length is exceeded. -enum MiniMap { - Array(ArrayVec<[(K, V); 8]>), - Map(FxHashMap), -} - -impl MiniMap { - /// Creates an empty `MiniMap`. - pub fn new() -> Self { - MiniMap::Array(ArrayVec::new()) - } - - /// Inserts or updates value in the map. - pub fn insert(&mut self, key: K, value: V) { - match self { - MiniMap::Array(array) => { - for pair in array.iter_mut() { - if pair.0 == key { - pair.1 = value; - return; - } - } - if let Err(error) = array.try_push((key, value)) { - let mut map: FxHashMap = array.drain(..).collect(); - let (key, value) = error.element(); - map.insert(key, value); - *self = MiniMap::Map(map); - } - } - MiniMap::Map(map) => { - map.insert(key, value); - } - } - } - - /// Return value by key if any. - pub fn get(&self, key: &K) -> Option<&V> { - match self { - MiniMap::Array(array) => { - for pair in array { - if pair.0 == *key { - return Some(&pair.1); - } - } - return None; - } - MiniMap::Map(map) => { - return map.get(key); - } - } - } -} - #[derive(Clone)] pub struct CombineFields<'infcx, 'tcx> { pub infcx: &'infcx InferCtxt<'infcx, 'tcx>, diff --git a/src/librustc_trait_selection/traits/query/normalize.rs b/src/librustc_trait_selection/traits/query/normalize.rs index 93652329305a5..17963a6c8290f 100644 --- a/src/librustc_trait_selection/traits/query/normalize.rs +++ b/src/librustc_trait_selection/traits/query/normalize.rs @@ -7,6 +7,7 @@ use crate::infer::canonical::OriginalQueryValues; use crate::infer::{InferCtxt, InferOk}; use crate::traits::error_reporting::InferCtxtExt; use crate::traits::{Obligation, ObligationCause, PredicateObligation, Reveal}; +use rustc_data_structures::mini_map::MiniMap; use rustc_data_structures::stack::ensure_sufficient_stack; use rustc_infer::traits::Normalized; use rustc_middle::ty::fold::{TypeFoldable, TypeFolder}; @@ -57,6 +58,7 @@ impl<'cx, 'tcx> AtExt<'tcx> for At<'cx, 'tcx> { param_env: self.param_env, obligations: vec![], error: false, + cache: MiniMap::new(), anon_depth: 0, }; @@ -85,6 +87,7 @@ struct QueryNormalizer<'cx, 'tcx> { cause: &'cx ObligationCause<'tcx>, param_env: ty::ParamEnv<'tcx>, obligations: Vec>, + cache: MiniMap, Ty<'tcx>>, error: bool, anon_depth: usize, } @@ -99,8 +102,12 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> { return ty; } + if let Some(ty) = self.cache.get(&ty) { + return ty; + } + let ty = ty.super_fold_with(self); - match ty.kind { + let res = (|| match ty.kind { ty::Opaque(def_id, substs) => { // Only normalize `impl Trait` after type-checking, usually in codegen. match self.param_env.reveal() { @@ -197,7 +204,9 @@ impl<'cx, 'tcx> TypeFolder<'tcx> for QueryNormalizer<'cx, 'tcx> { } _ => ty, - } + })(); + self.cache.insert(ty, res); + res } fn fold_const(&mut self, constant: &'tcx ty::Const<'tcx>) -> &'tcx ty::Const<'tcx> { From 5b3a05a04b87492af44b200a065004062b83540e Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Thu, 1 Oct 2020 19:26:07 +0200 Subject: [PATCH 6/9] ci: switch to environment files to change the environment on GHA See GitHub's blog post on why the change was necessary: https://github.blog/changelog/2020-10-01-github-actions-deprecating-set-env-and-add-path-commands/ --- src/ci/shared.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ci/shared.sh b/src/ci/shared.sh index 8222758ed6dc4..c93d4774e3992 100644 --- a/src/ci/shared.sh +++ b/src/ci/shared.sh @@ -104,7 +104,7 @@ function ciCommandAddPath { if isAzurePipelines; then echo "##vso[task.prependpath]${path}" elif isGitHubActions; then - echo "::add-path::${path}" + echo "${path}" >> "${GITHUB_PATH}" else echo "ciCommandAddPath only works inside CI!" exit 1 @@ -122,7 +122,7 @@ function ciCommandSetEnv { if isAzurePipelines; then echo "##vso[task.setvariable variable=${name}]${value}" elif isGitHubActions; then - echo "::set-env name=${name}::${value}" + echo "${name}=${value}" >> "${GITHUB_ENV}" else echo "ciCommandSetEnv only works inside CI!" exit 1 From 5c29a949b3d3366b962230982059fa7befc6e49b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Deharbe?= Date: Fri, 11 Sep 2020 11:11:11 +0200 Subject: [PATCH 7/9] replacing sub's that can wrap by saturating_sub's --- src/librustc_errors/emitter.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustc_errors/emitter.rs b/src/librustc_errors/emitter.rs index 5a654e83aed8e..4555168af0ab5 100644 --- a/src/librustc_errors/emitter.rs +++ b/src/librustc_errors/emitter.rs @@ -959,15 +959,15 @@ impl EmitterWriter { '_', line_offset + pos, width_offset + depth, - code_offset + annotation.start_col - left, + (code_offset + annotation.start_col).saturating_sub(left), style, ); } _ if self.teach => { buffer.set_style_range( line_offset, - code_offset + annotation.start_col - left, - code_offset + annotation.end_col - left, + (code_offset + annotation.start_col).saturating_sub(left), + (code_offset + annotation.end_col).saturating_sub(left), style, annotation.is_primary, ); From f3ab317e12916c28dd9fd55ff9c7697ee465ac49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Deharbe?= Date: Fri, 11 Sep 2020 13:58:03 +0200 Subject: [PATCH 8/9] add non-regression test for issue #76597 --- src/test/ui/issue-76597.rs | 7 +++++++ src/test/ui/issue-76597.stderr | 14 ++++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 src/test/ui/issue-76597.rs create mode 100644 src/test/ui/issue-76597.stderr diff --git a/src/test/ui/issue-76597.rs b/src/test/ui/issue-76597.rs new file mode 100644 index 0000000000000..879e6b49e9b77 --- /dev/null +++ b/src/test/ui/issue-76597.rs @@ -0,0 +1,7 @@ +fn f( + x: u8 + y: u8, +) {} +//~^^ ERROR: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `y` + +fn main() {} diff --git a/src/test/ui/issue-76597.stderr b/src/test/ui/issue-76597.stderr new file mode 100644 index 0000000000000..163ce61cb18e3 --- /dev/null +++ b/src/test/ui/issue-76597.stderr @@ -0,0 +1,14 @@ +error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `y` + --> $DIR/issue-76597.rs:3:38 + | +LL | ... x: u8 + | - expected one of 7 possible tokens + | ____________________________| + | | +LL | | ... y: u8, +| | | ^ unexpected token +| | | + | help: missing `,` + +error: aborting due to previous error + From bf38fa5517bacd8e4af4aa08f7abdce3627a24f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20Deharbe?= Date: Fri, 11 Sep 2020 17:31:52 +0200 Subject: [PATCH 9/9] repairing broken error message and rustfix application for the new test case --- src/librustc_parse/parser/mod.rs | 6 +++++- src/test/ui/issue-76597.fixed | 11 +++++++++++ src/test/ui/issue-76597.rs | 4 ++++ src/test/ui/issue-76597.stderr | 17 ++++++++--------- 4 files changed, 28 insertions(+), 10 deletions(-) create mode 100644 src/test/ui/issue-76597.fixed diff --git a/src/librustc_parse/parser/mod.rs b/src/librustc_parse/parser/mod.rs index d67ed74bc9976..8803e3add46a8 100644 --- a/src/librustc_parse/parser/mod.rs +++ b/src/librustc_parse/parser/mod.rs @@ -694,9 +694,13 @@ impl<'a> Parser<'a> { Ok(t) => { // Parsed successfully, therefore most probably the code only // misses a separator. + let mut exp_span = self.sess.source_map().next_point(sp); + if self.sess.source_map().is_multiline(exp_span) { + exp_span = sp; + } expect_err .span_suggestion_short( - self.sess.source_map().next_point(sp), + exp_span, &format!("missing `{}`", token_str), token_str, Applicability::MaybeIncorrect, diff --git a/src/test/ui/issue-76597.fixed b/src/test/ui/issue-76597.fixed new file mode 100644 index 0000000000000..2d7a30b8361ad --- /dev/null +++ b/src/test/ui/issue-76597.fixed @@ -0,0 +1,11 @@ +// run-rustfix + +#![allow(dead_code)] +#![allow(unused_variables)] +fn f( + x: u8, + y: u8, +) {} +//~^^ ERROR: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `y` + +fn main() {} diff --git a/src/test/ui/issue-76597.rs b/src/test/ui/issue-76597.rs index 879e6b49e9b77..521b9c64b1c57 100644 --- a/src/test/ui/issue-76597.rs +++ b/src/test/ui/issue-76597.rs @@ -1,3 +1,7 @@ +// run-rustfix + +#![allow(dead_code)] +#![allow(unused_variables)] fn f( x: u8 y: u8, diff --git a/src/test/ui/issue-76597.stderr b/src/test/ui/issue-76597.stderr index 163ce61cb18e3..50b23329f0ceb 100644 --- a/src/test/ui/issue-76597.stderr +++ b/src/test/ui/issue-76597.stderr @@ -1,14 +1,13 @@ error: expected one of `!`, `(`, `)`, `+`, `,`, `::`, or `<`, found `y` - --> $DIR/issue-76597.rs:3:38 + --> $DIR/issue-76597.rs:7:38 | -LL | ... x: u8 - | - expected one of 7 possible tokens - | ____________________________| - | | -LL | | ... y: u8, -| | | ^ unexpected token -| | | - | help: missing `,` +LL | ... x: u8 + | - + | | + | expected one of 7 possible tokens + | help: missing `,` +LL | ... y: u8, + | ^ unexpected token error: aborting due to previous error