From 54d581babf45b513f1da47b85911996908d85f06 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Tue, 22 Oct 2019 18:07:04 -0400 Subject: [PATCH 1/3] Add some docs around the lint store --- src/SUMMARY.md | 1 + src/diagnostics.md | 51 +++++++++++++++++++++++++++--------- src/diagnostics/lintstore.md | 40 ++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 src/diagnostics/lintstore.md diff --git a/src/SUMMARY.md b/src/SUMMARY.md index fdec4ffde..770686f01 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -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) diff --git a/src/diagnostics.md b/src/diagnostics.md index 0a21c91dc..bf1cec8d9 100644 --- a/src/diagnostics.md +++ b/src/diagnostics.md @@ -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 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. For example, the following lint checks for uses of `while true { ... }` and suggests using `loop { ... }` instead. @@ -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 @@ -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 ", + 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 diff --git a/src/diagnostics/lintstore.md b/src/diagnostics/lintstore.md new file mode 100644 index 000000000..b12f90e34 --- /dev/null +++ b/src/diagnostics/lintstore.md @@ -0,0 +1,40 @@ +# LintStore + +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 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 +lints with associated metadata (default lint levels, notably), and the lint +passes. We store constructors for the lint passes rather than the passes +themselves, because lint passes often want to change state to track something +in the code (so their methods take `&mut self`) and we want to avoid needing to +synchronize access to the LintStore. + +For the most part, we currently only instantiate each lint pass once, though in +the future we'll likely want more fine-grained linting to better support +incremental compilation. We already have late module passes which are +constructed per-module if the module changes in an incremental session. + +## Registration + +We must register both every lint and every lint pass into the `LintStore` for +the code to work properly. This often is as simple as calling +`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 +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`. From ac4f927f6d9f85bc282c55e5ccb7ac92b0304d4c Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 18:58:04 -0400 Subject: [PATCH 2/3] Update src/diagnostics.md Co-Authored-By: Niko Matsakis --- src/diagnostics.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/diagnostics.md b/src/diagnostics.md index bf1cec8d9..785dca73c 100644 --- a/src/diagnostics.md +++ b/src/diagnostics.md @@ -174,7 +174,7 @@ crate. 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 the linter walks the source code. You can +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!` From d97ddcd3c668c76788a093be182a34e3ee7248e0 Mon Sep 17 00:00:00 2001 From: Mark Rousskov Date: Fri, 25 Oct 2019 21:20:15 -0400 Subject: [PATCH 3/3] restructure --- src/diagnostics/lintstore.md | 109 ++++++++++++++++++++++++++++------- 1 file changed, 87 insertions(+), 22 deletions(-) diff --git a/src/diagnostics/lintstore.md b/src/diagnostics/lintstore.md index b12f90e34..50538c2b5 100644 --- a/src/diagnostics/lintstore.md +++ b/src/diagnostics/lintstore.md @@ -1,40 +1,105 @@ -# LintStore +# 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 of the code, as we need to fill it in with all of the lints, -which can only happen after plugin registration. +TyCtxt) in most 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 -lints with associated metadata (default lint levels, notably), and the lint -passes. We store constructors for the lint passes rather than the passes -themselves, because lint passes often want to change state to track something -in the code (so their methods take `&mut self`) and we want to avoid needing to -synchronize access to the LintStore. +## Lints vs. lint passes -For the most part, we currently only instantiate each lint pass once, though in -the future we'll likely want more fine-grained linting to better support -incremental compilation. We already have late module passes which are -constructed per-module if the module changes in an incremental session. +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 -We must register both every lint and every lint pass into the `LintStore` for -the code to work properly. This often is as simple as calling -`register_lints(PassType::get_lints())` and then `register_late_pass(|| -Box::new(PassType))`. +### 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 +`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`, 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 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. +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`. +`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) +[`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::declare_tool_lint!`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc/macro.declare_tool_lint.html +[`register_lints`]: https://doc.rust-lang.org/nightly/nightly-rustc/rustc_interface/interface/struct.Config.html#structfield.register_lints