From f8d4cdc170bead42db3ffa647318ecf2bd6430e7 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 14 Oct 2019 12:56:18 +0900 Subject: Avoid cloning `Arc<[T]>` into a vec if possible --- crates/ra_hir/src/lib.rs | 1 + crates/ra_hir/src/ty.rs | 36 +++++++++++++++++------------------- crates/ra_hir/src/ty/infer/unify.rs | 23 +++++++++++++++-------- crates/ra_hir/src/ty/lower.rs | 7 +++---- crates/ra_hir/src/util.rs | 19 +++++++++++++++++++ 5 files changed, 55 insertions(+), 31 deletions(-) create mode 100644 crates/ra_hir/src/util.rs (limited to 'crates') diff --git a/crates/ra_hir/src/lib.rs b/crates/ra_hir/src/lib.rs index 9cbd9a8ae..ca261e8f5 100644 --- a/crates/ra_hir/src/lib.rs +++ b/crates/ra_hir/src/lib.rs @@ -51,6 +51,7 @@ mod lang_item; mod generics; mod resolve; pub mod diagnostics; +mod util; mod code_model; diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs index d161735e8..fc4909d11 100644 --- a/crates/ra_hir/src/ty.rs +++ b/crates/ra_hir/src/ty.rs @@ -17,8 +17,8 @@ use std::sync::Arc; use std::{fmt, iter, mem}; use crate::{ - db::HirDatabase, expr::ExprId, type_ref::Mutability, Adt, Crate, DefWithBody, GenericParams, - HasGenericParams, Name, Trait, TypeAlias, + db::HirDatabase, expr::ExprId, type_ref::Mutability, util::make_mut_arc_slice, Adt, Crate, + DefWithBody, GenericParams, HasGenericParams, Name, Trait, TypeAlias, }; use display::{HirDisplay, HirFormatter}; @@ -308,12 +308,11 @@ impl Substs { } pub fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) { - // Without an Arc::make_mut_slice, we can't avoid the clone here: - let mut v: Vec<_> = self.0.iter().cloned().collect(); - for t in &mut v { - t.walk_mut(f); - } - self.0 = v.into(); + make_mut_arc_slice(&mut self.0, |s| { + for t in s { + t.walk_mut(f); + } + }); } pub fn as_single(&self) -> &Ty { @@ -541,12 +540,11 @@ impl TypeWalk for FnSig { } fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) { - // Without an Arc::make_mut_slice, we can't avoid the clone here: - let mut v: Vec<_> = self.params_and_return.iter().cloned().collect(); - for t in &mut v { - t.walk_mut(f); - } - self.params_and_return = v.into(); + make_mut_arc_slice(&mut self.params_and_return, |s| { + for t in s { + t.walk_mut(f); + } + }); } } @@ -756,11 +754,11 @@ impl TypeWalk for Ty { p_ty.parameters.walk_mut(f); } Ty::Dyn(predicates) | Ty::Opaque(predicates) => { - let mut v: Vec<_> = predicates.iter().cloned().collect(); - for p in &mut v { - p.walk_mut(f); - } - *predicates = v.into(); + make_mut_arc_slice(predicates, |s| { + for p in s { + p.walk_mut(f); + } + }); } Ty::Param { .. } | Ty::Bound(_) | Ty::Infer(_) | Ty::Unknown => {} } diff --git a/crates/ra_hir/src/ty/infer/unify.rs b/crates/ra_hir/src/ty/infer/unify.rs index d161aa6b3..5e86ed260 100644 --- a/crates/ra_hir/src/ty/infer/unify.rs +++ b/crates/ra_hir/src/ty/infer/unify.rs @@ -6,6 +6,7 @@ use crate::ty::{ Canonical, InEnvironment, InferTy, ProjectionPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeWalk, }; +use crate::util::make_mut_arc_slice; impl<'a, D: HirDatabase> InferenceContext<'a, D> { pub(super) fn canonicalizer<'b>(&'b mut self) -> Canonicalizer<'a, 'b, D> @@ -74,10 +75,13 @@ where }) } - fn do_canonicalize_trait_ref(&mut self, trait_ref: TraitRef) -> TraitRef { - let substs = - trait_ref.substs.iter().map(|ty| self.do_canonicalize_ty(ty.clone())).collect(); - TraitRef { trait_: trait_ref.trait_, substs: Substs(substs) } + fn do_canonicalize_trait_ref(&mut self, mut trait_ref: TraitRef) -> TraitRef { + make_mut_arc_slice(&mut trait_ref.substs.0, |tys| { + for ty in tys { + *ty = self.do_canonicalize_ty(ty.clone()); + } + }); + trait_ref } fn into_canonicalized(self, result: T) -> Canonicalized { @@ -87,10 +91,13 @@ where } } - fn do_canonicalize_projection_ty(&mut self, projection_ty: ProjectionTy) -> ProjectionTy { - let params = - projection_ty.parameters.iter().map(|ty| self.do_canonicalize_ty(ty.clone())).collect(); - ProjectionTy { associated_ty: projection_ty.associated_ty, parameters: Substs(params) } + fn do_canonicalize_projection_ty(&mut self, mut projection_ty: ProjectionTy) -> ProjectionTy { + make_mut_arc_slice(&mut projection_ty.parameters.0, |params| { + for ty in params { + *ty = self.do_canonicalize_ty(ty.clone()); + } + }); + projection_ty } fn do_canonicalize_projection_predicate( diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index a604c02e2..003aa9bab 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs @@ -392,10 +392,9 @@ impl TraitRef { ) -> Self { let mut substs = TraitRef::substs_from_path(db, resolver, segment, resolved); if let Some(self_ty) = explicit_self_ty { - // FIXME this could be nicer - let mut substs_vec = substs.0.to_vec(); - substs_vec[0] = self_ty; - substs.0 = substs_vec.into(); + crate::util::make_mut_arc_slice(&mut substs.0, |substs| { + substs[0] = self_ty; + }); } TraitRef { trait_: resolved, substs } } diff --git a/crates/ra_hir/src/util.rs b/crates/ra_hir/src/util.rs new file mode 100644 index 000000000..46f423c91 --- /dev/null +++ b/crates/ra_hir/src/util.rs @@ -0,0 +1,19 @@ +//! Internal utility functions. + +use std::sync::Arc; + +/// Helper for mutating `Arc<[T]>` (i.e. `Arc::make_mut` for Arc slices). +/// The underlying values are cloned if there are other strong references. +pub(crate) fn make_mut_arc_slice( + a: &mut Arc<[T]>, + f: impl FnOnce(&mut [T]) -> R, +) -> R { + if let Some(s) = Arc::get_mut(a) { + f(s) + } else { + let mut v = a.to_vec(); + let r = f(&mut v); + *a = Arc::from(v); + r + } +} -- cgit v1.2.3 From 965ca0d271bdb467ec1f7fe309094aff8cac6aaa Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 14 Oct 2019 13:06:05 +0900 Subject: `.collect()` directly into `Arc<[T]>` --- crates/ra_hir/src/ty.rs | 6 ++---- crates/ra_hir/src/ty/lower.rs | 32 ++++++++++++++------------------ crates/ra_hir/src/ty/traits.rs | 2 +- crates/ra_hir/src/ty/traits/chalk.rs | 3 +-- 4 files changed, 18 insertions(+), 25 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs index fc4909d11..bf7445ea3 100644 --- a/crates/ra_hir/src/ty.rs +++ b/crates/ra_hir/src/ty.rs @@ -329,8 +329,7 @@ impl Substs { .params_including_parent() .into_iter() .map(|p| Ty::Param { idx: p.idx, name: p.name.clone() }) - .collect::>() - .into(), + .collect(), ) } @@ -341,8 +340,7 @@ impl Substs { .params_including_parent() .into_iter() .map(|p| Ty::Bound(p.idx)) - .collect::>() - .into(), + .collect(), ) } diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index 003aa9bab..2f9a2ee05 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs @@ -31,11 +31,11 @@ impl Ty { match type_ref { TypeRef::Never => Ty::simple(TypeCtor::Never), TypeRef::Tuple(inner) => { - let inner_tys = - inner.iter().map(|tr| Ty::from_hir(db, resolver, tr)).collect::>(); + let inner_tys: Arc<[Ty]> = + inner.iter().map(|tr| Ty::from_hir(db, resolver, tr)).collect(); Ty::apply( TypeCtor::Tuple { cardinality: inner_tys.len() as u16 }, - Substs(inner_tys.into()), + Substs(inner_tys), ) } TypeRef::Path(path) => Ty::from_hir_path(db, resolver, path), @@ -57,9 +57,7 @@ impl Ty { } TypeRef::Placeholder => Ty::Unknown, TypeRef::Fn(params) => { - let inner_tys = - params.iter().map(|tr| Ty::from_hir(db, resolver, tr)).collect::>(); - let sig = Substs(inner_tys.into()); + let sig = Substs(params.iter().map(|tr| Ty::from_hir(db, resolver, tr)).collect()); Ty::apply(TypeCtor::FnPtr { num_args: sig.len() as u16 - 1 }, sig) } TypeRef::DynTrait(bounds) => { @@ -69,8 +67,8 @@ impl Ty { .flat_map(|b| { GenericPredicate::from_type_bound(db, resolver, b, self_ty.clone()) }) - .collect::>(); - Ty::Dyn(predicates.into()) + .collect(); + Ty::Dyn(predicates) } TypeRef::ImplTrait(bounds) => { let self_ty = Ty::Bound(0); @@ -79,8 +77,8 @@ impl Ty { .flat_map(|b| { GenericPredicate::from_type_bound(db, resolver, b, self_ty.clone()) }) - .collect::>(); - Ty::Opaque(predicates.into()) + .collect(); + Ty::Opaque(predicates) } TypeRef::Error => Ty::Unknown, } @@ -557,13 +555,12 @@ pub(crate) fn generic_predicates_for_param_query( param_idx: u32, ) -> Arc<[GenericPredicate]> { let resolver = def.resolver(db); - let predicates = resolver + resolver .where_predicates_in_scope() // we have to filter out all other predicates *first*, before attempting to lower them .filter(|pred| Ty::from_hir_only_param(db, &resolver, &pred.type_ref) == Some(param_idx)) .flat_map(|pred| GenericPredicate::from_where_predicate(db, &resolver, pred)) - .collect::>(); - predicates.into() + .collect() } pub(crate) fn trait_env( @@ -584,11 +581,10 @@ pub(crate) fn generic_predicates_query( def: GenericDef, ) -> Arc<[GenericPredicate]> { let resolver = def.resolver(db); - let predicates = resolver + resolver .where_predicates_in_scope() .flat_map(|pred| GenericPredicate::from_where_predicate(db, &resolver, pred)) - .collect::>(); - predicates.into() + .collect() } /// Resolve the default type params from generics @@ -602,9 +598,9 @@ pub(crate) fn generic_defaults_query(db: &impl HirDatabase, def: GenericDef) -> .map(|p| { p.default.as_ref().map_or(Ty::Unknown, |path| Ty::from_hir_path(db, &resolver, path)) }) - .collect::>(); + .collect(); - Substs(defaults.into()) + Substs(defaults) } fn fn_sig_for_fn(db: &impl HirDatabase, def: Function) -> FnSig { diff --git a/crates/ra_hir/src/ty/traits.rs b/crates/ra_hir/src/ty/traits.rs index b0f67ae50..0cb5c3798 100644 --- a/crates/ra_hir/src/ty/traits.rs +++ b/crates/ra_hir/src/ty/traits.rs @@ -89,7 +89,7 @@ pub(crate) fn impls_for_trait_query( } let crate_impl_blocks = db.impls_in_crate(krate); impls.extend(crate_impl_blocks.lookup_impl_blocks_for_trait(trait_)); - impls.into_iter().collect::>().into() + impls.into_iter().collect() } /// A set of clauses that we assume to be true. E.g. if we are inside this function: diff --git a/crates/ra_hir/src/ty/traits/chalk.rs b/crates/ra_hir/src/ty/traits/chalk.rs index 9168de709..00aaf65d9 100644 --- a/crates/ra_hir/src/ty/traits/chalk.rs +++ b/crates/ra_hir/src/ty/traits/chalk.rs @@ -126,8 +126,7 @@ impl ToChalk for Substs { chalk_ir::Parameter(chalk_ir::ParameterKind::Ty(ty)) => from_chalk(db, ty), chalk_ir::Parameter(chalk_ir::ParameterKind::Lifetime(_)) => unimplemented!(), }) - .collect::>() - .into(); + .collect(); Substs(tys) } } -- cgit v1.2.3 From b462eb96b867cd38c60fb3d94ffd7688cec70f21 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 14 Oct 2019 17:21:38 +0900 Subject: import make_mut_arc_slice --- crates/ra_hir/src/ty/lower.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'crates') diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index 2f9a2ee05..639518f27 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs @@ -22,6 +22,7 @@ use crate::{ resolve::{Resolver, TypeNs}, ty::Adt, type_ref::{TypeBound, TypeRef}, + util::make_mut_arc_slice, BuiltinType, Const, Enum, EnumVariant, Function, ModuleDef, Path, Static, Struct, StructField, Trait, TypeAlias, Union, }; @@ -390,7 +391,7 @@ impl TraitRef { ) -> Self { let mut substs = TraitRef::substs_from_path(db, resolver, segment, resolved); if let Some(self_ty) = explicit_self_ty { - crate::util::make_mut_arc_slice(&mut substs.0, |substs| { + make_mut_arc_slice(&mut substs.0, |substs| { substs[0] = self_ty; }); } -- cgit v1.2.3 From 3a55b5bf01ddc581a3f00fa56db725db93a131c6 Mon Sep 17 00:00:00 2001 From: Shotaro Yamada Date: Mon, 14 Oct 2019 19:50:12 +0900 Subject: make_mut_slice --- crates/ra_hir/src/ty.rs | 26 ++++++++++---------------- crates/ra_hir/src/ty/infer/unify.rs | 18 +++++++----------- crates/ra_hir/src/ty/lower.rs | 6 ++---- crates/ra_hir/src/util.rs | 15 ++++----------- 4 files changed, 23 insertions(+), 42 deletions(-) (limited to 'crates') diff --git a/crates/ra_hir/src/ty.rs b/crates/ra_hir/src/ty.rs index bf7445ea3..cc9746f6d 100644 --- a/crates/ra_hir/src/ty.rs +++ b/crates/ra_hir/src/ty.rs @@ -17,7 +17,7 @@ use std::sync::Arc; use std::{fmt, iter, mem}; use crate::{ - db::HirDatabase, expr::ExprId, type_ref::Mutability, util::make_mut_arc_slice, Adt, Crate, + db::HirDatabase, expr::ExprId, type_ref::Mutability, util::make_mut_slice, Adt, Crate, DefWithBody, GenericParams, HasGenericParams, Name, Trait, TypeAlias, }; use display::{HirDisplay, HirFormatter}; @@ -308,11 +308,9 @@ impl Substs { } pub fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) { - make_mut_arc_slice(&mut self.0, |s| { - for t in s { - t.walk_mut(f); - } - }); + for t in make_mut_slice(&mut self.0) { + t.walk_mut(f); + } } pub fn as_single(&self) -> &Ty { @@ -538,11 +536,9 @@ impl TypeWalk for FnSig { } fn walk_mut(&mut self, f: &mut impl FnMut(&mut Ty)) { - make_mut_arc_slice(&mut self.params_and_return, |s| { - for t in s { - t.walk_mut(f); - } - }); + for t in make_mut_slice(&mut self.params_and_return) { + t.walk_mut(f); + } } } @@ -752,11 +748,9 @@ impl TypeWalk for Ty { p_ty.parameters.walk_mut(f); } Ty::Dyn(predicates) | Ty::Opaque(predicates) => { - make_mut_arc_slice(predicates, |s| { - for p in s { - p.walk_mut(f); - } - }); + for p in make_mut_slice(predicates) { + p.walk_mut(f); + } } Ty::Param { .. } | Ty::Bound(_) | Ty::Infer(_) | Ty::Unknown => {} } diff --git a/crates/ra_hir/src/ty/infer/unify.rs b/crates/ra_hir/src/ty/infer/unify.rs index 5e86ed260..014c7981f 100644 --- a/crates/ra_hir/src/ty/infer/unify.rs +++ b/crates/ra_hir/src/ty/infer/unify.rs @@ -6,7 +6,7 @@ use crate::ty::{ Canonical, InEnvironment, InferTy, ProjectionPredicate, ProjectionTy, Substs, TraitRef, Ty, TypeWalk, }; -use crate::util::make_mut_arc_slice; +use crate::util::make_mut_slice; impl<'a, D: HirDatabase> InferenceContext<'a, D> { pub(super) fn canonicalizer<'b>(&'b mut self) -> Canonicalizer<'a, 'b, D> @@ -76,11 +76,9 @@ where } fn do_canonicalize_trait_ref(&mut self, mut trait_ref: TraitRef) -> TraitRef { - make_mut_arc_slice(&mut trait_ref.substs.0, |tys| { - for ty in tys { - *ty = self.do_canonicalize_ty(ty.clone()); - } - }); + for ty in make_mut_slice(&mut trait_ref.substs.0) { + *ty = self.do_canonicalize_ty(ty.clone()); + } trait_ref } @@ -92,11 +90,9 @@ where } fn do_canonicalize_projection_ty(&mut self, mut projection_ty: ProjectionTy) -> ProjectionTy { - make_mut_arc_slice(&mut projection_ty.parameters.0, |params| { - for ty in params { - *ty = self.do_canonicalize_ty(ty.clone()); - } - }); + for ty in make_mut_slice(&mut projection_ty.parameters.0) { + *ty = self.do_canonicalize_ty(ty.clone()); + } projection_ty } diff --git a/crates/ra_hir/src/ty/lower.rs b/crates/ra_hir/src/ty/lower.rs index 639518f27..366556134 100644 --- a/crates/ra_hir/src/ty/lower.rs +++ b/crates/ra_hir/src/ty/lower.rs @@ -22,7 +22,7 @@ use crate::{ resolve::{Resolver, TypeNs}, ty::Adt, type_ref::{TypeBound, TypeRef}, - util::make_mut_arc_slice, + util::make_mut_slice, BuiltinType, Const, Enum, EnumVariant, Function, ModuleDef, Path, Static, Struct, StructField, Trait, TypeAlias, Union, }; @@ -391,9 +391,7 @@ impl TraitRef { ) -> Self { let mut substs = TraitRef::substs_from_path(db, resolver, segment, resolved); if let Some(self_ty) = explicit_self_ty { - make_mut_arc_slice(&mut substs.0, |substs| { - substs[0] = self_ty; - }); + make_mut_slice(&mut substs.0)[0] = self_ty; } TraitRef { trait_: resolved, substs } } diff --git a/crates/ra_hir/src/util.rs b/crates/ra_hir/src/util.rs index 46f423c91..0095ee45d 100644 --- a/crates/ra_hir/src/util.rs +++ b/crates/ra_hir/src/util.rs @@ -4,16 +4,9 @@ use std::sync::Arc; /// Helper for mutating `Arc<[T]>` (i.e. `Arc::make_mut` for Arc slices). /// The underlying values are cloned if there are other strong references. -pub(crate) fn make_mut_arc_slice( - a: &mut Arc<[T]>, - f: impl FnOnce(&mut [T]) -> R, -) -> R { - if let Some(s) = Arc::get_mut(a) { - f(s) - } else { - let mut v = a.to_vec(); - let r = f(&mut v); - *a = Arc::from(v); - r +pub(crate) fn make_mut_slice(a: &mut Arc<[T]>) -> &mut [T] { + if Arc::get_mut(a).is_none() { + *a = a.iter().cloned().collect(); } + Arc::get_mut(a).unwrap() } -- cgit v1.2.3