-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
Replace DefId with LocalDefId where possible #70853
Copy link
Copy link
Closed
Labels
C-cleanupCategory: PRs that clean code up or issues documenting cleanup.Category: PRs that clean code up or issues documenting cleanup.E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.E-help-wantedCall for participation: Help is requested to fix this issue.Call for participation: Help is requested to fix this issue.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Metadata
Metadata
Assignees
Labels
C-cleanupCategory: PRs that clean code up or issues documenting cleanup.Category: PRs that clean code up or issues documenting cleanup.E-easyCall for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.E-help-wantedCall for participation: Help is requested to fix this issue.Call for participation: Help is requested to fix this issue.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.Relevant to the compiler team, which will review and decide on the PR/issue.
Type
Fields
Give feedbackNo fields configured for issues without a type.
In the compiler, a
DefIdis used to lookup a single "definition" (type, method,const, etc.) somewhere in the code. It can refer to definitions in the crate currently being compiled or to definitions in other crates. There are quite a few places in the compiler which will only work if passed a localDefId--maybe they need to access the HIR for that definition, which is only available in the current crate--but acceptDefIdas a parameter. These places should useLocalDefIdinstead.To resolve this issue, you need to find functions or methods that will panic if a
DefIdis not local. Such places should be callingDefId::expect_localand then working with the returnedLocalDefId, but you are more likely to see older idioms (e.g.,tcx.as_local_hir_id(def_id).unwrap()). Code like this should be refactored to take aLocalDefIdinstead of aDefIdand their caller made responsible for asserting that a givenDefIdis local. The end goal is to move the call toexpect_localas high up in the call graph as we can. If possible, it should be the first thing we do when executing a query like so,rust/src/librustc_mir/const_eval/fn_queries.rs
Line 174 in 4015890
Ideally this would be done module-by-module so it can be reviewed more easily (not in a single, giant PR). See the last commit in #66132 for prior art.
This issue has been assigned to @marmeladema via this comment.