Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seperate HIR owner from LocalDefId in the type system #85587

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ impl<'hir> LoweringContext<'_, 'hir> {

// Add a definition for the in-band const def.
self.resolver.create_def(
parent_def_id,
parent_def_id.def_id,
node_id,
DefPathData::AnonConst,
ExpnId::root(),
Expand Down
20 changes: 13 additions & 7 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ impl<'a> Visitor<'a> for ItemLowerer<'a, '_, '_> {
ItemKind::Mod(..) => {
let def_id = this.lctx.lower_node_id(item.id).expect_owner();
let old_current_module =
mem::replace(&mut this.lctx.current_module, def_id);
mem::replace(&mut this.lctx.current_module, def_id.def_id);
visit::walk_item(this, item);
this.lctx.current_module = old_current_module;
}
Expand Down Expand Up @@ -404,7 +404,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let lowered_trait_def_id = self.lower_node_id(id).expect_owner();
let (generics, (trait_ref, lowered_ty)) = self.add_in_band_defs(
ast_generics,
lowered_trait_def_id,
lowered_trait_def_id.def_id,
AnonymousLifetimeMode::CreateParameter,
|this, _| {
let trait_ref = trait_ref.as_ref().map(|trait_ref| {
Expand All @@ -416,7 +416,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
this.trait_impls
.entry(def_id)
.or_default()
.push(lowered_trait_def_id);
.push(lowered_trait_def_id.def_id);
}
}

Expand Down Expand Up @@ -714,7 +714,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let fdec = &sig.decl;
let (generics, (fn_dec, fn_args)) = self.add_in_band_defs(
generics,
def_id,
def_id.def_id,
AnonymousLifetimeMode::PassThrough,
|this, _| {
(
Expand Down Expand Up @@ -832,8 +832,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
}
AssocItemKind::Fn(box FnKind(_, ref sig, ref generics, None)) => {
let names = self.lower_fn_params_to_names(&sig.decl);
let (generics, sig) =
self.lower_method_sig(generics, sig, trait_item_def_id, false, None, i.id);
let (generics, sig) = self.lower_method_sig(
generics,
sig,
trait_item_def_id.def_id,
false,
None,
i.id,
);
(generics, hir::TraitItemKind::Fn(sig, hir::TraitFn::Required(names)))
}
AssocItemKind::Fn(box FnKind(_, ref sig, ref generics, Some(ref body))) => {
Expand All @@ -843,7 +849,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (generics, sig) = self.lower_method_sig(
generics,
sig,
trait_item_def_id,
trait_item_def_id.def_id,
false,
asyncness.opt_return_id(),
i.id,
Expand Down
49 changes: 33 additions & 16 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ use rustc_hir::def::{DefKind, Namespace, PartialRes, PerNS, Res};
use rustc_hir::def_id::{DefId, DefIdMap, LocalDefId, CRATE_DEF_ID};
use rustc_hir::definitions::{DefKey, DefPathData, Definitions};
use rustc_hir::intravisit;
use rustc_hir::{ConstArg, GenericArg, ParamName};
use rustc_hir::{ConstArg, GenericArg, HirOwner, ParamName};
use rustc_index::vec::{Idx, IndexVec};
use rustc_session::lint::builtin::{BARE_TRAIT_OBJECTS, MISSING_ABI};
use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer};
Expand Down Expand Up @@ -166,7 +166,7 @@ struct LoweringContext<'a, 'hir: 'a> {

type_def_lifetime_params: DefIdMap<usize>,

current_hir_id_owner: (LocalDefId, u32),
current_hir_id_owner: (HirOwner, u32),
item_local_id_counters: NodeMap<u32>,
node_id_to_hir_id: IndexVec<NodeId, Option<hir::HirId>>,

Expand Down Expand Up @@ -223,7 +223,7 @@ enum ImplTraitContext<'b, 'a> {
/// equivalent to a fresh universal parameter like `fn foo<T: Debug>(x: T)`.
///
/// Newly generated parameters should be inserted into the given `Vec`.
Universal(&'b mut Vec<hir::GenericParam<'a>>, LocalDefId),
Universal(&'b mut Vec<hir::GenericParam<'a>>, HirOwner),

/// Treat `impl Trait` as shorthand for a new opaque type.
/// Example: `fn foo() -> impl Debug`, where `impl Debug` is conceptually
Expand Down Expand Up @@ -322,7 +322,7 @@ pub fn lower_crate<'a, 'hir>(
anonymous_lifetime_mode: AnonymousLifetimeMode::PassThrough,
type_def_lifetime_params: Default::default(),
current_module: CRATE_DEF_ID,
current_hir_id_owner: (CRATE_DEF_ID, 0),
current_hir_id_owner: (HirOwner { def_id: CRATE_DEF_ID }, 0),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A CRATE_OWNER_ID const could be handy.

item_local_id_counters: Default::default(),
node_id_to_hir_id: IndexVec::new(),
generator_kind: None,
Expand Down Expand Up @@ -594,7 +594,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
.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 def_id = HirOwner { def_id: self.resolver.local_def_id(owner) };
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) =
Expand Down Expand Up @@ -637,10 +637,12 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
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 \
let owner = HirOwner {
def_id: 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) }
})
Expand Down Expand Up @@ -1133,7 +1135,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let impl_trait_node_id = self.resolver.next_node_id();
self.resolver.create_def(
parent_def_id,
parent_def_id.def_id,
impl_trait_node_id,
DefPathData::ImplTrait,
ExpnId::root(),
Expand Down Expand Up @@ -1201,7 +1203,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

// Add a definition for the in-band const def.
self.resolver.create_def(
parent_def_id,
parent_def_id.def_id,
node_id,
DefPathData::AnonConst,
ExpnId::root(),
Expand Down Expand Up @@ -1512,18 +1514,26 @@ 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);
lctx.generate_opaque_type(
HirOwner { def_id: opaque_ty_def_id },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allocate_hir_id_counter should return a HirOwner you can use here instead of constructing it each time.

opaque_ty_item,
span,
opaque_ty_span,
);

// `impl Trait` now just becomes `Foo<'a, 'b, ..>`.
hir::TyKind::OpaqueDef(hir::ItemId { def_id: opaque_ty_def_id }, lifetimes)
hir::TyKind::OpaqueDef(
hir::ItemId { def_id: HirOwner { def_id: opaque_ty_def_id } },
lifetimes,
)
})
}

/// Registers a new opaque type with the proper `NodeId`s and
/// returns the lowered node-ID for the opaque type.
fn generate_opaque_type(
&mut self,
opaque_ty_id: LocalDefId,
opaque_ty_id: HirOwner,
opaque_ty_item: hir::OpaqueTy<'hir>,
span: Span,
opaque_ty_span: Span,
Expand Down Expand Up @@ -2015,7 +2025,12 @@ 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);
this.generate_opaque_type(
HirOwner { def_id: opaque_ty_def_id },
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likewise

opaque_ty_item,
span,
opaque_ty_span,
);

lifetime_params
});
Expand Down Expand Up @@ -2060,8 +2075,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
// Foo = impl Trait` is, internally, created as a child of the
// async fn, so the *type parameters* are inherited. It's
// only the lifetime parameters that we must supply.
let opaque_ty_ref =
hir::TyKind::OpaqueDef(hir::ItemId { def_id: opaque_ty_def_id }, generic_args);
let opaque_ty_ref = hir::TyKind::OpaqueDef(
hir::ItemId { def_id: HirOwner { def_id: opaque_ty_def_id } },
generic_args,
);
let opaque_ty = self.ty(opaque_ty_span, opaque_ty_ref);
hir::FnRetTy::Return(self.arena.alloc(opaque_ty))
}
Expand Down
20 changes: 10 additions & 10 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// ignore-tidy-filelength
use crate::def::{CtorKind, DefKind, Res};
use crate::def_id::DefId;
crate use crate::hir_id::HirId;
crate use crate::hir_id::{HirId, HirOwner};
use crate::{itemlikevisit, LangItem};

use rustc_ast::util::parser::ExprPrecedence;
Expand Down Expand Up @@ -753,7 +753,7 @@ impl Crate<'_> {
pub struct MacroDef<'hir> {
pub ident: Ident,
pub vis: Visibility<'hir>,
pub def_id: LocalDefId,
pub def_id: HirOwner,
pub span: Span,
pub ast: ast::MacroDef,
}
Expand Down Expand Up @@ -1994,7 +1994,7 @@ pub struct FnSig<'hir> {
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
pub struct TraitItemId {
pub def_id: LocalDefId,
pub def_id: HirOwner,
}

impl TraitItemId {
Expand All @@ -2012,7 +2012,7 @@ impl TraitItemId {
#[derive(Debug)]
pub struct TraitItem<'hir> {
pub ident: Ident,
pub def_id: LocalDefId,
pub def_id: HirOwner,
pub generics: Generics<'hir>,
pub kind: TraitItemKind<'hir>,
pub span: Span,
Expand Down Expand Up @@ -2057,7 +2057,7 @@ pub enum TraitItemKind<'hir> {
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
pub struct ImplItemId {
pub def_id: LocalDefId,
pub def_id: HirOwner,
}

impl ImplItemId {
Expand All @@ -2072,7 +2072,7 @@ impl ImplItemId {
#[derive(Debug)]
pub struct ImplItem<'hir> {
pub ident: Ident,
pub def_id: LocalDefId,
pub def_id: HirOwner,
pub vis: Visibility<'hir>,
pub defaultness: Defaultness,
pub generics: Generics<'hir>,
Expand Down Expand Up @@ -2655,7 +2655,7 @@ impl VariantData<'hir> {
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug, Hash)]
pub struct ItemId {
pub def_id: LocalDefId,
pub def_id: HirOwner,
}

impl ItemId {
Expand All @@ -2672,7 +2672,7 @@ impl ItemId {
#[derive(Debug)]
pub struct Item<'hir> {
pub ident: Ident,
pub def_id: LocalDefId,
pub def_id: HirOwner,
pub kind: ItemKind<'hir>,
pub vis: Visibility<'hir>,
pub span: Span,
Expand Down Expand Up @@ -2860,7 +2860,7 @@ pub enum AssocItemKind {
// so it can fetched later.
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Encodable, Debug)]
pub struct ForeignItemId {
pub def_id: LocalDefId,
pub def_id: HirOwner,
}

impl ForeignItemId {
Expand Down Expand Up @@ -2890,7 +2890,7 @@ pub struct ForeignItemRef<'hir> {
pub struct ForeignItem<'hir> {
pub ident: Ident,
pub kind: ForeignItemKind<'hir>,
pub def_id: LocalDefId,
pub def_id: HirOwner,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think owner would be a better name now for this field here and elsewhere.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already a struct Owner here

#[derive(Debug)]
pub struct Owner<'tcx> {
node: Node<'tcx>,
}

pub span: Span,
pub vis: Visibility<'hir>,
}
Expand Down
Loading