diff --git a/rustc-bug-fix-procedure.md b/rustc-bug-fix-procedure.md index 90d0d7b5d..09e207507 100644 --- a/rustc-bug-fix-procedure.md +++ b/rustc-bug-fix-procedure.md @@ -230,49 +230,115 @@ process that we use for unstable features: Ideally, breaking changes should have landed on the **stable branch** of the compiler before they are finalized. -### Batching breaking changes to libsyntax + -Due to the lack of stable plugins, making changes to libsyntax can -currently be quite disruptive to the ecosystem that relies on plugins. -In an effort to ease this pain, we generally try to batch up such -changes so that they occur all at once, rather than occuring in a -piecemeal fashion. In practice, this means that you should add: +### Removing a lint - cc #31645 @Manishearth - -to the PR and avoid directly merging it. In the future we may develop -a more polished procedure here, but the hope is that this is a -relatively temporary state of affairs. +Once we have decided to make a "future warning" into a hard error, we +need a PR that removes the custom lint. As an example, here are the +steps required to remove the `overlapping_inherent_impls` +compatibility lint. First, convert the name of the lint to uppercase +(`OVERLAPPING_INHERENT_IMPLS`) ripgrep thorugh the source for that +string. We will basically by converting each place where this lint +name is mentioned (in the compiler, we use the upper-case name, and a +macro automatically generates the lower-case string; so searching for +`overlapping_inherent_impls` would not find much). -# Drawbacks -[drawbacks]: #drawbacks +#### Remove the lint. -Following this policy can require substantial effort and slows the -time it takes for a change to become final. However, this is far -outweighed by the benefits of avoiding sharp disruptions in the -ecosystem. +The first reference you will likely find is the lint definition +[in `librustc/lint/builtin.rs` that resembles this][defsource]: -# Alternatives -[alternatives]: #alternatives +[defsource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc/lint/builtin.rs#L171-L175 + +```rust +declare_lint! { + pub OVERLAPPING_INHERENT_IMPLS, + Deny, // this may also say Warning + "two overlapping inherent impls define an item with the same name were erroneously allowed" +} +``` + +This `declare_lint!` macro creates the relevant data +structures. Remove it. You will also find that there is a mention of +`OVERLAPPING_INHERENT_IMPLS` later in the file as +[part of a `lint_array!`][lintarraysource]; remove it too, + +[lintarraysource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc/lint/builtin.rs#L252-L290 + +Next, you see see +[a reference to `OVERLAPPING_INHERENT_IMPLS` in `librustc_lint/lib.rs`][futuresource]. This +defining the lint as a "future compatibility lint": -There are obviously many points that we could tweak in this policy: +```rust +FutureIncompatibleInfo { + id: LintId::of(OVERLAPPING_INHERENT_IMPLS), + reference: "issue #36889 ", +}, +``` + +Remove this too. + +#### Add the lint to the list of removed lists. + +In `src/librustc_lint/lib.rs` there is a list of "renamed and removed +lints". You can add this lint to the list: + +```rust +store.register_removed("overlapping_inherent_impls", "converted into hard error, see #36889"); +``` -- Eliminate the tracking issue. -- Change the stabilization schedule. +where `#36889` is the tracking issue for your lint. + +#### Update the places that issue the lint + +Finally, the last class of references you will see are the places that +actually **trigger** the lint itself (i.e., what causes the warnings +to appear). These you do not want to delete. Instead, you want to +convert them into errors. In this case, the +[`add_lint` call][addlintsource] looks like this: + +```rust +self.tcx.sess.add_lint(lint::builtin::OVERLAPPING_INHERENT_IMPLS, + node_id, + self.tcx.span_of_impl(item1).unwrap(), + msg); +``` + +We want to convert this into an error. In some cases, there may be an +existing error for this scenario. In others, we will need to allocate +a fresh diagnostic +code. [Instructions for allocating a fresh diagnostic code can be found here.](rustc-diagnostic-code.html) +You may want to mention in the extended descripiton that the compiler +behavior changed on this point, and include a reference to the +tracking issue for the change. + +Let's say that we've adopted `E0592` as our code. Then we can change +the `add_lint()` call above to something like: + +```rust +struct_span_err!(self.tcx.sess, self.tcx.span_of_impl(item1).unwrap(), msg) + .emit(); +``` + +#### Update tests + +Finally, run the test suite. These should be some tests that used to +reference the `overlapping_inherent_impls` lint, those will need to be +updated. In general, if the test used to have +`#[deny(overlapping_inherent_impls)]`, that can just be removed. + +``` +./x.py test +``` -Two other obvious (and rather extreme) alternatives are not having a -policy and not making any sort of breaking change at all: +#### All done! -- Not having a policy at all (as is the case today) encourages - inconsistent treatment of issues. -- Not making any sorts of breaking changes would mean that Rust simply - has to stop evolving, or else would issue new major versions quite - frequently, causing undue disruption. +Open a PR. =) -# Unresolved questions -[unresolved]: #unresolved-questions +[addlintsource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc_typeck/coherence/inherent.rs#L300-L303 -N/A +[futuresource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc_lint/lib.rs#L202-L205 diff --git a/rustc-diagnostic-code.md b/rustc-diagnostic-code.md new file mode 100644 index 000000000..e5508274e --- /dev/null +++ b/rustc-diagnostic-code.md @@ -0,0 +1,82 @@ +--- +layout: default +title: rustc diagnostic codes +--- + +We generally try assign each error message a unique code like `E0123`. +These codes are defined in the compiler in the `diagnostics.rs` files +found in each crate, which basically consist of macros. The codes come +in two varieties: those that have an extended write-up, and those that +do not. Whenever possible, if you are making a new code, you should +write an extended write-up. + +### Allocating a fresh code + +If you want to create a new error, you first need to find the next available +code. This is a bit tricky since the codes are defined in various crates. +To do it, run this obscure command: + +``` +./x.py test --stage 0 src/tools/tidy +``` + +This will invoke the tidy script, which generally checks that your +code obeys our coding conventions. One of those jobs is to check that +diagnostic codes are indeed unique. Once it is finished with that, +tidy will print out the lowest unused code: + +``` +... +tidy check (x86_64-apple-darwin) +* 470 error codes +* highest error code: E0591 +... +``` + +Here we see the highest error code in use is `E0591`, so we *probably* +want `E0592`. To be sure, run `rg E0592` and check, you should see no +references. + +Next, open `src/{crate}/diagnostics.rs` within the crate where you wish +to issue the error (e.g., `src/librustc_typeck/diagnostics.rs`). Ideally, +you will add the code (in its proper numerical order) into the `register_long_diagnostics!` macro, +sort of like this: + +```rust +register_long_diagnostics! { + ... + E0592: r##" +Your extended error text goes here! +"##, +} +``` + +But you can also add it without an extended description: + +```rust +register_diagnostics! { + ... + E0592, // put a description here +} +``` + +To actually issue the error, you can use the `struct_span_err!` macro: + +```rust +struct_span_err!(self.tcx.sess, // some path to the session here + span, // whatever span in the source you want + E0592, // your new error code + &format!("text of the error")) + .emit() // actually issue the error +``` + +If you want to add notes or other snippets, you can invoke methods +before you call `.emit()`: + +```rust +struct_span_err!(...) + .span_label(another_span, "something to label in the source") + .span_note(another_span, "some separate note, probably avoid these") + .emit_() +``` +