From 2232321ac7d03ed8c1d191de0653d1c32db877d6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 28 May 2019 07:43:05 +1000 Subject: [PATCH] Optimize `TyCtxt::adjust_ident`. It's a hot function that returns a 2-tuple, but the hottest call site (`hygienic_eq`) discards the second element. This commit renames `adjust_ident` as `adjust_ident_and_get_scope`, and then introduces a new `adjust_ident` that only computes the first element. This change also avoids the need to pass in an unused `DUMMY_HIR_ID` argument in a couple of places, which is nice. --- src/librustc/ty/mod.rs | 24 +++++++++++++++-------- src/librustc_privacy/lib.rs | 2 +- src/librustc_typeck/astconv.rs | 5 +++-- src/librustc_typeck/check/_match.rs | 2 +- src/librustc_typeck/check/method/probe.rs | 3 ++- src/librustc_typeck/check/mod.rs | 7 ++++--- 6 files changed, 27 insertions(+), 16 deletions(-) diff --git a/src/librustc/ty/mod.rs b/src/librustc/ty/mod.rs index 96bb05cedb6e8..cfe20c5b03fbc 100644 --- a/src/librustc/ty/mod.rs +++ b/src/librustc/ty/mod.rs @@ -3089,20 +3089,28 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> { // comparison fails frequently, and we want to avoid the expensive // `modern()` calls required for the span comparison whenever possible. use_name.name == def_name.name && - self.adjust_ident(use_name, def_parent_def_id, hir::DUMMY_HIR_ID).0.span.ctxt() == - def_name.modern().span.ctxt() + self.adjust_ident(use_name, def_parent_def_id).span.ctxt() == def_name.modern().span.ctxt() } - pub fn adjust_ident(self, mut ident: Ident, scope: DefId, block: hir::HirId) -> (Ident, DefId) { - ident = ident.modern(); - let target_expansion = match scope.krate { + fn expansion_that_defined(self, scope: DefId) -> Mark { + match scope.krate { LOCAL_CRATE => self.hir().definitions().expansion_that_defined(scope.index), _ => Mark::root(), - }; - let scope = match ident.span.adjust(target_expansion) { + } + } + + pub fn adjust_ident(self, mut ident: Ident, scope: DefId) -> Ident { + ident = ident.modern(); + ident.span.adjust(self.expansion_that_defined(scope)); + ident + } + + pub fn adjust_ident_and_get_scope(self, mut ident: Ident, scope: DefId, block: hir::HirId) + -> (Ident, DefId) { + ident = ident.modern(); + let scope = match ident.span.adjust(self.expansion_that_defined(scope)) { Some(actual_expansion) => self.hir().definitions().parent_module_of_macro_def(actual_expansion), - None if block == hir::DUMMY_HIR_ID => DefId::local(CRATE_DEF_INDEX), // Dummy DefId None => self.hir().get_module_parent_by_hir_id(block), }; (ident, scope) diff --git a/src/librustc_privacy/lib.rs b/src/librustc_privacy/lib.rs index 68930533a285c..644cf1a3012f2 100644 --- a/src/librustc_privacy/lib.rs +++ b/src/librustc_privacy/lib.rs @@ -845,7 +845,7 @@ impl<'a, 'tcx> NamePrivacyVisitor<'a, 'tcx> { field: &'tcx ty::FieldDef) { // definition of the field let ident = Ident::new(kw::Invalid, use_ctxt); let current_hir = self.current_item; - let def_id = self.tcx.adjust_ident(ident, def.did, current_hir).1; + let def_id = self.tcx.adjust_ident_and_get_scope(ident, def.did, current_hir).1; if !def.is_enum() && !field.vis.is_accessible_from(def_id, self.tcx) { struct_span_err!(self.tcx.sess, span, E0451, "field `{}` of {} `{}` is private", field.ident, def.variant_descr(), self.tcx.def_path_str(def.did)) diff --git a/src/librustc_typeck/astconv.rs b/src/librustc_typeck/astconv.rs index 5b1a2e29c7642..cd53bdc6ed0a0 100644 --- a/src/librustc_typeck/astconv.rs +++ b/src/librustc_typeck/astconv.rs @@ -903,7 +903,7 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { }?; let (assoc_ident, def_scope) = - tcx.adjust_ident(binding.item_name, candidate.def_id(), hir_ref_id); + tcx.adjust_ident_and_get_scope(binding.item_name, candidate.def_id(), hir_ref_id); let assoc_ty = tcx.associated_items(candidate.def_id()).find(|i| { i.kind == ty::AssocKind::Type && i.ident.modern() == assoc_ident }).expect("missing associated type"); @@ -1433,7 +1433,8 @@ impl<'o, 'gcx: 'tcx, 'tcx> dyn AstConv<'gcx, 'tcx> + 'o { }; let trait_did = bound.def_id(); - let (assoc_ident, def_scope) = tcx.adjust_ident(assoc_ident, trait_did, hir_ref_id); + let (assoc_ident, def_scope) = + tcx.adjust_ident_and_get_scope(assoc_ident, trait_did, hir_ref_id); let item = tcx.associated_items(trait_did).find(|i| { Namespace::from(i.kind) == Namespace::Type && i.ident.modern() == assoc_ident diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index a74a33b7448e2..f43d2bab35215 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -1184,7 +1184,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let mut inexistent_fields = vec![]; // Typecheck each field. for &Spanned { node: ref field, span } in fields { - let ident = tcx.adjust_ident(field.ident, variant.def_id, self.body_id).0; + let ident = tcx.adjust_ident(field.ident, variant.def_id); let field_ty = match used_fields.entry(ident) { Occupied(occupied) => { struct_span_err!(tcx.sess, span, E0025, diff --git a/src/librustc_typeck/check/method/probe.rs b/src/librustc_typeck/check/method/probe.rs index 596ce008099a6..2c20585c5942c 100644 --- a/src/librustc_typeck/check/method/probe.rs +++ b/src/librustc_typeck/check/method/probe.rs @@ -510,7 +510,8 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> { { let is_accessible = if let Some(name) = self.method_name { let item = candidate.item; - let def_scope = self.tcx.adjust_ident(name, item.container.id(), self.body_id).1; + let def_scope = + self.tcx.adjust_ident_and_get_scope(name, item.container.id(), self.body_id).1; item.vis.is_accessible_from(def_scope, self.tcx) } else { true diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index f60ad5547a278..3596a8680ab1f 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -3339,7 +3339,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ty::Adt(base_def, substs) if !base_def.is_enum() => { debug!("struct named {:?}", base_t); let (ident, def_scope) = - self.tcx.adjust_ident(field, base_def.did, self.body_id); + self.tcx.adjust_ident_and_get_scope(field, base_def.did, self.body_id); let fields = &base_def.non_enum_variant().fields; if let Some(index) = fields.iter().position(|f| f.ident.modern() == ident) { let field = &fields[index]; @@ -3510,7 +3510,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { fn available_field_names(&self, variant: &'tcx ty::VariantDef) -> Vec { variant.fields.iter().filter(|field| { - let def_scope = self.tcx.adjust_ident(field.ident, variant.def_id, self.body_id).1; + let def_scope = + self.tcx.adjust_ident_and_get_scope(field.ident, variant.def_id, self.body_id).1; field.vis.is_accessible_from(def_scope, self.tcx) }) .map(|field| field.ident.name) @@ -3628,7 +3629,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Type-check each field. for field in ast_fields { - let ident = tcx.adjust_ident(field.ident, variant.def_id, self.body_id).0; + let ident = tcx.adjust_ident(field.ident, variant.def_id); let field_type = if let Some((i, v_field)) = remaining_fields.remove(&ident) { seen_fields.insert(ident, field.span); self.write_field_index(field.hir_id, i);