Skip to content

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

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.

Sign up for GitHub

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

Closed
camelid wants to merge 17 commits into from
Closed

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

Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
73be83c
Delay def collection for expression-like nodes
camelid Aug 8, 2024
60bc83d
Create defs before lowering NodeId
camelid Aug 8, 2024
d1b810a
Create defs for generators
camelid Aug 8, 2024
7e9fbf7
Create def for implicit closure in async fn
camelid Aug 8, 2024
6c70fc3
Bless tests
camelid Aug 8, 2024
a146e1d
Remove extraneous with_def_id_parent call
camelid Aug 8, 2024
a36ce9b
Visit closures and opaque types in rustc_dump_def_parents
camelid Aug 8, 2024
60ddcf1
Use current_def_id_parent in more places
camelid Aug 8, 2024
79a3e7a
Show nested items in rustc_dump_def_parents
camelid Aug 8, 2024
134314a
wip: rm `ItemKind::OpaqueTy`
camelid Aug 10, 2024
d29f44a
Handle opaques in NodeCollector
camelid Aug 10, 2024
ca48eee
Fix nested behavior
camelid Aug 10, 2024
c8cefb2
INTRA not INTER
camelid Aug 10, 2024
d7b0b0e
Fix bound vars ICE
camelid Aug 10, 2024
96f38fc
Fix generics ICE
camelid Aug 10, 2024
dcb9e9a
Fix dead code analysis for opaque types
camelid Aug 11, 2024
6d13474
[do not merge] Bless impl-trait tests (incorrect!)
camelid Aug 11, 2024
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
6 changes: 3 additions & 3 deletions compiler/rustc_ast_lowering/src/lib.rs
View file
Open in desktop
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

@cjgillot cjgillot Aug 10, 2024

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

@BoxyUwU BoxyUwU Aug 10, 2024

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 @@ -1620,7 +1620,7 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
lower_item_bounds: impl FnOnce(&mut Self) -> &'hir [hir::GenericBound<'hir>],
) -> hir::TyKind<'hir> {
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,
Expand Down