Skip to content

Instantly share code, notes, and snippets.

@nikomatsakis
Last active April 22, 2016 10:32
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save nikomatsakis/1268ef0457a376d8fc24bf55aa7fc15b to your computer and use it in GitHub Desktop.
Save nikomatsakis/1268ef0457a376d8fc24bf55aa7fc15b to your computer and use it in GitHub Desktop.
compiler breaking change policy

Breaking change policy for rustc

This document describes the procedure for making a breaking change in rustc.

When can we make a breaking change?

RFC 1122 defined the conditions under which it is legal to make a breaking change. 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 for full details!

What is the procedure for making a breaking change?

The procedure is as follows (each of these steps is described in more detail below):

  1. Do a crater run to assess the impact of the change.
  2. Make a special tracking issue dedicated to the change.
  3. 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
  4. Once the change has been in the wild for at least one cycle, we can stabilize the change, converting those warnings into errors.

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.

The issue should be tagged with (at least) B-unstable and T-compiler.

Tracking issue template

What follows is a template for tracking issues. Please edit and replace the XXX placeholders with your own content.


This is the summary issue for the XXX 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.

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.

// 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.

What if issuing a warning is too hard?

It does happen from time to time that it is nigh impossible to isolate the breaking change so that you can issue warnings. In such cases, the best strategy is to mitigate:

  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.

If you will be issuing a new hard warning, then it is mandatory to at least notify authors of affected crates which we know about. Submitting PRs to fix the problem is strongly recommended. If the impact is too large to make that practical, this is warning sign that we should consider another strategy or try to avoid making the change.

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.
@llogiq
Copy link

llogiq commented Apr 21, 2016

I'd like to see a solution to dependencies with #![deny(warnings)] breaking because of future incompatibilities. Either we somehow handle those differently (e.g. cap at warn) or we at least add another future lint group that responsible library authors can set to warn explicitly.

Apart from that, I very much like the current practice of informing stakeholders of internal changes, and would suggest we add some description on record, but I'm unsure if this is the right place for that.

@nikomatsakis
Copy link
Author

Notes from compiler team meeting:

  • Add discussion of lints and how they are treated somewhat differently, owing to -A cap-lints.
  • Discuss breaking changes in libsyntax and how we try to roll them up. Rather than landing immediately, mark it as "breaking batch" (cc manish).

@nikomatsakis
Copy link
Author

@llogiq we already do cap lints on your dependencies at warn, so that changes to lints (including future-compat lints) never affect your dependencies directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment