From d7795d302adbb8c1547c952cd0d04a7f9ca29262 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Thu, 15 Jul 2021 22:19:39 +0200 Subject: [PATCH 1/4] Do not store visibility in *ItemRef. --- compiler/rustc_ast_lowering/src/expr.rs | 1 - compiler/rustc_ast_lowering/src/item.rs | 36 +++++++------------ compiler/rustc_ast_lowering/src/path.rs | 13 ++----- compiler/rustc_hir/src/arena.rs | 4 +-- compiler/rustc_hir/src/hir.rs | 10 +++--- compiler/rustc_hir/src/intravisit.rs | 14 ++++---- compiler/rustc_metadata/src/native_libs.rs | 4 +-- .../rustc_middle/src/hir/map/collector.rs | 8 ++--- compiler/rustc_passes/src/hir_id_validator.rs | 4 +-- compiler/rustc_privacy/src/lib.rs | 11 ++++-- .../rustc_trait_selection/src/traits/wf.rs | 2 +- compiler/rustc_ty_utils/src/ty.rs | 2 +- compiler/rustc_typeck/src/check/check.rs | 2 +- .../src/coherence/inherent_impls.rs | 2 +- compiler/rustc_typeck/src/impl_wf_check.rs | 4 +-- .../clippy_lints/src/fallible_impl_from.rs | 2 +- 16 files changed, 49 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index ac7145bed78..ddb00d17c0a 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -55,7 +55,6 @@ impl<'hir> LoweringContext<'_, 'hir> { 0, ParenthesizedGenericArgs::Err, ImplTraitContext::disallowed(), - None, )); let args = self.lower_exprs(args); hir::ExprKind::MethodCall( diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index 980036c662a..b18e5733115 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -188,7 +188,7 @@ impl<'hir> LoweringContext<'_, 'hir> { pub fn lower_item(&mut self, i: &Item) -> hir::Item<'hir> { let mut ident = i.ident; - let mut vis = self.lower_visibility(&i.vis, None); + let mut vis = self.lower_visibility(&i.vis); let hir_id = self.lower_node_id(i.id); let attrs = self.lower_attrs(hir_id, &i.attrs); let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, &mut vis, &i.kind); @@ -493,7 +493,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self.with_hir_id_owner(new_node_id, |this| { let res = this.lower_res(res); - let path = this.lower_path_extra(res, &path, ParamMode::Explicit, None); + let path = this.lower_path_extra(res, &path, ParamMode::Explicit); let kind = hir::ItemKind::Use(path, hir::UseKind::Single); let vis = this.rebuild_vis(&vis); if let Some(attrs) = attrs { @@ -510,7 +510,7 @@ impl<'hir> LoweringContext<'_, 'hir> { }); } - let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit, None); + let path = self.lower_path_extra(ret_res, &path, ParamMode::Explicit); hir::ItemKind::Use(path, hir::UseKind::Single) } UseTreeKind::Glob => { @@ -610,7 +610,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let res = self.expect_full_res_from_use(id).next().unwrap_or(Res::Err); let res = self.lower_res(res); - let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit, None); + let path = self.lower_path_extra(res, &prefix, ParamMode::Explicit); hir::ItemKind::Use(path, hir::UseKind::ListStem) } } @@ -679,17 +679,16 @@ impl<'hir> LoweringContext<'_, 'hir> { ForeignItemKind::TyAlias(..) => hir::ForeignItemKind::Type, ForeignItemKind::MacCall(_) => panic!("macro shouldn't exist here"), }, - vis: self.lower_visibility(&i.vis, None), + vis: self.lower_visibility(&i.vis), span: self.lower_span(i.span), } } - fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef<'hir> { + fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef { hir::ForeignItemRef { id: hir::ForeignItemId { def_id: self.allocate_hir_id_counter(i.id) }, ident: self.lower_ident(i.ident), span: self.lower_span(i.span), - vis: self.lower_visibility(&i.vis, Some(i.id)), } } @@ -757,7 +756,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // FIXME(jseyfried): positional field hygiene. None => Ident::new(sym::integer(index), self.lower_span(f.span)), }, - vis: self.lower_visibility(&f.vis, None), + vis: self.lower_visibility(&f.vis), ty, } } @@ -899,14 +898,14 @@ impl<'hir> LoweringContext<'_, 'hir> { def_id: hir_id.expect_owner(), ident: self.lower_ident(i.ident), generics, - vis: self.lower_visibility(&i.vis, None), + vis: self.lower_visibility(&i.vis), defaultness, kind, span: self.lower_span(i.span), } } - fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef<'hir> { + fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef { // Since `default impl` is not yet implemented, this is always true in impls. let has_value = true; let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value); @@ -914,7 +913,6 @@ impl<'hir> LoweringContext<'_, 'hir> { id: hir::ImplItemId { def_id: self.allocate_hir_id_counter(i.id) }, ident: self.lower_ident(i.ident), span: self.lower_span(i.span), - vis: self.lower_visibility(&i.vis, Some(i.id)), defaultness, kind: match &i.kind { AssocItemKind::Const(..) => hir::AssocItemKind::Const, @@ -932,25 +930,15 @@ impl<'hir> LoweringContext<'_, 'hir> { /// lowered. This can happen during `lower_impl_item_ref()` where we need to /// lower a `Visibility` value although we haven't lowered the owning /// `ImplItem` in question yet. - fn lower_visibility( - &mut self, - v: &Visibility, - explicit_owner: Option, - ) -> hir::Visibility<'hir> { + fn lower_visibility(&mut self, v: &Visibility) -> hir::Visibility<'hir> { let node = match v.kind { VisibilityKind::Public => hir::VisibilityKind::Public, VisibilityKind::Crate(sugar) => hir::VisibilityKind::Crate(sugar), VisibilityKind::Restricted { ref path, id } => { debug!("lower_visibility: restricted path id = {:?}", id); - let lowered_id = if let Some(owner) = explicit_owner { - self.lower_node_id_with_owner(id, owner) - } else { - self.lower_node_id(id) - }; - let res = self.expect_full_res(id); - let res = self.lower_res(res); + let lowered_id = self.lower_node_id(id); hir::VisibilityKind::Restricted { - path: self.lower_path_extra(res, path, ParamMode::Explicit, explicit_owner), + path: self.lower_path(id, path, ParamMode::Explicit), hir_id: lowered_id, } } diff --git a/compiler/rustc_ast_lowering/src/path.rs b/compiler/rustc_ast_lowering/src/path.rs index 90a22b5c209..929f427484d 100644 --- a/compiler/rustc_ast_lowering/src/path.rs +++ b/compiler/rustc_ast_lowering/src/path.rs @@ -99,7 +99,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { num_lifetimes, parenthesized_generic_args, itctx.reborrow(), - None, ) }, )), @@ -147,7 +146,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { 0, ParenthesizedGenericArgs::Err, itctx.reborrow(), - None, )); let qpath = hir::QPath::TypeRelative(ty, hir_segment); @@ -178,7 +176,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { res: Res, p: &Path, param_mode: ParamMode, - explicit_owner: Option, ) -> &'hir hir::Path<'hir> { self.arena.alloc(hir::Path { res, @@ -190,7 +187,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { 0, ParenthesizedGenericArgs::Err, ImplTraitContext::disallowed(), - explicit_owner, ) })), span: self.lower_span(p.span), @@ -205,7 +201,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ) -> &'hir hir::Path<'hir> { let res = self.expect_full_res(id); let res = self.lower_res(res); - self.lower_path_extra(res, p, param_mode, None) + self.lower_path_extra(res, p, param_mode) } crate fn lower_path_segment( @@ -216,7 +212,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { expected_lifetimes: usize, parenthesized_generic_args: ParenthesizedGenericArgs, itctx: ImplTraitContext<'_, 'hir>, - explicit_owner: Option, ) -> hir::PathSegment<'hir> { debug!( "path_span: {:?}, lower_path_segment(segment: {:?}, expected_lifetimes: {:?})", @@ -354,11 +349,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } let res = self.expect_full_res(segment.id); - let id = if let Some(owner) = explicit_owner { - self.lower_node_id_with_owner(segment.id, owner) - } else { - self.lower_node_id(segment.id) - }; + let id = self.lower_node_id(segment.id); debug!( "lower_path_segment: ident={:?} original-id={:?} new-id={:?}", segment.ident, segment.id, id, diff --git a/compiler/rustc_hir/src/arena.rs b/compiler/rustc_hir/src/arena.rs index 3e8b98e9f54..f07e52e04da 100644 --- a/compiler/rustc_hir/src/arena.rs +++ b/compiler/rustc_hir/src/arena.rs @@ -28,9 +28,9 @@ macro_rules! arena_types { [] pat_field: rustc_hir::PatField<$tcx>, [] fn_decl: rustc_hir::FnDecl<$tcx>, [] foreign_item: rustc_hir::ForeignItem<$tcx>, - [few] foreign_item_ref: rustc_hir::ForeignItemRef<$tcx>, + [few] foreign_item_ref: rustc_hir::ForeignItemRef, [] impl_item: rustc_hir::ImplItem<$tcx>, - [] impl_item_ref: rustc_hir::ImplItemRef<$tcx>, + [] impl_item_ref: rustc_hir::ImplItemRef, [] item: rustc_hir::Item<$tcx>, [few] inline_asm: rustc_hir::InlineAsm<$tcx>, [few] llvm_inline_asm: rustc_hir::LlvmInlineAsm<$tcx>, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index f5fc693ce25..05b652fd5af 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -2745,7 +2745,7 @@ pub enum ItemKind<'hir> { /// A module. Mod(Mod<'hir>), /// An external module, e.g. `extern { .. }`. - ForeignMod { abi: Abi, items: &'hir [ForeignItemRef<'hir>] }, + ForeignMod { abi: Abi, items: &'hir [ForeignItemRef] }, /// Module-level inline assembly (from `global_asm!`). GlobalAsm(&'hir InlineAsm<'hir>), /// A type alias, e.g., `type Foo = Bar`. @@ -2782,7 +2782,7 @@ pub struct Impl<'hir> { pub of_trait: Option>, pub self_ty: &'hir Ty<'hir>, - pub items: &'hir [ImplItemRef<'hir>], + pub items: &'hir [ImplItemRef], } impl ItemKind<'_> { @@ -2846,13 +2846,12 @@ pub struct TraitItemRef { /// passes to find the impl they want without loading the ID (which /// means fewer edges in the incremental compilation graph). #[derive(Debug, HashStable_Generic)] -pub struct ImplItemRef<'hir> { +pub struct ImplItemRef { pub id: ImplItemId, #[stable_hasher(project(name))] pub ident: Ident, pub kind: AssocItemKind, pub span: Span, - pub vis: Visibility<'hir>, pub defaultness: Defaultness, } @@ -2886,12 +2885,11 @@ impl ForeignItemId { /// passes to find the impl they want without loading the ID (which /// means fewer edges in the incremental compilation graph). #[derive(Debug, HashStable_Generic)] -pub struct ForeignItemRef<'hir> { +pub struct ForeignItemRef { pub id: ForeignItemId, #[stable_hasher(project(name))] pub ident: Ident, pub span: Span, - pub vis: Visibility<'hir>, } #[derive(Debug)] diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 137782a6dc7..1ac2625dd47 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -392,10 +392,10 @@ pub trait Visitor<'v>: Sized { fn visit_impl_item(&mut self, ii: &'v ImplItem<'v>) { walk_impl_item(self, ii) } - fn visit_foreign_item_ref(&mut self, ii: &'v ForeignItemRef<'v>) { + fn visit_foreign_item_ref(&mut self, ii: &'v ForeignItemRef) { walk_foreign_item_ref(self, ii) } - fn visit_impl_item_ref(&mut self, ii: &'v ImplItemRef<'v>) { + fn visit_impl_item_ref(&mut self, ii: &'v ImplItemRef) { walk_impl_item_ref(self, ii) } fn visit_trait_ref(&mut self, t: &'v TraitRef<'v>) { @@ -1042,22 +1042,20 @@ pub fn walk_impl_item<'v, V: Visitor<'v>>(visitor: &mut V, impl_item: &'v ImplIt pub fn walk_foreign_item_ref<'v, V: Visitor<'v>>( visitor: &mut V, - foreign_item_ref: &'v ForeignItemRef<'v>, + foreign_item_ref: &'v ForeignItemRef, ) { // N.B., deliberately force a compilation error if/when new fields are added. - let ForeignItemRef { id, ident, span: _, ref vis } = *foreign_item_ref; + let ForeignItemRef { id, ident, span: _ } = *foreign_item_ref; visitor.visit_nested_foreign_item(id); visitor.visit_ident(ident); - visitor.visit_vis(vis); } -pub fn walk_impl_item_ref<'v, V: Visitor<'v>>(visitor: &mut V, impl_item_ref: &'v ImplItemRef<'v>) { +pub fn walk_impl_item_ref<'v, V: Visitor<'v>>(visitor: &mut V, impl_item_ref: &'v ImplItemRef) { // N.B., deliberately force a compilation error if/when new fields are added. - let ImplItemRef { id, ident, ref kind, span: _, ref vis, ref defaultness } = *impl_item_ref; + let ImplItemRef { id, ident, ref kind, span: _, ref defaultness } = *impl_item_ref; visitor.visit_nested_impl_item(id); visitor.visit_ident(ident); visitor.visit_associated_item_kind(kind); - visitor.visit_vis(vis); visitor.visit_defaultness(defaultness); } diff --git a/compiler/rustc_metadata/src/native_libs.rs b/compiler/rustc_metadata/src/native_libs.rs index 5f0d8c46f20..5ad55dbf5c8 100644 --- a/compiler/rustc_metadata/src/native_libs.rs +++ b/compiler/rustc_metadata/src/native_libs.rs @@ -382,7 +382,7 @@ impl Collector<'tcx> { } } - fn i686_arg_list_size(&self, item: &hir::ForeignItemRef<'_>) -> usize { + fn i686_arg_list_size(&self, item: &hir::ForeignItemRef) -> usize { let argument_types: &List> = self.tcx.erase_late_bound_regions( self.tcx .type_of(item.id.def_id) @@ -406,7 +406,7 @@ impl Collector<'tcx> { .sum() } - fn build_dll_import(&self, abi: Abi, item: &hir::ForeignItemRef<'_>) -> DllImport { + fn build_dll_import(&self, abi: Abi, item: &hir::ForeignItemRef) -> DllImport { let calling_convention = if self.tcx.sess.target.arch == "x86" { match abi { Abi::C { .. } | Abi::Cdecl => DllCallingConvention::C, diff --git a/compiler/rustc_middle/src/hir/map/collector.rs b/compiler/rustc_middle/src/hir/map/collector.rs index 082948eba41..5ecb1c9b0ff 100644 --- a/compiler/rustc_middle/src/hir/map/collector.rs +++ b/compiler/rustc_middle/src/hir/map/collector.rs @@ -413,18 +413,18 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> { self.visit_nested_trait_item(id); } - fn visit_impl_item_ref(&mut self, ii: &'hir ImplItemRef<'hir>) { + fn visit_impl_item_ref(&mut self, ii: &'hir ImplItemRef) { // Do not visit the duplicate information in ImplItemRef. We want to // map the actual nodes, not the duplicate ones in the *Ref. - let ImplItemRef { id, ident: _, kind: _, span: _, vis: _, defaultness: _ } = *ii; + let ImplItemRef { id, ident: _, kind: _, span: _, defaultness: _ } = *ii; self.visit_nested_impl_item(id); } - fn visit_foreign_item_ref(&mut self, fi: &'hir ForeignItemRef<'hir>) { + fn visit_foreign_item_ref(&mut self, fi: &'hir ForeignItemRef) { // Do not visit the duplicate information in ForeignItemRef. We want to // map the actual nodes, not the duplicate ones in the *Ref. - let ForeignItemRef { id, ident: _, span: _, vis: _ } = *fi; + let ForeignItemRef { id, ident: _, span: _ } = *fi; self.visit_nested_foreign_item(id); } diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index eff1096c855..0e60ca9f900 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -163,14 +163,14 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { self.hir_ids_seen.insert(hir_id.local_id); } - fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef<'hir>) { + fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) { // Explicitly do nothing here. ImplItemRefs contain hir::Visibility // values that actually belong to an ImplItem instead of the ItemKind::Impl // we are currently in. So for those it's correct that they have a // different owner. } - fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef<'hir>) { + fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef) { // Explicitly do nothing here. ForeignItemRefs contain hir::Visibility // values that actually belong to an ForeignItem instead of the ItemKind::ForeignMod // we are currently in. So for those it's correct that they have a diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index 391e4305423..e14f758ddae 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -743,7 +743,9 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { } hir::ItemKind::Impl(ref impl_) => { for impl_item_ref in impl_.items { - if impl_.of_trait.is_some() || impl_item_ref.vis.node.is_pub() { + if impl_.of_trait.is_some() + || self.tcx.visibility(impl_item_ref.id.def_id) == ty::Visibility::Public + { self.update(impl_item_ref.id.def_id, item_level); } } @@ -768,7 +770,7 @@ impl Visitor<'tcx> for EmbargoVisitor<'tcx> { } hir::ItemKind::ForeignMod { items, .. } => { for foreign_item in items { - if foreign_item.vis.node.is_pub() { + if self.tcx.visibility(foreign_item.id.def_id) == ty::Visibility::Public { self.update(foreign_item.id.def_id, item_level); } } @@ -1678,7 +1680,10 @@ impl<'a, 'tcx> Visitor<'tcx> for ObsoleteVisiblePrivateTypesVisitor<'a, 'tcx> { // methods will be visible as `Public::foo`. let mut found_pub_static = false; for impl_item_ref in impl_.items { - if self.item_is_public(impl_item_ref.id.def_id, &impl_item_ref.vis) { + if self.access_levels.is_reachable(impl_item_ref.id.def_id) + || self.tcx.visibility(impl_item_ref.id.def_id) + == ty::Visibility::Public + { let impl_item = self.tcx.hir().impl_item(impl_item_ref.id); match impl_item_ref.kind { AssocItemKind::Const => { diff --git a/compiler/rustc_trait_selection/src/traits/wf.rs b/compiler/rustc_trait_selection/src/traits/wf.rs index 75307f13563..611ff26d652 100644 --- a/compiler/rustc_trait_selection/src/traits/wf.rs +++ b/compiler/rustc_trait_selection/src/traits/wf.rs @@ -209,7 +209,7 @@ fn extend_cause_with_original_assoc_item_obligation<'tcx>( _ => return, }; let fix_span = - |impl_item_ref: &hir::ImplItemRef<'_>| match tcx.hir().impl_item(impl_item_ref.id).kind { + |impl_item_ref: &hir::ImplItemRef| match tcx.hir().impl_item(impl_item_ref.id).kind { hir::ImplItemKind::Const(ty, _) | hir::ImplItemKind::TyAlias(ty) => ty.span, _ => impl_item_ref.span, }; diff --git a/compiler/rustc_ty_utils/src/ty.rs b/compiler/rustc_ty_utils/src/ty.rs index 27ad7bf4c2d..3d3b2743700 100644 --- a/compiler/rustc_ty_utils/src/ty.rs +++ b/compiler/rustc_ty_utils/src/ty.rs @@ -100,7 +100,7 @@ fn associated_item_from_trait_item_ref( fn associated_item_from_impl_item_ref( tcx: TyCtxt<'_>, parent_def_id: LocalDefId, - impl_item_ref: &hir::ImplItemRef<'_>, + impl_item_ref: &hir::ImplItemRef, ) -> ty::AssocItem { let def_id = impl_item_ref.id.def_id; let (kind, has_self) = match impl_item_ref.kind { diff --git a/compiler/rustc_typeck/src/check/check.rs b/compiler/rustc_typeck/src/check/check.rs index 54e4eb47688..d6c59312c0b 100644 --- a/compiler/rustc_typeck/src/check/check.rs +++ b/compiler/rustc_typeck/src/check/check.rs @@ -906,7 +906,7 @@ pub(super) fn check_impl_items_against_trait<'tcx>( full_impl_span: Span, impl_id: LocalDefId, impl_trait_ref: ty::TraitRef<'tcx>, - impl_item_refs: &[hir::ImplItemRef<'_>], + impl_item_refs: &[hir::ImplItemRef], ) { // If the trait reference itself is erroneous (so the compilation is going // to fail), skip checking the items here -- the `impl_item` table in `tcx` diff --git a/compiler/rustc_typeck/src/coherence/inherent_impls.rs b/compiler/rustc_typeck/src/coherence/inherent_impls.rs index c7be9e21235..f3fe09ac003 100644 --- a/compiler/rustc_typeck/src/coherence/inherent_impls.rs +++ b/compiler/rustc_typeck/src/coherence/inherent_impls.rs @@ -401,7 +401,7 @@ impl InherentCollect<'tcx> { lang: &str, ty: &str, span: Span, - assoc_items: &[hir::ImplItemRef<'_>], + assoc_items: &[hir::ImplItemRef], ) { match (lang_def_id, lang_def_id2) { (Some(lang_def_id), _) if lang_def_id == impl_def_id.to_def_id() => { diff --git a/compiler/rustc_typeck/src/impl_wf_check.rs b/compiler/rustc_typeck/src/impl_wf_check.rs index 9b23bf241cc..5d2f8fc4242 100644 --- a/compiler/rustc_typeck/src/impl_wf_check.rs +++ b/compiler/rustc_typeck/src/impl_wf_check.rs @@ -97,7 +97,7 @@ impl ItemLikeVisitor<'tcx> for ImplWfCheck<'tcx> { fn enforce_impl_params_are_constrained( tcx: TyCtxt<'_>, impl_def_id: LocalDefId, - impl_item_refs: &[hir::ImplItemRef<'_>], + impl_item_refs: &[hir::ImplItemRef], ) { // Every lifetime used in an associated type must be constrained. let impl_self_ty = tcx.type_of(impl_def_id); @@ -228,7 +228,7 @@ fn report_unused_parameter(tcx: TyCtxt<'_>, span: Span, kind: &str, name: &str) } /// Enforce that we do not have two items in an impl with the same name. -fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef<'_>]) { +fn enforce_impl_items_are_distinct(tcx: TyCtxt<'_>, impl_item_refs: &[hir::ImplItemRef]) { let mut seen_type_items = FxHashMap::default(); let mut seen_value_items = FxHashMap::default(); for impl_item_ref in impl_item_refs { diff --git a/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs b/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs index 7e4d1b3ef9f..f22f52b949e 100644 --- a/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs +++ b/src/tools/clippy/clippy_lints/src/fallible_impl_from.rs @@ -65,7 +65,7 @@ impl<'tcx> LateLintPass<'tcx> for FallibleImplFrom { } } -fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef<'_>]) { +fn lint_impl_body<'tcx>(cx: &LateContext<'tcx>, impl_span: Span, impl_items: &[hir::ImplItemRef]) { use rustc_hir::intravisit::{self, NestedVisitorMap, Visitor}; use rustc_hir::{Expr, ExprKind, ImplItemKind, QPath}; From c1bac9229ab93d1582b3974095e55fa56581b93b Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Wed, 14 Jul 2021 23:50:53 +0200 Subject: [PATCH 2/4] Remove lower_node_id_with_owner. --- compiler/rustc_ast_lowering/src/lib.rs | 99 +++++++++----------------- 1 file changed, 35 insertions(+), 64 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 66bcd698e6a..e93db721fcf 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -80,8 +80,6 @@ mod item; mod pat; mod path; -const HIR_ID_COUNTER_LOCKED: u32 = 0xFFFFFFFF; - rustc_hir::arena_types!(rustc_arena::declare_arena, 'tcx); struct LoweringContext<'a, 'hir: 'a> { @@ -151,7 +149,7 @@ struct LoweringContext<'a, 'hir: 'a> { in_scope_lifetimes: Vec, current_hir_id_owner: (LocalDefId, u32), - item_local_id_counters: NodeMap, + item_local_id_counters: IndexVec, node_id_to_hir_id: IndexVec>, allow_try_trait: Option>, @@ -488,15 +486,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { id } - fn allocate_hir_id_counter(&mut self, owner: NodeId) -> LocalDefId { - // Set up the counter if needed. - self.item_local_id_counters.entry(owner).or_insert(0); - // Always allocate the first `HirId` for the owner itself. - let lowered = self.lower_node_id_with_owner(owner, owner); - debug_assert_eq!(lowered.local_id.as_u32(), 0); - lowered.owner - } - fn create_stable_hashing_context(&self) -> LoweringHasher<'_> { LoweringHasher { source_map: CachingSourceMapView::new(self.sess.source_map()), @@ -504,36 +493,34 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } - fn lower_node_id_generic( - &mut self, - ast_node_id: NodeId, - alloc_hir_id: impl FnOnce(&mut Self) -> hir::HirId, - ) -> hir::HirId { - assert_ne!(ast_node_id, DUMMY_NODE_ID); + fn allocate_hir_id_counter(&mut self, owner: NodeId) -> LocalDefId { + // Set up the counter if needed. + let def_id = self.resolver.local_def_id(owner); - let min_size = ast_node_id.as_usize() + 1; - - if min_size > self.node_id_to_hir_id.len() { - self.node_id_to_hir_id.resize(min_size, None); - } - - if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] { - existing_hir_id + // Always allocate the first `HirId` for the owner itself. + self.node_id_to_hir_id.ensure_contains_elem(owner, || None); + if let Some(_lowered) = self.node_id_to_hir_id[owner] { + debug_assert_eq!(_lowered.owner, def_id); + debug_assert_eq!(_lowered.local_id.as_u32(), 0); } else { - // Generate a new `HirId`. - let hir_id = alloc_hir_id(self); - self.node_id_to_hir_id[ast_node_id] = Some(hir_id); + self.item_local_id_counters.ensure_contains_elem(def_id, || 0); + let local_id_counter = &mut self.item_local_id_counters[def_id]; + let local_id = *local_id_counter; - hir_id + // We want to be sure not to modify the counter in the map while it + // is also on the stack. Otherwise we'll get lost updates when writing + // back from the stack to the map. + debug_assert_eq!(local_id, 0); + + *local_id_counter += 1; + self.node_id_to_hir_id[owner] = Some(hir::HirId::make_owner(def_id)); } + def_id } fn with_hir_id_owner(&mut self, owner: NodeId, f: impl FnOnce(&mut Self) -> T) -> T { - let counter = self - .item_local_id_counters - .insert(owner, HIR_ID_COUNTER_LOCKED) - .unwrap_or_else(|| panic!("no `item_local_id_counters` entry for {:?}", owner)); let def_id = self.resolver.local_def_id(owner); + let counter = self.item_local_id_counters[def_id]; let old_owner = std::mem::replace(&mut self.current_hir_id_owner, (def_id, counter)); let ret = f(self); let (new_def_id, new_counter) = @@ -542,8 +529,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { debug_assert!(def_id == new_def_id); debug_assert!(new_counter >= counter); - let prev = self.item_local_id_counters.insert(owner, new_counter).unwrap(); - debug_assert!(prev == HIR_ID_COUNTER_LOCKED); + self.item_local_id_counters[def_id] = new_counter; ret } @@ -554,35 +540,20 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// `HirIdValidator` later on, which makes sure that all `NodeId`s got mapped /// properly. Calling the method twice with the same `NodeId` is fine though. fn lower_node_id(&mut self, ast_node_id: NodeId) -> hir::HirId { - self.lower_node_id_generic(ast_node_id, |this| { - let &mut (owner, ref mut local_id_counter) = &mut this.current_hir_id_owner; + assert_ne!(ast_node_id, DUMMY_NODE_ID); + + self.node_id_to_hir_id.ensure_contains_elem(ast_node_id, || None); + if let Some(existing_hir_id) = self.node_id_to_hir_id[ast_node_id] { + existing_hir_id + } else { + // Generate a new `HirId`. + let &mut (owner, ref mut local_id_counter) = &mut self.current_hir_id_owner; let local_id = *local_id_counter; *local_id_counter += 1; - hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) } - }) - } - - fn lower_node_id_with_owner(&mut self, ast_node_id: NodeId, owner: NodeId) -> hir::HirId { - self.lower_node_id_generic(ast_node_id, |this| { - let local_id_counter = this - .item_local_id_counters - .get_mut(&owner) - .expect("called `lower_node_id_with_owner` before `allocate_hir_id_counter`"); - let local_id = *local_id_counter; - - // We want to be sure not to modify the counter in the map while it - // is also on the stack. Otherwise we'll get lost updates when writing - // back from the stack to the map. - debug_assert!(local_id != HIR_ID_COUNTER_LOCKED); - - *local_id_counter += 1; - let owner = this.resolver.opt_local_def_id(owner).expect( - "you forgot to call `create_def` or are lowering node-IDs \ - that do not belong to the current owner", - ); - - hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) } - }) + let hir_id = hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) }; + self.node_id_to_hir_id[ast_node_id] = Some(hir_id); + hir_id + } } fn next_id(&mut self) -> hir::HirId { @@ -592,7 +563,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_res(&mut self, res: Res) -> Res { res.map_id(|id| { - self.lower_node_id_generic(id, |_| { + self.node_id_to_hir_id.get(id).copied().flatten().unwrap_or_else(|| { panic!("expected `NodeId` to be lowered already for res {:#?}", res); }) }) From a1a35576eb708e0bc04a47313bfdf3ff8aec9bc9 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Fri, 16 Jul 2021 10:16:23 +0200 Subject: [PATCH 3/4] Make with_hir_id_owner responsible for registering the item. --- compiler/rustc_ast_lowering/src/expr.rs | 2 +- compiler/rustc_ast_lowering/src/item.rs | 84 ++++++------- compiler/rustc_ast_lowering/src/lib.rs | 151 ++++++++---------------- 3 files changed, 94 insertions(+), 143 deletions(-) diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index ddb00d17c0a..a6ea4aa8923 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -327,7 +327,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let mut generic_args = vec![]; for (idx, arg) in args.into_iter().enumerate() { if legacy_args_idx.contains(&idx) { - let parent_def_id = self.current_hir_id_owner.0; + let parent_def_id = self.current_hir_id_owner; let node_id = self.resolver.next_node_id(); // Add a definition for the in-band const def. diff --git a/compiler/rustc_ast_lowering/src/item.rs b/compiler/rustc_ast_lowering/src/item.rs index b18e5733115..25e6fed68b5 100644 --- a/compiler/rustc_ast_lowering/src/item.rs +++ b/compiler/rustc_ast_lowering/src/item.rs @@ -40,12 +40,9 @@ impl ItemLowerer<'_, '_, '_> { impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> { fn visit_item(&mut self, item: &'a Item) { - self.lctx.allocate_hir_id_counter(item.id); let hir_id = self.lctx.with_hir_id_owner(item.id, |lctx| { - lctx.without_in_scope_lifetime_defs(|lctx| { - let hir_item = lctx.lower_item(item); - lctx.insert_item(hir_item) - }) + let node = lctx.without_in_scope_lifetime_defs(|lctx| lctx.lower_item(item)); + hir::OwnerNode::Item(node) }); self.lctx.with_parent_item_lifetime_defs(hir_id, |this| { @@ -72,26 +69,17 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> { } fn visit_assoc_item(&mut self, item: &'a AssocItem, ctxt: AssocCtxt) { - self.lctx.allocate_hir_id_counter(item.id); self.lctx.with_hir_id_owner(item.id, |lctx| match ctxt { - AssocCtxt::Trait => { - let hir_item = lctx.lower_trait_item(item); - lctx.insert_trait_item(hir_item); - } - AssocCtxt::Impl => { - let hir_item = lctx.lower_impl_item(item); - lctx.insert_impl_item(hir_item); - } + AssocCtxt::Trait => hir::OwnerNode::TraitItem(lctx.lower_trait_item(item)), + AssocCtxt::Impl => hir::OwnerNode::ImplItem(lctx.lower_impl_item(item)), }); visit::walk_assoc_item(self, item, ctxt); } fn visit_foreign_item(&mut self, item: &'a ForeignItem) { - self.lctx.allocate_hir_id_counter(item.id); self.lctx.with_hir_id_owner(item.id, |lctx| { - let hir_item = lctx.lower_foreign_item(item); - lctx.insert_foreign_item(hir_item); + hir::OwnerNode::ForeignItem(lctx.lower_foreign_item(item)) }); visit::walk_foreign_item(self, item); @@ -106,12 +94,12 @@ impl<'hir> LoweringContext<'_, 'hir> { // only used when lowering a child item of a trait or impl. fn with_parent_item_lifetime_defs( &mut self, - parent_hir_id: hir::ItemId, + parent_hir_id: LocalDefId, f: impl FnOnce(&mut Self) -> T, ) -> T { let old_len = self.in_scope_lifetimes.len(); - let parent_generics = match self.owners[parent_hir_id.def_id].unwrap().expect_item().kind { + let parent_generics = match self.owners[parent_hir_id].unwrap().expect_item().kind { hir::ItemKind::Impl(hir::Impl { ref generics, .. }) | hir::ItemKind::Trait(_, _, ref generics, ..) => generics.params, _ => &[], @@ -186,19 +174,20 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - pub fn lower_item(&mut self, i: &Item) -> hir::Item<'hir> { + fn lower_item(&mut self, i: &Item) -> &'hir hir::Item<'hir> { let mut ident = i.ident; let mut vis = self.lower_visibility(&i.vis); let hir_id = self.lower_node_id(i.id); let attrs = self.lower_attrs(hir_id, &i.attrs); let kind = self.lower_item_kind(i.span, i.id, hir_id, &mut ident, attrs, &mut vis, &i.kind); - hir::Item { + let item = hir::Item { def_id: hir_id.expect_owner(), ident: self.lower_ident(ident), kind, vis, span: self.lower_span(i.span), - } + }; + self.arena.alloc(item) } fn lower_item_kind( @@ -480,10 +469,16 @@ impl<'hir> LoweringContext<'_, 'hir> { // Essentially a single `use` which imports two names is desugared into // two imports. for new_node_id in [id1, id2] { - // Associate an HirId to both ids even if there is no resolution. - let new_id = self.allocate_hir_id_counter(new_node_id); - - let res = if let Some(res) = resolutions.next() { res } else { continue }; + let new_id = self.resolver.local_def_id(new_node_id); + let res = if let Some(res) = resolutions.next() { + res + } else { + // Associate an HirId to both ids even if there is no resolution. + self.node_id_to_hir_id.ensure_contains_elem(new_node_id, || None); + debug_assert!(self.node_id_to_hir_id[new_node_id].is_none()); + self.node_id_to_hir_id[new_node_id] = Some(hir::HirId::make_owner(new_id)); + continue; + }; let ident = *ident; let mut path = path.clone(); for seg in &mut path.segments { @@ -500,13 +495,14 @@ impl<'hir> LoweringContext<'_, 'hir> { this.attrs.insert(hir::HirId::make_owner(new_id), attrs); } - this.insert_item(hir::Item { + let item = hir::Item { def_id: new_id, ident: this.lower_ident(ident), kind, vis, span: this.lower_span(span), - }); + }; + hir::OwnerNode::Item(this.arena.alloc(item)) }); } @@ -550,7 +546,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // Add all the nested `PathListItem`s to the HIR. for &(ref use_tree, id) in trees { - let new_hir_id = self.allocate_hir_id_counter(id); + let new_hir_id = self.resolver.local_def_id(id); let mut prefix = prefix.clone(); @@ -574,13 +570,14 @@ impl<'hir> LoweringContext<'_, 'hir> { this.attrs.insert(hir::HirId::make_owner(new_hir_id), attrs); } - this.insert_item(hir::Item { + let item = hir::Item { def_id: new_hir_id, ident: this.lower_ident(ident), kind, vis, span: this.lower_span(use_tree.span), - }); + }; + hir::OwnerNode::Item(this.arena.alloc(item)) }); } @@ -647,11 +644,11 @@ impl<'hir> LoweringContext<'_, 'hir> { respan(self.lower_span(vis.span), vis_kind) } - fn lower_foreign_item(&mut self, i: &ForeignItem) -> hir::ForeignItem<'hir> { + fn lower_foreign_item(&mut self, i: &ForeignItem) -> &'hir hir::ForeignItem<'hir> { let hir_id = self.lower_node_id(i.id); let def_id = hir_id.expect_owner(); self.lower_attrs(hir_id, &i.attrs); - hir::ForeignItem { + let item = hir::ForeignItem { def_id, ident: self.lower_ident(i.ident), kind: match i.kind { @@ -681,12 +678,13 @@ impl<'hir> LoweringContext<'_, 'hir> { }, vis: self.lower_visibility(&i.vis), span: self.lower_span(i.span), - } + }; + self.arena.alloc(item) } fn lower_foreign_item_ref(&mut self, i: &ForeignItem) -> hir::ForeignItemRef { hir::ForeignItemRef { - id: hir::ForeignItemId { def_id: self.allocate_hir_id_counter(i.id) }, + id: hir::ForeignItemId { def_id: self.resolver.local_def_id(i.id) }, ident: self.lower_ident(i.ident), span: self.lower_span(i.span), } @@ -761,7 +759,7 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - fn lower_trait_item(&mut self, i: &AssocItem) -> hir::TraitItem<'hir> { + fn lower_trait_item(&mut self, i: &AssocItem) -> &'hir hir::TraitItem<'hir> { let hir_id = self.lower_node_id(i.id); let trait_item_def_id = hir_id.expect_owner(); @@ -804,13 +802,14 @@ impl<'hir> LoweringContext<'_, 'hir> { }; self.lower_attrs(hir_id, &i.attrs); - hir::TraitItem { + let item = hir::TraitItem { def_id: trait_item_def_id, ident: self.lower_ident(i.ident), generics, kind, span: self.lower_span(i.span), - } + }; + self.arena.alloc(item) } fn lower_trait_item_ref(&mut self, i: &AssocItem) -> hir::TraitItemRef { @@ -840,7 +839,7 @@ impl<'hir> LoweringContext<'_, 'hir> { self.expr(span, hir::ExprKind::Err, AttrVec::new()) } - fn lower_impl_item(&mut self, i: &AssocItem) -> hir::ImplItem<'hir> { + fn lower_impl_item(&mut self, i: &AssocItem) -> &'hir hir::ImplItem<'hir> { let impl_item_def_id = self.resolver.local_def_id(i.id); let (generics, kind) = match &i.kind { @@ -894,7 +893,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value); let hir_id = self.lower_node_id(i.id); self.lower_attrs(hir_id, &i.attrs); - hir::ImplItem { + let item = hir::ImplItem { def_id: hir_id.expect_owner(), ident: self.lower_ident(i.ident), generics, @@ -902,7 +901,8 @@ impl<'hir> LoweringContext<'_, 'hir> { defaultness, kind, span: self.lower_span(i.span), - } + }; + self.arena.alloc(item) } fn lower_impl_item_ref(&mut self, i: &AssocItem) -> hir::ImplItemRef { @@ -910,7 +910,7 @@ impl<'hir> LoweringContext<'_, 'hir> { let has_value = true; let (defaultness, _) = self.lower_defaultness(i.kind.defaultness(), has_value); hir::ImplItemRef { - id: hir::ImplItemId { def_id: self.allocate_hir_id_counter(i.id) }, + id: hir::ImplItemId { def_id: self.resolver.local_def_id(i.id) }, ident: self.lower_ident(i.ident), span: self.lower_span(i.span), defaultness, diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index e93db721fcf..5ec060f6540 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -148,8 +148,8 @@ struct LoweringContext<'a, 'hir: 'a> { /// vector. in_scope_lifetimes: Vec, - current_hir_id_owner: (LocalDefId, u32), - item_local_id_counters: IndexVec, + current_hir_id_owner: LocalDefId, + item_local_id_counter: hir::ItemLocalId, node_id_to_hir_id: IndexVec>, allow_try_trait: Option>, @@ -328,8 +328,8 @@ pub fn lower_crate<'a, 'hir>( is_in_trait_impl: false, is_in_dyn_type: false, anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough, - current_hir_id_owner: (CRATE_DEF_ID, 0), - item_local_id_counters: Default::default(), + current_hir_id_owner: CRATE_DEF_ID, + item_local_id_counter: hir::ItemLocalId::new(0), node_id_to_hir_id: IndexVec::new(), generator_kind: None, task_context: None, @@ -410,15 +410,15 @@ enum AnonymousLifetimeMode { impl<'a, 'hir> LoweringContext<'a, 'hir> { fn lower_crate(mut self, c: &Crate) -> &'hir hir::Crate<'hir> { - self.lower_node_id(CRATE_NODE_ID); - debug_assert!(self.node_id_to_hir_id[CRATE_NODE_ID] == Some(hir::CRATE_HIR_ID)); + debug_assert_eq!(self.resolver.local_def_id(CRATE_NODE_ID), CRATE_DEF_ID); visit::walk_crate(&mut item::ItemLowerer { lctx: &mut self }, c); - let module = self.arena.alloc(self.lower_mod(&c.items, c.span)); - self.lower_attrs(hir::CRATE_HIR_ID, &c.attrs); - self.owners.ensure_contains_elem(CRATE_DEF_ID, || None); - self.owners[CRATE_DEF_ID] = Some(hir::OwnerNode::Crate(module)); + self.with_hir_id_owner(CRATE_NODE_ID, |lctx| { + let module = lctx.lower_mod(&c.items, c.span); + lctx.lower_attrs(hir::CRATE_HIR_ID, &c.attrs); + hir::OwnerNode::Crate(lctx.arena.alloc(module)) + }); let mut trait_map: FxHashMap<_, FxHashMap<_, _>> = FxHashMap::default(); for (k, v) in self.resolver.take_trait_map().into_iter() { @@ -454,38 +454,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { self.arena.alloc(krate) } - fn insert_item(&mut self, item: hir::Item<'hir>) -> hir::ItemId { - let id = item.item_id(); - let item = self.arena.alloc(item); - self.owners.ensure_contains_elem(id.def_id, || None); - self.owners[id.def_id] = Some(hir::OwnerNode::Item(item)); - id - } - - fn insert_foreign_item(&mut self, item: hir::ForeignItem<'hir>) -> hir::ForeignItemId { - let id = item.foreign_item_id(); - let item = self.arena.alloc(item); - self.owners.ensure_contains_elem(id.def_id, || None); - self.owners[id.def_id] = Some(hir::OwnerNode::ForeignItem(item)); - id - } - - fn insert_impl_item(&mut self, item: hir::ImplItem<'hir>) -> hir::ImplItemId { - let id = item.impl_item_id(); - let item = self.arena.alloc(item); - self.owners.ensure_contains_elem(id.def_id, || None); - self.owners[id.def_id] = Some(hir::OwnerNode::ImplItem(item)); - id - } - - fn insert_trait_item(&mut self, item: hir::TraitItem<'hir>) -> hir::TraitItemId { - let id = item.trait_item_id(); - let item = self.arena.alloc(item); - self.owners.ensure_contains_elem(id.def_id, || None); - self.owners[id.def_id] = Some(hir::OwnerNode::TraitItem(item)); - id - } - fn create_stable_hashing_context(&self) -> LoweringHasher<'_> { LoweringHasher { source_map: CachingSourceMapView::new(self.sess.source_map()), @@ -493,46 +461,35 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } } - fn allocate_hir_id_counter(&mut self, owner: NodeId) -> LocalDefId { - // Set up the counter if needed. + fn with_hir_id_owner( + &mut self, + owner: NodeId, + f: impl FnOnce(&mut Self) -> hir::OwnerNode<'hir>, + ) -> LocalDefId { let def_id = self.resolver.local_def_id(owner); // Always allocate the first `HirId` for the owner itself. self.node_id_to_hir_id.ensure_contains_elem(owner, || None); if let Some(_lowered) = self.node_id_to_hir_id[owner] { - debug_assert_eq!(_lowered.owner, def_id); - debug_assert_eq!(_lowered.local_id.as_u32(), 0); - } else { - self.item_local_id_counters.ensure_contains_elem(def_id, || 0); - let local_id_counter = &mut self.item_local_id_counters[def_id]; - let local_id = *local_id_counter; - - // We want to be sure not to modify the counter in the map while it - // is also on the stack. Otherwise we'll get lost updates when writing - // back from the stack to the map. - debug_assert_eq!(local_id, 0); - - *local_id_counter += 1; - self.node_id_to_hir_id[owner] = Some(hir::HirId::make_owner(def_id)); + panic!("with_hir_id_owner must not be called multiple times on owner {:?}", def_id); } + self.node_id_to_hir_id[owner] = Some(hir::HirId::make_owner(def_id)); + + let current_owner = std::mem::replace(&mut self.current_hir_id_owner, def_id); + let current_local_counter = + std::mem::replace(&mut self.item_local_id_counter, hir::ItemLocalId::new(1)); + + let item = f(self); + + self.current_hir_id_owner = current_owner; + self.item_local_id_counter = current_local_counter; + + self.owners.ensure_contains_elem(def_id, || None); + self.owners[def_id] = Some(item); + def_id } - fn with_hir_id_owner(&mut self, owner: NodeId, f: impl FnOnce(&mut Self) -> T) -> T { - let def_id = self.resolver.local_def_id(owner); - let counter = self.item_local_id_counters[def_id]; - let old_owner = std::mem::replace(&mut self.current_hir_id_owner, (def_id, counter)); - let ret = f(self); - let (new_def_id, new_counter) = - std::mem::replace(&mut self.current_hir_id_owner, old_owner); - - debug_assert!(def_id == new_def_id); - debug_assert!(new_counter >= counter); - - self.item_local_id_counters[def_id] = new_counter; - ret - } - /// This method allocates a new `HirId` for the given `NodeId` and stores it in /// the `LoweringContext`'s `NodeId => HirId` map. /// Take care not to call this method if the resulting `HirId` is then not @@ -547,10 +504,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { existing_hir_id } else { // Generate a new `HirId`. - let &mut (owner, ref mut local_id_counter) = &mut self.current_hir_id_owner; - let local_id = *local_id_counter; - *local_id_counter += 1; - let hir_id = hir::HirId { owner, local_id: hir::ItemLocalId::from_u32(local_id) }; + let owner = self.current_hir_id_owner; + let local_id = self.item_local_id_counter; + self.item_local_id_counter.increment_by(1); + let hir_id = hir::HirId { owner, local_id }; self.node_id_to_hir_id[ast_node_id] = Some(hir_id); hir_id } @@ -626,7 +583,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { /// Mark a span as relative to the current owning item. fn lower_span(&self, span: Span) -> Span { if self.sess.opts.debugging_opts.incremental_relative_spans { - span.with_parent(Some(self.current_hir_id_owner.0)) + span.with_parent(Some(self.current_hir_id_owner)) } else { // Do not make spans relative when not using incremental compilation. span @@ -799,7 +756,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // wouldn't have been added yet. let generics = this.lower_generics_mut( generics, - ImplTraitContext::Universal(&mut params, this.current_hir_id_owner.0), + ImplTraitContext::Universal(&mut params, this.current_hir_id_owner), ); let res = f(this, &mut params); (params, (generics, res)) @@ -1005,7 +962,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { } AssocTyConstraintKind::Bound { ref bounds } => { let mut capturable_lifetimes; - let mut parent_def_id = self.current_hir_id_owner.0; + let mut parent_def_id = self.current_hir_id_owner; // Piggy-back on the `impl Trait` context to figure out the correct behavior. let (desugar_to_impl_trait, itctx) = match itctx { // We are in the return position: @@ -1133,7 +1090,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // Construct an AnonConst where the expr is the "ty"'s path. - let parent_def_id = self.current_hir_id_owner.0; + let parent_def_id = self.current_hir_id_owner; let node_id = self.resolver.next_node_id(); // Add a definition for the in-band const def. @@ -1399,12 +1356,13 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // frequently opened issues show. let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::OpaqueTy, span, None); - let opaque_ty_def_id = self.allocate_hir_id_counter(opaque_ty_node_id); + let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id); - let collected_lifetimes = self.with_hir_id_owner(opaque_ty_node_id, move |lctx| { + let mut collected_lifetimes = Vec::new(); + self.with_hir_id_owner(opaque_ty_node_id, |lctx| { let hir_bounds = lower_bounds(lctx); - let collected_lifetimes = lifetimes_from_impl_trait_bounds( + collected_lifetimes = lifetimes_from_impl_trait_bounds( opaque_ty_node_id, &hir_bounds, capturable_lifetimes, @@ -1457,9 +1415,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { }; trace!("lower_opaque_impl_trait: {:#?}", opaque_ty_def_id); - lctx.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span); - - collected_lifetimes + lctx.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span) }); let lifetimes = @@ -1481,7 +1437,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { opaque_ty_item: hir::OpaqueTy<'hir>, span: Span, opaque_ty_span: Span, - ) { + ) -> hir::OwnerNode<'hir> { let opaque_ty_item_kind = hir::ItemKind::OpaqueTy(opaque_ty_item); // Generate an `type Foo = impl Trait;` declaration. trace!("registering opaque type with id {:#?}", opaque_ty_id); @@ -1492,11 +1448,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { vis: respan(self.lower_span(span.shrink_to_lo()), hir::VisibilityKind::Inherited), span: self.lower_span(opaque_ty_span), }; - - // Insert the item into the global item list. This usually happens - // automatically for all AST items. But this opaque type item - // does not actually exist in the AST. - self.insert_item(opaque_ty_item); + hir::OwnerNode::Item(self.arena.alloc(opaque_ty_item)) } fn lower_fn_params_to_names(&mut self, decl: &FnDecl) -> &'hir [Ident] { @@ -1565,7 +1517,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { if let Some((_, ibty)) = &mut in_band_ty_params { this.lower_ty_direct( ¶m.ty, - ImplTraitContext::Universal(ibty, this.current_hir_id_owner.0), + ImplTraitContext::Universal(ibty, this.current_hir_id_owner), ) } else { this.lower_ty_direct(¶m.ty, ImplTraitContext::disallowed()) @@ -1656,7 +1608,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { let opaque_ty_span = self.mark_span_with_reason(DesugaringKind::Async, span, None); - let opaque_ty_def_id = self.allocate_hir_id_counter(opaque_ty_node_id); + let opaque_ty_def_id = self.resolver.local_def_id(opaque_ty_node_id); // When we create the opaque type for this async fn, it is going to have // to capture all the lifetimes involved in the signature (including in the @@ -1706,7 +1658,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // grow. let input_lifetimes_count = self.in_scope_lifetimes.len() + self.lifetimes_to_define.len(); - let lifetime_params = self.with_hir_id_owner(opaque_ty_node_id, |this| { + let mut lifetime_params = Vec::new(); + self.with_hir_id_owner(opaque_ty_node_id, |this| { // We have to be careful to get elision right here. The // idea is that we create a lifetime parameter for each // lifetime in the return type. So, given a return type @@ -1728,7 +1681,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { // // Note: this must be done after lowering the output type, // as the output type may introduce new in-band lifetimes. - let lifetime_params: Vec<(Span, ParamName)> = this + lifetime_params = this .in_scope_lifetimes .iter() .cloned() @@ -1757,9 +1710,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { }; trace!("exist ty from async fn def id: {:#?}", opaque_ty_def_id); - this.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span); - - lifetime_params + this.generate_opaque_type(opaque_ty_def_id, opaque_ty_item, span, opaque_ty_span) }); // As documented above on the variable From 11024b26bfc0930548ee717fb6d743d80a8e56f2 Mon Sep 17 00:00:00 2001 From: Camille GILLOT Date: Sun, 29 Aug 2021 15:33:03 +0200 Subject: [PATCH 4/4] Bless incremental tests. --- src/test/incremental/hashes/extern_mods.rs | 6 +++--- src/test/incremental/hashes/inherent_impls.rs | 12 ++++++++---- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/test/incremental/hashes/extern_mods.rs b/src/test/incremental/hashes/extern_mods.rs index 783407a9e04..4bc98fd7cd0 100644 --- a/src/test/incremental/hashes/extern_mods.rs +++ b/src/test/incremental/hashes/extern_mods.rs @@ -143,13 +143,13 @@ extern "rust-call" { // Make function public -------------------------------------------------------- #[cfg(any(cfail1,cfail4))] extern "C" { - fn make_function_public(c: i32); + fn make_function_public(c: i32); } #[cfg(not(any(cfail1,cfail4)))] -#[rustc_clean(cfg = "cfail2", except = "hir_owner")] +#[rustc_clean(cfg = "cfail2")] #[rustc_clean(cfg = "cfail3")] -#[rustc_clean(cfg = "cfail5", except = "hir_owner")] +#[rustc_clean(cfg = "cfail5")] #[rustc_clean(cfg = "cfail6")] extern "C" { pub fn make_function_public(c: i32); diff --git a/src/test/incremental/hashes/inherent_impls.rs b/src/test/incremental/hashes/inherent_impls.rs index d711cc20dd7..3a59377e819 100644 --- a/src/test/incremental/hashes/inherent_impls.rs +++ b/src/test/incremental/hashes/inherent_impls.rs @@ -116,20 +116,24 @@ impl Foo { // Change Method Privacy ------------------------------------------------------- #[cfg(any(cfail1,cfail4))] impl Foo { + //------------------------------------------------------------------------------ + //-------------------------- + //------------------------------------------------------------------------------ + //-------------------------- pub fn method_privacy() { } } #[cfg(not(any(cfail1,cfail4)))] -#[rustc_clean(cfg="cfail2", except="hir_owner")] +#[rustc_clean(cfg="cfail2")] #[rustc_clean(cfg="cfail3")] -#[rustc_clean(cfg="cfail5", except="hir_owner")] +#[rustc_clean(cfg="cfail5")] #[rustc_clean(cfg="cfail6")] impl Foo { #[rustc_clean(cfg="cfail2", except="associated_item,hir_owner,hir_owner_nodes")] #[rustc_clean(cfg="cfail3")] - #[rustc_clean(cfg="cfail5", except="associated_item,hir_owner,hir_owner_nodes,optimized_mir")] + #[rustc_clean(cfg="cfail5", except="associated_item,hir_owner,hir_owner_nodes")] #[rustc_clean(cfg="cfail6")] - fn method_privacy() { } + fn method_privacy() { } } // Change Method Selfness -----------------------------------------------------------