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

[WIP] Delay def collection for expression-like nodes #128844

Closed
wants to merge 17 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
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
12 changes: 0 additions & 12 deletions compiler/rustc_ast_lowering/src/asm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap};
use rustc_hir as hir;
use rustc_hir::def::{DefKind, Res};
use rustc_session::parse::feature_err;
use rustc_span::symbol::kw;
use rustc_span::{sym, Span};
use rustc_target::asm;

Expand Down Expand Up @@ -222,18 +221,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
};

// Wrap the expression in an AnonConst.
let parent_def_id = self.current_def_id_parent;
let node_id = self.next_node_id();
// HACK(min_generic_const_args): see lower_anon_const
if !expr.is_potential_trivial_const_arg() {
self.create_def(
parent_def_id,
node_id,
kw::Empty,
DefKind::AnonConst,
*op_sp,
);
}
let anon_const = AnonConst { id: node_id, value: P(expr) };
hir::InlineAsmOperand::SymFn {
anon_const: self.lower_anon_const_to_anon_const(&anon_const),
Expand Down
57 changes: 49 additions & 8 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
_ => (),
}

self.create_def_if_needed_for(e);
let hir_id = self.lower_node_id(e.id);
self.lower_attrs(hir_id, &e.attrs);

Expand Down Expand Up @@ -356,6 +357,54 @@ impl<'hir> LoweringContext<'_, 'hir> {
})
}

/// HACK(min_generic_const_args): we delay creation of expression defs until ast_lowering
///
/// This only creates a def for the top-level expression. If it has nested expressions that
/// need defs, those are handled by the recursion in the main lowering logic.
fn create_def_if_needed_for(&mut self, e: &Expr) {
match &e.kind {
ExprKind::ConstBlock(c) => {
self.create_def(
self.current_def_id_parent,
c.id,
kw::Empty,
DefKind::InlineConst,
c.value.span,
);
}
ExprKind::Closure(box Closure { coroutine_kind, .. }) => {
let closure_def = self.create_def(
self.current_def_id_parent,
e.id,
kw::Empty,
DefKind::Closure,
e.span,
);
if let Some(coroutine_kind) = coroutine_kind {
self.with_def_id_parent(closure_def, |this| {
this.create_def(
this.current_def_id_parent,
coroutine_kind.closure_id(),
kw::Empty,
DefKind::Closure,
e.span,
);
})
}
}
ExprKind::Gen(..) => {
self.create_def(
self.current_def_id_parent,
e.id,
kw::Empty,
DefKind::Closure,
e.span,
);
}
_ => {}
}
}

fn lower_unop(&mut self, u: UnOp) -> hir::UnOp {
match u {
UnOp::Deref => hir::UnOp::Deref,
Expand Down Expand Up @@ -383,15 +432,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
let mut generic_args = ThinVec::new();
for (idx, arg) in args.into_iter().enumerate() {
if legacy_args_idx.contains(&idx) {
let parent_def_id = self.current_def_id_parent;
let node_id = self.next_node_id();

// HACK(min_generic_const_args): see lower_anon_const
if !arg.is_potential_trivial_const_arg() {
// Add a definition for the in-band const def.
self.create_def(parent_def_id, node_id, kw::Empty, DefKind::AnonConst, f.span);
}

let anon_const = AnonConst { id: node_id, value: arg };
generic_args.push(AngleBracketedArg::Arg(GenericArg::Const(anon_const)));
} else {
Expand Down
12 changes: 12 additions & 0 deletions compiler/rustc_ast_lowering/src/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,10 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
self.visit_body(body);
}

fn visit_nested_opaque_ty(&mut self, opaq: &'hir OpaqueTy<'hir>) -> Self::Result {
self.visit_opaque_ty(opaq)
}

fn visit_param(&mut self, param: &'hir Param<'hir>) {
let node = Node::Param(param);
self.insert(param.pat.span, param.hir_id, node);
Expand Down Expand Up @@ -228,6 +232,14 @@ impl<'a, 'hir> Visitor<'hir> for NodeCollector<'a, 'hir> {
});
}

fn visit_opaque_ty(&mut self, opaq: &'hir OpaqueTy<'hir>) {
self.insert(opaq.span, opaq.hir_id, Node::OpaqueTy(opaq));

self.with_parent(opaq.hir_id, |this| {
intravisit::walk_opaque_ty(this, opaq);
});
}

fn visit_anon_const(&mut self, constant: &'hir AnonConst) {
// FIXME: use real span?
self.insert(DUMMY_SP, constant.hir_id, Node::AnonConst(constant));
Expand Down
7 changes: 7 additions & 0 deletions compiler/rustc_ast_lowering/src/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
let (Some(coroutine_kind), Some(body)) = (coroutine_kind, body) else {
return self.lower_fn_body_block(span, decl, body);
};
self.create_def(
self.current_def_id_parent,
coroutine_kind.closure_id(),
kw::Empty,
DefKind::Closure,
span,
);
self.lower_body(|this| {
let (parameters, expr) = this.lower_coroutine_body_with_moved_arguments(
decl,
Expand Down
56 changes: 22 additions & 34 deletions compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,7 +828,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
LifetimeRes::Fresh { param, kind, .. } => {
// Late resolution delegates to us the creation of the `LocalDefId`.
let _def_id = self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
self.current_def_id_parent,
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove that parameter and have create_def always use current_def_id_parent?

Copy link
Member

Choose a reason for hiding this comment

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

That would make sense to me 👍 I think using current_hir_id_owner only makes sense if either:

  • We statically know that the current_def_id_parent is the innermost hir owner
  • All definitions participating in the def parenting tree are also hir owners (aka, the former point is always trivially true)

I think right now there should never be any cases where we're creating defs that wants to skip a parent. The struct definition in fn foo() -> Foo<{ struct Bar; 1 }> does have that behaviour since its parent is foo not constant#0 but that' happens via creating the DefId in DefCollector.

param,
kw::UnderscoreLifetime,
DefKind::LifetimeParam,
Expand Down Expand Up @@ -1412,7 +1412,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
);

self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
self.current_def_id_parent,
*def_node_id,
ident.name,
DefKind::TyParam,
Expand Down Expand Up @@ -1619,14 +1619,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
opaque_ty_span: Span,
lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>],
) -> hir::TyKind<'hir> {
let opaque_ty_hir_id = self.next_id();
let opaque_ty_def_id = self.create_def(
self.current_hir_id_owner.def_id, // FIXME: should this use self.current_def_id_parent?
self.current_def_id_parent,
opaque_ty_node_id,
kw::Empty,
DefKind::OpaqueTy,
opaque_ty_span,
);
debug!(?opaque_ty_def_id);
self.children.push((opaque_ty_def_id, hir::MaybeOwner::NonOwner(opaque_ty_hir_id)));

// Map from captured (old) lifetime to synthetic (new) lifetime.
// Used to resolve lifetimes in the bounds of the opaque.
Expand Down Expand Up @@ -1699,7 +1701,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
}
}

self.with_hir_id_owner(opaque_ty_node_id, |this| {
let opaque_ty_def = self.with_def_id_parent(opaque_ty_def_id, |this| {
// Install the remapping from old to new (if any). This makes sure that
// any lifetimes that would have resolved to the def-id of captured
// lifetimes are remapped to the new *synthetic* lifetimes of the opaque.
Expand Down Expand Up @@ -1737,7 +1739,10 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {

let lifetime_mapping = self.arena.alloc_slice(&synthesized_lifetime_args);

let opaque_ty_item = hir::OpaqueTy {
trace!("registering opaque type with id {:#?}", opaque_ty_def_id);
let opaque_ty_def = hir::OpaqueTy {
hir_id: opaque_ty_hir_id,
def_id: opaque_ty_def_id,
generics: this.arena.alloc(hir::Generics {
params: generic_params,
predicates: &[],
Expand All @@ -1749,19 +1754,9 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
origin,
lifetime_mapping,
in_trait,
};

// Generate an `type Foo = impl Trait;` declaration.
trace!("registering opaque type with id {:#?}", opaque_ty_def_id);
let opaque_ty_item = hir::Item {
owner_id: hir::OwnerId { def_id: opaque_ty_def_id },
ident: Ident::empty(),
kind: hir::ItemKind::OpaqueTy(this.arena.alloc(opaque_ty_item)),
vis_span: this.lower_span(span.shrink_to_lo()),
span: this.lower_span(opaque_ty_span),
};

hir::OwnerNode::Item(this.arena.alloc(opaque_ty_item))
this.arena.alloc(opaque_ty_def)
});

let generic_args = self.arena.alloc_from_iter(
Expand All @@ -1774,11 +1769,7 @@ 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.
hir::TyKind::OpaqueDef(
hir::ItemId { owner_id: hir::OwnerId { def_id: opaque_ty_def_id } },
generic_args,
in_trait,
)
hir::TyKind::OpaqueDef(opaque_ty_def, generic_args, in_trait)
}

fn lower_precise_capturing_args(
Expand Down Expand Up @@ -2465,19 +2456,16 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
/// See [`hir::ConstArg`] for when to use this function vs
/// [`Self::lower_anon_const_to_const_arg`].
fn lower_anon_const_to_anon_const(&mut self, c: &AnonConst) -> &'hir hir::AnonConst {
if c.value.is_potential_trivial_const_arg() {
// HACK(min_generic_const_args): see DefCollector::visit_anon_const
// Over there, we guess if this is a bare param and only create a def if
// we think it's not. However we may can guess wrong (see there for example)
// in which case we have to create the def here.
self.create_def(
self.current_def_id_parent,
c.id,
kw::Empty,
DefKind::AnonConst,
c.value.span,
);
}
// HACK(min_generic_const_args): see DefCollector::visit_anon_const
// We delay creation of defs for expression-like things, including anon consts.
// This is because some anon consts end up as `ConstArgKind::Path`'s instead.
self.create_def(
self.current_def_id_parent,
c.id,
kw::Empty,
DefKind::AnonConst,
c.value.span,
);

self.arena.alloc(self.with_new_scopes(c.value.span, |this| {
let def_id = this.local_def_id(c.id);
Expand Down
10 changes: 2 additions & 8 deletions compiler/rustc_borrowck/src/diagnostics/region_name.rs
Original file line number Diff line number Diff line change
Expand Up @@ -829,20 +829,14 @@ impl<'tcx> MirBorrowckCtxt<'_, '_, '_, 'tcx> {
///
/// [`OpaqueDef`]: hir::TyKind::OpaqueDef
fn get_future_inner_return_ty(&self, hir_ty: &'tcx hir::Ty<'tcx>) -> &'tcx hir::Ty<'tcx> {
let hir = self.infcx.tcx.hir();

let hir::TyKind::OpaqueDef(id, _, _) = hir_ty.kind else {
let hir::TyKind::OpaqueDef(opaque_ty, _, _) = hir_ty.kind else {
span_bug!(
hir_ty.span,
"lowered return type of async fn is not OpaqueDef: {:?}",
hir_ty
);
};
let opaque_ty = hir.item(id);
if let hir::ItemKind::OpaqueTy(hir::OpaqueTy {
bounds: [hir::GenericBound::Trait(trait_ref, _)],
..
}) = opaque_ty.kind
if let hir::OpaqueTy { bounds: [hir::GenericBound::Trait(trait_ref, _)], .. } = opaque_ty
&& let Some(segment) = trait_ref.trait_ref.path.segments.last()
&& let Some(args) = segment.args
&& let [constraint] = args.constraints
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_borrowck/src/region_infer/opaque_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -328,8 +328,8 @@ fn check_opaque_type_well_formed<'tcx>(
) -> Result<Ty<'tcx>, ErrorGuaranteed> {
// Only check this for TAIT. RPIT already supports `tests/ui/impl-trait/nested-return-type2.rs`
// on stable and we'd break that.
let opaque_ty_hir = tcx.hir().expect_item(def_id);
let OpaqueTyOrigin::TyAlias { .. } = opaque_ty_hir.expect_opaque_ty().origin else {
let opaque_ty_hir = tcx.hir().expect_opaque_ty(def_id);
let OpaqueTyOrigin::TyAlias { .. } = opaque_ty_hir.origin else {
return Ok(definition_ty);
};
let param_env = tcx.param_env(def_id);
Expand Down
15 changes: 8 additions & 7 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2727,6 +2727,8 @@ pub struct BareFnTy<'hir> {

#[derive(Debug, Clone, Copy, HashStable_Generic)]
pub struct OpaqueTy<'hir> {
pub hir_id: HirId,
pub def_id: LocalDefId,
pub generics: &'hir Generics<'hir>,
pub bounds: GenericBounds<'hir>,
pub origin: OpaqueTyOrigin,
Expand All @@ -2744,6 +2746,7 @@ pub struct OpaqueTy<'hir> {
/// originating from a trait method. This makes it so that the opaque is
/// lowered as an associated type.
pub in_trait: bool,
pub span: Span,
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
Expand Down Expand Up @@ -2835,7 +2838,7 @@ pub enum TyKind<'hir> {
/// possibly parameters) that are actually bound on the `impl Trait`.
///
/// The last parameter specifies whether this opaque appears in a trait definition.
OpaqueDef(ItemId, &'hir [GenericArg<'hir>], bool),
OpaqueDef(&'hir OpaqueTy<'hir>, &'hir [GenericArg<'hir>], bool),
/// A trait object type `Bound1 + Bound2 + Bound3`
/// where `Bound` is a trait or a lifetime.
TraitObject(
Expand Down Expand Up @@ -3302,8 +3305,6 @@ impl<'hir> Item<'hir> {
expect_ty_alias, (&'hir Ty<'hir>, &'hir Generics<'hir>),
ItemKind::TyAlias(ty, generics), (ty, generics);

expect_opaque_ty, &OpaqueTy<'hir>, ItemKind::OpaqueTy(ty), ty;

expect_enum, (&EnumDef<'hir>, &'hir Generics<'hir>), ItemKind::Enum(def, generics), (def, generics);

expect_struct, (&VariantData<'hir>, &'hir Generics<'hir>),
Expand Down Expand Up @@ -3416,8 +3417,6 @@ pub enum ItemKind<'hir> {
GlobalAsm(&'hir InlineAsm<'hir>),
/// A type alias, e.g., `type Foo = Bar<u8>`.
TyAlias(&'hir Ty<'hir>, &'hir Generics<'hir>),
/// An opaque `impl Trait` type alias, e.g., `type Foo = impl Bar;`.
OpaqueTy(&'hir OpaqueTy<'hir>),
/// An enum definition, e.g., `enum Foo<A, B> {C<A>, D<B>}`.
Enum(EnumDef<'hir>, &'hir Generics<'hir>),
/// A struct definition, e.g., `struct Foo<A> {x: A}`.
Expand Down Expand Up @@ -3461,7 +3460,6 @@ impl ItemKind<'_> {
ItemKind::Fn(_, ref generics, _)
| ItemKind::TyAlias(_, ref generics)
| ItemKind::Const(_, ref generics, _)
| ItemKind::OpaqueTy(OpaqueTy { ref generics, .. })
| ItemKind::Enum(_, ref generics)
| ItemKind::Struct(_, ref generics)
| ItemKind::Union(_, ref generics)
Expand All @@ -3484,7 +3482,6 @@ impl ItemKind<'_> {
ItemKind::ForeignMod { .. } => "extern block",
ItemKind::GlobalAsm(..) => "global asm item",
ItemKind::TyAlias(..) => "type alias",
ItemKind::OpaqueTy(..) => "opaque type",
ItemKind::Enum(..) => "enum",
ItemKind::Struct(..) => "struct",
ItemKind::Union(..) => "union",
Expand Down Expand Up @@ -3769,6 +3766,7 @@ pub enum Node<'hir> {
Ty(&'hir Ty<'hir>),
AssocItemConstraint(&'hir AssocItemConstraint<'hir>),
TraitRef(&'hir TraitRef<'hir>),
OpaqueTy(&'hir OpaqueTy<'hir>),
Pat(&'hir Pat<'hir>),
PatField(&'hir PatField<'hir>),
Arm(&'hir Arm<'hir>),
Expand Down Expand Up @@ -3834,6 +3832,7 @@ impl<'hir> Node<'hir> {
| Node::Crate(..)
| Node::Ty(..)
| Node::TraitRef(..)
| Node::OpaqueTy(..)
| Node::Infer(..)
| Node::WhereBoundPredicate(..)
| Node::ArrayLenInfer(..)
Expand Down Expand Up @@ -3954,6 +3953,7 @@ impl<'hir> Node<'hir> {
| Node::TraitItem(TraitItem { generics, .. })
| Node::ImplItem(ImplItem { generics, .. }) => Some(generics),
Node::Item(item) => item.kind.generics(),
Node::OpaqueTy(opaq) => Some(opaq.generics),
_ => None,
}
}
Expand Down Expand Up @@ -4013,6 +4013,7 @@ impl<'hir> Node<'hir> {
expect_ty, &'hir Ty<'hir>, Node::Ty(n), n;
expect_assoc_item_constraint, &'hir AssocItemConstraint<'hir>, Node::AssocItemConstraint(n), n;
expect_trait_ref, &'hir TraitRef<'hir>, Node::TraitRef(n), n;
expect_opaque_ty, &'hir OpaqueTy<'hir>, Node::OpaqueTy(n), n;
expect_pat, &'hir Pat<'hir>, Node::Pat(n), n;
expect_pat_field, &'hir PatField<'hir>, Node::PatField(n), n;
expect_arm, &'hir Arm<'hir>, Node::Arm(n), n;
Expand Down
Loading