rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
98.4k stars 12.73k forks source link

Forbid lowering the same `NodeId` multiple times #96346

Closed cjgillot closed 2 weeks ago

cjgillot commented 2 years ago

During AST->HIR lowering, the method lower_node_id is tasked to transform NodeIds that identify AST nodes into HirIds that identify HIR nodes. This method maintains a node_id_to_local_id: FxHashMap<NodeId, hir::ItemLocalId> to remember this mapping.

However, this mapping is not entirely useful, and mostly exists (1) to make the developer's life easier, (2) to lower resolutions to local bindings Res::Local in lower_res. The mapping node_id_to_local_id should be removed, and multiple calls to local_node_id with the same NodeId should be forbidden. For usage (2), Res::Local only appears for ident patterns (PatKind::Ident) which are lowered by lower_pat_ident. Hence, lower_res should use a dedicated hash-map filled by lower_pat_ident.

Furthermore, next_id calls lower_node_id with a node_id: NodeId which is entirely local to that function, and will never be known to any other code. Manipulations of node_id_to_local_id there are entirely useless.

Instructions:

kckeiks commented 2 years ago

@rustbot claim

raiyansayeed commented 2 years ago

Hey @kckeiks , are you still working on this task by chance?

kckeiks commented 2 years ago

@raiyansayeed I was but I have a couple of other issues that I'm working on so you can take this one if you want.

spastorino commented 2 years ago

@raiyansayeed feel free to ping me too on Zulip if you need help or something.

raiyansayeed commented 2 years ago

@rustbot claim

trevyn commented 2 years ago

@raiyansayeed are you still working on this?

Noratrieb commented 1 year ago

No activity, feel free to claim it again. @rustbot release-assignment

azadnn commented 1 year ago

@rustbot claim

azadnn commented 1 year ago

https://rust-lang.zulipchat.com/#narrow/stream/315146-t-compiler.2Fetc.2Fincremental-hir/topic/reduce.2Feliminate.20duplicate.20invocation.20of.20.60lower_node_id.60/near/324846414

adwinwhite commented 2 months ago

@rustbot claim