Add some docs around the lint store #476
Conversation
There was a problem hiding this comment.
nice, @Mark-Simulacrum! I left some feedback to maybe improve it a bit? Let me know what you think.
| macro. This includes the name, the default level, a short description, and some | ||
| more details. | ||
|
|
||
| Note that the lint and the lint pass must be registered with the compiler. |
There was a problem hiding this comment.
can we maybe link to a method in the rustc API docs here? e.g., register_lints?
src/diagnostics/lintstore.md
Outdated
| The `LintStore` is the central piece of infrastructure, around which everything | ||
| rotates. It's not available during the early parts of compilation (i.e., before | ||
| TyCtxt) in most of the code, as we need to fill it in with all of the lints, | ||
| which can only happen after plugin registration. |
There was a problem hiding this comment.
Can you describe where the LintStore is created and how it eventually gets frozen? IIRC, it is created by the plugin registrar (where it is mutable), but then it is "frozen" (put into a Rc) and given to the tcx?
src/diagnostics/lintstore.md
Outdated
| TyCtxt) in most of the code, as we need to fill it in with all of the lints, | ||
| which can only happen after plugin registration. | ||
|
|
||
| We store essentially two separate things in the `LintStore`: a list of all known |
There was a problem hiding this comment.
I think I would move the ## Registration section up here -- basically say:
During the mutable period, we can register lints by providing a "constructor function", like so:
This existing paragraph says "We store constructors for the lint passes rather than the passes
themselves" but this feels (to me) like something that makes more sense if you happen to be familiar with the old code than from first principles. i.e., there is nothing all that strange about the fact that we register a function that can produce a lint (also, it's not always a function right? it's basically any closure?)
So I would just describe it as is: to register the lint, you give metadata + a constructor. When it is time for us to run the lints, we invoke the closure and get back a Box<dyn LintPass> value. We can then invoke the visitor methods on this lint pass. Note that the lint pass methods get &mut self so the lint can keep internal, mutable state that it updates as we traverse the input program.
| `register_lints(PassType::get_lints())` and then `register_late_pass(|| | ||
| Box::new(PassType))`. | ||
|
|
||
| Within the compiler, for performance reasons, we usually do not register dozens |
There was a problem hiding this comment.
This is a good detail to include. I would add a section header before it like
## Compiler lint passes are combined into one pass
Co-Authored-By: Niko Matsakis <niko@alum.mit.edu>
10e6c4f to
d97ddcd
Compare
|
Okay, I've pretty much restructured the text completely, let me know what you think. I tried to address the review comments, though some of them are sort of indirectly addressed. |
|
I plan to do some additional work around lint buffering -- but I think having that happen in a separate PR to have at least some docs sooner makes sense? |
|
|
||
| The lint store is created and all lints are registered during plugin registration, in | ||
| [`rustc_interface::register_plugins`]. There are three 'sources' of lint: the internal lints, plugin | ||
| lints, and `rustc_interface::Config` `register_lints`. All are registered here, in |
| auto-propagate into the other. | ||
|
|
||
| [`LintStore::register_lint`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/lint/struct.LintStore.html#method.register_lints | ||
| [`rustc_interface::register_plugins`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/passes/fn.register_plugins.html) |
| [`rustc_lint::register_builtins`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/fn.register_builtins.html | ||
| [`rustc_lint::register_internals`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/fn.register_internals.html | ||
| [`rustc_lint::new_lint_store`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/fn.new_lint_store.html | ||
| [`rustc::declare_lint!`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/macro.declare_lint.html |
| [`rustc_lint::register_internals`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/fn.register_internals.html | ||
| [`rustc_lint::new_lint_store`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/fn.new_lint_store.html | ||
| [`rustc::declare_lint!`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/macro.declare_lint.html | ||
| [`rustc::declare_tool_lint!`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/macro.declare_tool_lint.html |
* Add some docs around the lint store * Update src/diagnostics.md Co-Authored-By: Niko Matsakis <niko@alum.mit.edu> * restructure
r? (?) @nikomatsakis
This accompanies rust-lang/rust#65193. I would appreciate thoughts as to what we should be documenting (and how) -- once we figure things out here I'll also delete https://forge.rust-lang.org/compiler/bug-fix-procedure.html#issuing-future-compatibility-warnings as I believe it's subsumed by the documentation in rustc-guide.