rustc: rewrite the HirId validator to only check HIR map compactness.

[eddyb]

This should be cheaper than the current validator, and not assume item-like owners, but I did have to add a few more kinds of nodes to the HIR map, so that every node with a HirId is in it.

AFAICT, collecting the HIR map in the first place already checks owners, and I didn't want to duplicate any complications to that for #63849 (comment) (making all nodes with a DefId into HirId owners).

I've also cleaned up the errors to be more useful in determining the "neighborhood" of nodes around missing HirIds, at a glance (we can probably do even better but there's little point at this time).

r? @michaelwoerister cc @nikomatsakis

[eddyb]

r? @michaelwoerister

[eddyb]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit d85eab33a7ee8d87f351b8debc8c337d67b49282 with merge 4c60a7cbc36d1e73e33c8faf96f5eed91bb95bf9...

[eddyb]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion

[bors]

⌛ Trying commit 26e071163a2f7f97c9bcbddd7bc4841392c52951 with merge 5d5564441d81a42987f56955337921ed997ea920...

[bors]

☀️ Try build successful - checks-azure
Build commit: 5d5564441d81a42987f56955337921ed997ea920 (5d5564441d81a42987f56955337921ed997ea920)

[rust-timer]

Queued 5d5564441d81a42987f56955337921ed997ea920 with parent 084edc4, future comparison URL.

[rust-timer]

Finished benchmarking try commit 5d5564441d81a42987f56955337921ed997ea920, comparison URL.

[michaelwoerister]

Thanks, @eddyb! I've added it to my review queue.

[eddyb]

I think I've found another way of keeping the HIR validator working with non-item-like HirId owners (by adding a new method to hir::intravisit::Visitor for handling entering a new HirId owner), but for the validator itself that's probably unnecessary (as hir::map::collector already checks owners).

I'll probably use that technique to simplify various other visitors, so that they don't end up hardcoding what's an owner and what's not (eventually we might have a "def" concept that replaces it).

EDIT: I've since realized that it would be easier and more productive to just add a hir::Owner (or hir::Root) enum, and let visitors match on it if they want.

[bors]

☔ The latest upstream changes (presumably #66180) made this pull request unmergeable. Please resolve the merge conflicts.

[joelpalmer]

Ping from Triage: any updates @eddyb?

[eddyb]

This has been waiting for a review from @michaelwoerister since #65837 (comment).

[michaelwoerister]

Yeah, I need to page in a lot of information in order to review this and haven't had the time to do that yet.

[michaelwoerister]

Before I spend too much time on finding out how the HirId validator works in detail, would it be an option to just hide validation behind a debug_assertion? That would improve performance and we'd still catch errors on CI, right?

[JohnCSimon]

Ping from triage
@eddyb - can you please address the merge conflicts?
Thank you!

[JohnCSimon]

Pinging again from triage
@eddyb - can you please address the merge conflicts?
Thank you!

[JohnCSimon]

Pinging again from triage -
@eddyb - can you please address the comment here #65837 (comment)
and fix the merge conflicts?

[joelpalmer]

Ping from Triage: closing due to inactivity. Please re-open with changes @eddyb - Thanks for the PR