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

Assign HirIds in HIR traversal order #102522

Closed
cjgillot opened this issue Sep 30, 2022 · 8 comments
Closed

Assign HirIds in HIR traversal order #102522

cjgillot opened this issue Sep 30, 2022 · 8 comments
Labels
A-hir Area: The high-level intermediate representation (HIR) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@cjgillot
Copy link
Contributor

cjgillot commented Sep 30, 2022

AST->HIR lowering assigns a HirId to most nodes in the HIR tree. The HirId consists of an OwnerId, which tells to which item (function, trait, associated function...) a node belongs to, and an ItemLocalId which is its index inside that OwnerId.

ItemLocalIds are assigned in the order they are created during lowering, by a simple counter. As a consequence, their values when looking at the HIR tree do not follow a consistent order. Making that order consistent may help simplifying some code that needs to visit HIR when it may just iterate on ItemLocalIds.

Steps:

  • in rustc_passes::hir_id_validator, add a check that the ItemLocalId of the parent of each node has a lower value that the ItemLocalId of the node itself;
  • change rustc_ast_lowering, in particular calls to lower_node_id and next_id until the tests pass.

Bonus: manage to get the order of ItemLocalIds be the order in which a HIR visitor (defined in rustc_hir::intravisit) visits all the HirIds.

I recommend using -Zunpretty=hir-tree to debug. Please contact me on zulip for any question.

@cjgillot cjgillot added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. A-hir Area: The high-level intermediate representation (HIR) E-help-wanted Call for participation: Help is requested to fix this issue. labels Sep 30, 2022
@chenyukang
Copy link
Member

@rustbot claim
I'm learning the HIR part, will take some time for this.

@ericmcbride
Copy link

@rustbot claim

@Enselic Enselic added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Oct 22, 2023
@DavidCoy77
Copy link

Hi! I'd love to get started contributing to Rust, but I'd need a little help getting started since this would be my first ever GitHub contribution. You've included the tags Easy, Help-Wanted, and Mentor, so if you're willing to help, I'm eager to get to work. Thanks!

@the-shank
Copy link

This hasn't been assigned to anyone yet. @DavidCoy77 are you working on this? In case you are not working on this, I would like to pick this up.

@axelmagn
Copy link

axelmagn commented Jan 5, 2024

@rustbot claim

Since there has been no activity on this since November, I am assuming that there is no work in-flight. Please let me know if that's not the case.

I am a first-time contributor looking for a good mentorship project, and this fits my interest area.

@axelmagn
Copy link

axelmagn commented Jan 6, 2024

Dropping this issue after conversation with @cjgillot in favor of #99669 (higher priority)

@rustbot release-assignment

@the-shank
Copy link

@rustbot claim

@cjgillot I am a first-time contributor and would like to work on this.

@cjgillot
Copy link
Contributor Author

cjgillot commented Jan 6, 2024

Closing as not planned. It's really hard (= mostly rewrites lowering to change the id generation order), with very uncertain benefits (= simplify lint level computation).

@cjgillot cjgillot closed this as completed Jan 6, 2024
@the-shank the-shank removed their assignment Jan 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-hir Area: The high-level intermediate representation (HIR) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants