-
-
Notifications
You must be signed in to change notification settings - Fork 14.7k
refactor lint handling to be more "eager" #42511
Copy link
Copy link
Closed
Labels
A-incr-compArea: Incremental compilationArea: Incremental compilationA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.Area: Lints (warnings about flaws in source code) such as unused_mut.C-cleanupCategory: PRs that clean code up or issues documenting cleanup.Category: PRs that clean code up or issues documenting cleanup.E-hardCall for participation: Hard difficulty. Experience needed to fix: A lot.Call for participation: Hard difficulty. Experience needed to fix: A lot.E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.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
A-incr-compArea: Incremental compilationArea: Incremental compilationA-lintsArea: Lints (warnings about flaws in source code) such as unused_mut.Area: Lints (warnings about flaws in source code) such as unused_mut.C-cleanupCategory: PRs that clean code up or issues documenting cleanup.Category: PRs that clean code up or issues documenting cleanup.E-hardCall for participation: Hard difficulty. Experience needed to fix: A lot.Call for participation: Hard difficulty. Experience needed to fix: A lot.E-mentorCall for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion.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.
Lint handling at present works like this:
NodeIdthat identifies the AST/HIR node that the lint is attached to, along with the lint name, message, etc.This is not a very good setup. Among other things:
#[allow]. Wasteful.@eddyb proposed an alternative architecture that I think is better.
The best way to store this lint table is not 100% clear. Something sparse seems obviously like a good idea, since most nodes in the HIR do not have lint-related attributes on them (and hence just inherit the settings from their parent). I think maybe having each item store a link to the "current" settings for each lint makes sense; this can be easily shared between items. Then, within an item, if we need to compute the settings for some particular node, we would walk up the parents in the HIR map, looking for lint-related attributes and -- when we reach the item -- just using the settings stored on the item itself.
Alternatively, we could just cache nothing and compute everything this way, at least to start. If more caching is desired, maybe have passes keep a local cache that would then be populated within the things within a particular item.
I'm available to mentor this. It is a bit of a "design needed" sort of job, so I'm not sure what the immediate instructions would be. The first step is obviously familiarizing yourself with the current setup. After that, I would probably try to make a commit that adds a method to the HIR map that will compute the innermost applicable lint setting for a particular node (just by walking up the parents). We could then use this and remove all the existing lint code for tracking. That should work, but be slow (O(n^2) lookups). Presumably we can then adopt some sort of caching strategy.
@eddyb also mentioned that for bonus points, we should probably "decode" attributes during HIR lowering more generally, so that we are not walking the general attribute data structures, but rather something specific to the stuff the compiler cares about.