Skip to content

Add some docs around the lint store #476

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

Merged
merged 3 commits into from
Oct 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/SUMMARY.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
- [Coding conventions](./conventions.md)
- [crates.io Dependencies](./crates-io.md)
- [Emitting Errors and other Diagnostics](diagnostics.md)
- [`LintStore`](./diagnostics/lintstore.md)
- [ICE-breaker teams](ice-breaker/about.md)
- [LLVM ICE-breakers](ice-breaker/llvm.md)
- [Part 2: How rustc works](./part-2-intro.md)
Expand Down
51 changes: 38 additions & 13 deletions src/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -171,11 +171,17 @@ crate.

[builtin]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_lint/index.html

Each lint is defined as a `struct` that implements the `LintPass` `trait`. The
trait implementation allows you to check certain syntactic constructs the
linter walks the source code. You can then choose to emit lints in a very
similar way to compile errors. Finally, you register the lint to actually get
it to be run by the compiler by using the `declare_lint!` macro.
Every lint is implemented via a `struct` that implements the `LintPass` `trait`
(you also implement one of the more specific lint pass traits, either
`EarlyLintPass` or `LateLintPass`). The trait implementation allows you to
check certain syntactic constructs as the linter walks the source code. You can
then choose to emit lints in a very similar way to compile errors.

You also declare the metadata of a particular lint via the `declare_lint!`
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.
Copy link
Contributor

Choose a reason for hiding this comment

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

can we maybe link to a method in the rustc API docs here? e.g., register_lints?


For example, the following lint checks for uses
of `while true { ... }` and suggests using `loop { ... }` instead.
Expand All @@ -196,11 +202,15 @@ declare_lint! {
#[derive(Copy, Clone)]
pub struct WhileTrue;

impl LintPass for WhileTrue {
fn get_lints(&self) -> LintArray {
lint_array!(WHILE_TRUE)
}
}
// This declares a lint pass, providing a list of associated lints. The
// compiler currently doesn't use the associated lints directly (e.g., to not
// run the pass or otherwise check that the pass emits the appropriate set of
// lints). However, it's good to be accurate here as it's possible that we're
// going to register the lints via the get_lints method on our lint pass (that
// this macro generates).
impl_lint_pass!(
WhileTrue => [WHILE_TRUE],
);

// LateLintPass has lots of methods. We only override the definition of
// `check_expr` for this lint because that's all we need, but you could
Expand Down Expand Up @@ -242,9 +252,24 @@ declare_lint! {
This makes the `ANONYMOUS_PARAMETERS` lint allow-by-default in the 2015 edition
but warn-by-default in the 2018 edition.

Lints that represent an incompatibility (i.e. error) in the upcoming edition
should also be registered as `FutureIncompatibilityLint`s in
[`register_builtins`][rbuiltins] function in [`rustc_lint::lib`][builtin].
A future-incompatible lint should be declared with the `@future_incompatible`
additional "field":

```rust,ignore
declare_lint! {
pub ANONYMOUS_PARAMETERS,
Allow,
"detects anonymous parameters",
@future_incompatible = FutureIncompatibleInfo {
reference: "issue #41686 <https://github.com/rust-lang/rust/issues/41686>",
edition: Some(Edition::Edition2018),
};
}
```

If you need a combination of options that's not supported by the
`declare_lint!` macro, you can always define your own static with a type of
`&Lint` but this is currently linted against in the compiler tree.

### Lint Groups

Expand Down
105 changes: 105 additions & 0 deletions src/diagnostics/lintstore.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
# Lints

This page documents some of the machinery around lint registration and how we
run lints in the compiler.

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 code, as we need to fill it in with all of the lints, which can only happen after
plugin registration.

## Lints vs. lint passes

There are two parts to the linting mechanism within the compiler: lints and lint passes.
Unfortunately, a lot of the documentation we have refers to both of these as just "lints."

First, we have the lint declarations themselves: this is where the name and default lint level and
other metadata come from. These are normally defined by way of the [`declare_lint!`] macro, which
boils down to a static with type `&rustc::lint::Lint`. We lint against direct declarations without
the use of the macro today (though this may change in the future, as the macro is somewhat unwieldy
to add new fields to, like all macros by example).

Lint declarations don't carry any "state" - they are merely global identifers and descriptions of
lints. We assert at runtime that they are not registered twice (by lint name).

Lint passes are the meat of any lint. Notably, there is not a one-to-one relationship between
lints and lint passes; a lint might not have any lint pass that emits it, it could have many, or
just one -- the compiler doesn't track whether a pass is in any way associated with a particular
lint, and frequently lints are emitted as part of other work (e.g., type checking, etc.).

## Registration

### High-level overview

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
Copy link
Contributor

Choose a reason for hiding this comment

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

[register_lints]

`register_plugins`.

Once the registration is complete, we "freeze" the lint store by placing it in an `Lrc`. Later in
the driver, it's passed into the `GlobalCtxt` constructor where it lives in an immutable form from
then on.

Lints are registered via the [`LintStore::register_lint`] function. This should
happen just once for any lint, or an ICE will occur.

Lint passes are registered separately into one of the categories (pre-expansion,
early, late, late module). Passes are registered as a closure -- i.e., `impl
Fn() -> Box<dyn X>`, where `dyn X` is either an early or late lint pass trait
object. When we run the lint passes, we run the closure and then invoke the lint
pass methods, which take `&mut self` -- lint passes can keep track of state
internally.

#### Internal lints

Note, these include both rustc-internal lints, and the traditional lints, like, for example the dead
code lint.

These are primarily described in two places: `rustc::lint::builtin` and `rustc_lint::builtin`. The
first provides the definitions for the lints themselves, and the latter provides the lint pass
definitions (and implementations).

The internal lint registration happens in the [`rustc_lint::register_builtins`] function, along with
the [`rustc_lint::register_internals`] function. More generally, the LintStore "constructor"
function which is *the* way to get a `LintStore` in the compiler (you should not construct it
directly) is [`rustc_lint::new_lint_store`]; it calls the registration functions.

#### Plugin lints

This is one of the primary use cases remaining for plugins/drivers. Plugins are given access to the
mutable `LintStore` during registration to call any functions they need on the `LintStore`, just
like rustc code. Plugins are intended to declare lints with the `plugin` field set to true (e.g., by
way of the [`declare_tool_lint!`] macro), but this is purely for diagnostics and help text;
otherwise plugin lints are mostly just as first class as rustc builtin lints.

#### Driver lints

These are the lints provided by drivers via the `rustc_interface::Config` [`register_lints`] field,
which is a callback. Drivers should, if finding it already set, call the function currently set
within the callback they add. The best way for drivers to get access to this is by overriding the
`Callbacks::config` function which gives them direct access to the `Config` structure.

## Compiler lint passes are combined into one pass

Within the compiler, for performance reasons, we usually do not register dozens
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a good detail to include. I would add a section header before it like

## Compiler lint passes are combined into one pass

of lint passes. Instead, we have a single lint pass of each variety
(e.g. `BuiltinCombinedModuleLateLintPass`) which will internally call all of the
individual lint passes; this is because then we get the benefits of static over
dynamic dispatch for each of the (often empty) trait methods.

Ideally, we'd not have to do this, since it certainly adds to the complexity of
understanding the code. However, with the current type-erased lint store
approach, it is beneficial to do so for performance reasons.

New lints being added likely want to join one of the existing declarations like
`late_lint_mod_passes` in `librustc_lint/lib.rs`, which would then
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)
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove )

[`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
Copy link
Contributor

Choose a reason for hiding this comment

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

[`declare_lint!`]:

[`rustc::declare_tool_lint!`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/macro.declare_tool_lint.html
Copy link
Contributor

Choose a reason for hiding this comment

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

[`declare_tool_lint!`]:

[`register_lints`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/interface/struct.Config.html#structfield.register_lints