Lower only one HIR owner at a time

[cjgillot]

Based on #83723
Additional diff is here: cjgillot/rust@ownernode...lower-mono

Lowering is very tangled and has a tendency to intertwine the transformation of different items. This PR aims at simplifying the logic by:

• moving global analyses to the resolver (item_generics_num_lifetimes, proc_macros, trait_impls);
• removing a few special cases (non-exported macros and use statements);
• restricting the amount of available information at any one time;
• avoiding back-and-forth between different owners: an item must now be lowered all at once, and its parent cannot refer to its nodes.

I also removed the sorting of bodies by span. The diagnostic ordering changes marginally, since definitions are pretty much sorted already according to the AST. This uncovered a subtlety in thir-unsafeck.

(While these items could logically be in different PRs, the dependency between commits and the amount of conflicts force a monolithic PR.)

[rust-highfive]

Some changes occurred in src/tools/clippy.

cc @rust-lang/clippy

[rust-highfive]

r? @davidtwco

(rust-highfive has picked a reviewer for you, use r? to override)

[jyn514]

cc @petrochenkov since this moves some things to rustc_resolve

[petrochenkov]

Blocked on #84373.

[petrochenkov]

Wait a second, this PR is not actually based on #84373 like the description says.
Is it ready for review?

UPD: Looks like you've meant #83723.

[petrochenkov]

@cjgillot
Could you explain what's going on with the "HirId preallocation" changes in more details?
Why were the HirIds pre-allocated? Why don't they need to be pre-allocated now? When they are allocated if they are not pre-allocated? (What does it even mean to allocate a HirId?)
Why is item_local_id_counters keyed on NodeIds and not LocalDefIds despite the NodeIds having the owner semantics?

[cjgillot]

@cjgillot
Could you explain what's going on with the "HirId preallocation" changes in more details?
Why were the HirIds pre-allocated? Why don't they need to be pre-allocated now? When they are allocated if they are not pre-allocated? (What does it even mean to allocate a HirId?)

By "HirId preallocation", for lack of a better word, I mean calls to allocate_hir_id_counter.
This pre-allocation basically declared to the lowering context that such NodeId was a HIR owner, and had to be lowered as such.
For item-likes, this was not necessary: we know that the nodes are only lowered when lowering the item itself, or when lowering a reference to the item. Calling allocate_hir_id_counter at that moment is enough.
For use items, the situation is essentially the same, with the additional subtlety that each use is associated with several definitions, and only part of those definitions actually get referenced in the HIR.

Why is item_local_id_counters keyed on NodeIds and not LocalDefIds despite the NodeIds having the owner semantics?

I had changed that before messing up the rebase.

[petrochenkov]

Suggested change

	parent_hir_id: LocalDefId,
	parent_def_id: LocalDefId,

[petrochenkov]

Suggested change

	let new_hir_id = self.resolver.local_def_id(id);
	let new_def_id = self.resolver.local_def_id(id);

[petrochenkov]

@bors r+

[bors]

📌 Commit 11024b2 has been approved by petrochenkov

[bors]

⌛ Testing commit 11024b2 with merge 7da30f2d33a61ad8b1742097dd71ffc698d1dc8c...

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[cjgillot]

@bors retry

[bors]

⌛ Testing commit 11024b2 with merge 49c0861...

[bors]

☀️ Test successful - checks-actions
Approved by: petrochenkov
Pushing 49c0861 to master...

[pnkfelix]

I think this commit was one that was left out of the normal perf service.

Here is what the weekly perf triage report says about it:

Lower only one HIR owner at a time #87234

• Large improvement in instruction counts (up to -1.3% on full builds of unused-warnings)
• Small regression in instruction counts (up to 0.4% on incr-unchanged builds of helloworld)

Looking at the results by eye, I'd say its clearly a big win.

@rustbot label: perf-regression-triaged