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

Add {Local}ModDefId to more more strongly type DefIds #110862

Closed
wants to merge 2 commits into from

Conversation

Noratrieb
Copy link
Member

Currently, the compiler just uses DefId everywhere. This is bad for correctness (you just get ICEs instead of compiler errors when passing an ID of the wrong kind) and documentation (the type doesn't document which kinds are expected).

Instead, we should be using more strongly typed versions.

This PR adds such versions for modules and uses them in a bunch of places (not everywhere though).

I will open an MCP with more detail for doing this more generally.

This allows for better type safety in the compiler and also improves the
documentation for many things, making it clear that they expect modules.
@rustbot
Copy link
Collaborator

rustbot commented Apr 26, 2023

r? @lcnr

(rustbot has picked a reviewer for you, use r? to override)

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Apr 26, 2023
@lcnr
Copy link
Contributor

lcnr commented Apr 26, 2023

I think this has to wait for the MCP to finish? Won't have time to get to this until the end of next week so please reassign if you want some progress until then

@lcnr lcnr added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Apr 26, 2023
};
}

typed_def_id! { ModDefId, LocalModDefId }
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for implementation: you'll need to add impl DepNodeParams for all the created types, mirroring the one on LocalDefId. This is meant to allow the query system to directly force a query that takes a LocalModId, instead of starting at the last one that took a LocalDefId.

commented by @cjgillot on Zulip, mirroring it here to remember it

@bors
Copy link
Contributor

bors commented May 4, 2023

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

GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Aug 15, 2023
…aber

Add `{Local}ModDefId` to more strongly type DefIds`

Based on rust-lang#110862 by `@Nilstrieb`
@Noratrieb
Copy link
Member Author

superseeded by #114772

@Noratrieb Noratrieb closed this Aug 17, 2023
@Noratrieb Noratrieb deleted the typed-did branch August 17, 2023 18:12
flip1995 pushed a commit to flip1995/rust that referenced this pull request Aug 24, 2023
…aber

Add `{Local}ModDefId` to more strongly type DefIds`

Based on rust-lang#110862 by `@Nilstrieb`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants