diff --git a/text/0000-rustc-bug-fix-procedure.md b/text/0000-rustc-bug-fix-procedure.md new file mode 100644 index 00000000000..d709f58d740 --- /dev/null +++ b/text/0000-rustc-bug-fix-procedure.md @@ -0,0 +1,284 @@ +- Feature Name: N/A +- Start Date: 2016-04-22 +- RFC PR: (leave this empty) +- Rust Issue: (leave this empty) + +# Summary +[summary]: #summary + +Defines a best practices procedure for making bug fixes or soundness +corrections in the compiler that can cause existing code to stop +compiling. + +# Motivation +[motivation]: #motivation + +From time to time, we encounter the need to make a bug fix, soundness +correction, or other change in the compiler which will cause existing +code to stop compiling. When this happens, it is important that we +handle the change in a way that gives users of Rust a smooth +transition. What we want to avoid is that existing programs suddenly +stop compiling with opaque error messages: we would prefer to have a +gradual period of warnings, with clear guidance as to what the problem +is, how to fix it, and why the change was made. This RFC describes the +procedure that we have been developing for handling breaking changes +that aims to achieve that kind of smooth transition. + +One of the key points of this policy is that (a) warnings should be +issued initially rather than hard errors if at all possible and (b) +every change that causes existing code to stop compiling will have an +associated tracking issue. This issue provides a point to collect +feedback on the results of that change. Sometimes changes have +unexpectedly large consequences or there may be a way to avoid the +change that was not considered. In those cases, we may decide to +change course and roll back the change, or find another solution (if +warnings are being used, this is particularly easy to do). + +### What qualifies as a bug fix? + +Note that this RFC does not try to define when a breaking change is +permitted. That is already covered under [RFC 1122][]. This document +assumes that the change being made is in accordance with those +policies. Here is a summary of the conditions from RFC 1122: + +- **Soundness changes:** Fixes to holes uncovered in the type system. +- **Compiler bugs:** Places where the compiler is not implementing the + specified semantics found in an RFC or lang-team decision. +- **Underspecified language semantics:** Clarifications to grey areas + where the compiler behaves inconsistently and no formal behavior had + been previously decided. + +Please see [the RFC][RFC 1122] for full details! + +# Detailed design +[design]: #detailed-design + +The procedure for making a breaking change is as follows (each of +these steps is described in more detail below): + +0. Do a **crater run** to assess the impact of the change. +1. Make a **special tracking issue** dedicated to the change. +2. Do not report an error right away. Instead, **issue + forwards-compatibility lint warnings**. + - Sometimes this is not straightforward. See the text below for + suggestions on different techniques we have employed in the past. + - For cases where warnings are infeasible: + - Report errors, but make every effort to give a targeted error + message that directs users to the tracking issue + - Submit PRs to all known affected crates that fix the issue + - or, at minimum, alert the owners of those crates to the problem + and direct them to the tracking issue +3. Once the change has been in the wild for at least one cycle, we can + **stabilize the change**, converting those warnings into errors. + +Finally, for changes to libsyntax that will affect plugins, the +general policy is to batch these changes. That is discussed below in +more detail. + +### Tracking issue + +Every breaking change should be accompanied by a **dedicated tracking +issue** for that change. The main text of this issue should describe +the change being made, with a focus on what users must do to fix their +code. The issue should be approachable and practical; it may make +sense to direct users to an RFC or some other issue for the full +details. The issue also serves as a place where users can comment with +questions or other concerns. + +A template for these breaking-change tracking issues can be found +below. An example of how such an issue should look can be +[found here][breaking-change-issue]. + +The issue should be tagged with (at least) `B-unstable` and +`T-compiler`. + +### Tracking issue template + +What follows is a template for tracking issues. + +--------------------------------------------------------------------------- + +This is the **summary issue** for the `YOUR_LINT_NAME_HERE` +future-compatibility warning and other related errors. The goal of +this page is describe why this change was made and how you can fix +code that is affected by it. It also provides a place to ask questions +or register a complaint if you feel the change should not be made. For +more information on the policy around future-compatibility warnings, +see our [breaking change policy guidelines][guidelines]. + +[guidelines]: LINK_TO_THIS_RFC + +#### What is the warning for? + +*Describe the conditions that trigger the warning and how they can be +fixed. Also explain why the change was made.** + +#### When will this warning become a hard error? + +At the beginning of each 6-week release cycle, the Rust compiler team +will review the set of outstanding future compatibility warnings and +nominate some of them for **Final Comment Period**. Toward the end of +the cycle, we will review any comments and make a final determination +whether to convert the warning into a hard error or remove it +entirely. + +--------------------------------------------------------------------------- + +### Issuing future compatibility warnings + +The best way to handle a breaking change is to begin by issuing +future-compatibility warnings. These are a special category of lint +warning. Adding a new future-compatibility warning can be done as +follows. + +```rust +// 1. Define the lint in `src/librustc/lint/builtin.rs`: +declare_lint! { + pub YOUR_ERROR_HERE, + Warn, + "illegal use of foo bar baz" +} + +// 2. Add to the list of HardwiredLints in the same file: +impl LintPass for HardwiredLints { + fn get_lints(&self) -> LintArray { + lint_array!( + .., + YOUR_ERROR_HERE + ) + } +} + +// 3. Register the lint in `src/librustc_lint/lib.rs`: +store.register_future_incompatible(sess, vec![ + ..., + FutureIncompatibleInfo { + id: LintId::of(YOUR_ERROR_HERE), + reference: "issue #1234", // your tracking issue here! + }, +]); + +// 4. Report the lint: +tcx.sess.add_lint( + lint::builtin::YOUR_ERROR_HERE, + path_id, + binding.span, + format!("some helper message here")); +``` + +#### Helpful techniques + +It can often be challenging to filter out new warnings from older, +pre-existing errors. One technique that has been used in the past is +to run the older code unchanged and collect the errors it would have +reported. You can then issue warnings for any errors you would give +which do not appear in that original set. Another option is to abort +compilation after the original code completes if errors are reported: +then you know that your new code will only execute when there were no +errors before. + +#### Crater and crates.io + +We should always do a crater run to assess impact. It is polite and +considerate to at least notify the authors of affected crates the +breaking change. If we can submit PRs to fix the problem, so much the +better. + +#### Is it ever acceptable to go directly to issuing errors? + +Changes that are believed to have negligible impact can go directly to +issuing an error. One rule of thumb would be to check against +`crates.io`: if fewer than 10 **total** affected projects are found +(**not** root errors), we can move straight to an error. In such +cases, we should still make the "breaking change" page as before, and +we should ensure that the error directs users to this page. In other +words, everything should be the same except that users are getting an +error, and not a warning. Moreover, we should submit PRs to the +affected projects (ideally before the PR implementing the change lands +in rustc). + +If the impact is not believed to be negligible (e.g., more than 10 +crates are affected), then warnings are required (unless the compiler +team agrees to grant a special exemption in some particular case). If +implementing warnings is not feasible, then we should make an +aggressive strategy of migrating crates before we land the change so +as to lower the number of affected crates. Here are some techniques +for approaching this scenario: + +1. Issue warnings for subparts of the problem, and reserve the new + errors for the smallest set of cases you can. +2. Try to give a very precise error message that suggests how to fix + the problem and directs users to the tracking issue. +3. It may also make sense to layer the fix: + - First, add warnings where possible and let those land before proceeding + to issue errors. + - Work with authors of affected crates to ensure that corrected + versions are available *before* the fix lands, so that downstream + users can use them. + + +### Stabilization + +After a change is made, we will **stabilize** the change using the same +process that we use for unstable features: + +- After a new release is made, we will go through the outstanding tracking + issues corresponding to breaking changes and nominate some of them for + **final comment period** (FCP). +- The FCP for such issues lasts for one cycle. In the final week or two of the cycle, + we will review comments and make a final determination: + - Convert to error: the change should be made into a hard error. + - Revert: we should remove the warning and continue to allow the older code to compile. + - Defer: can't decide yet, wait longer, or try other strategies. + +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: + + 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. + +# Drawbacks +[drawbacks]: #drawbacks + +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. + +# Alternatives +[alternatives]: #alternatives + +There are obviously many points that we could tweak in this policy: + +- Eliminate the tracking issue. +- Change the stabilization schedule. + +Two other obvious (and rather extreme) alternatives are not having a +policy and not making any sort of breaking change at all: + +- 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. + +# Unresolved questions +[unresolved]: #unresolved-questions + +N/A + + + +[RFC 1122]: https://github.com/rust-lang/rfcs/blob/master/text/1122-language-semver.md +[breaking-change-issue]: https://gist.github.com/nikomatsakis/631ec8b4af9a18b5d062d9d9b7d3d967