Skip to content

Commit cd85fbd

Browse files
committed
move the last few things from the forge
1 parent 7c56708 commit cd85fbd

File tree

7 files changed

+314183
-0
lines changed

7 files changed

+314183
-0
lines changed

src/SUMMARY.md

+3
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
- [Adding new tests](./tests/adding.md)
1818
- [Using `compiletest` + commands to control test execution](./compiletest.md)
1919
- [Walkthrough: a typical contribution](./walkthrough.md)
20+
- [Bug Fix Procedure](./bug-fix-procedure.md)
2021
- [Implementing new features](./implementing_new_features.md)
2122
- [Stability attributes](./stability.md)
2223
- [Stabilizing Features](./stabilization_guide.md)
@@ -27,6 +28,7 @@
2728
- [crates.io Dependencies](./crates-io.md)
2829
- [Emitting Errors and other Diagnostics](diagnostics.md)
2930
- [`LintStore`](./diagnostics/lintstore.md)
31+
- [Diagnostic Codes](./diagnostics/diagnostic-codes.md)
3032
- [ICE-breaker teams](ice-breaker/about.md)
3133
- [LLVM ICE-breakers](ice-breaker/llvm.md)
3234
- [Part 2: How rustc works](./part-2-intro.md)
@@ -38,6 +40,7 @@
3840
- [Incremental compilation](./queries/incremental-compilation.md)
3941
- [Incremental compilation In Detail](./queries/incremental-compilation-in-detail.md)
4042
- [Debugging and Testing](./incrcomp-debugging.md)
43+
- [Profiling Queries](./queries/profiling.md)
4144
- [Salsa](./salsa.md)
4245
- [Lexing and Parsing](./the-parser.md)
4346
- [`#[test]` Implementation](./test-implementation.md)

src/bug-fix-procedure.md

+330
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,330 @@
1+
# Rustc Bug Fix Procedure
2+
This page defines the best practices procedure for making bug fixes or soundness
3+
corrections in the compiler that can cause existing code to stop compiling. This
4+
text is based on
5+
[RFC 1589](https://github.com/rust-lang/rfcs/blob/master/text/1589-rustc-bug-fix-procedure.md).
6+
7+
# Motivation
8+
9+
[motivation]: #motivation
10+
11+
From time to time, we encounter the need to make a bug fix, soundness
12+
correction, or other change in the compiler which will cause existing code to
13+
stop compiling. When this happens, it is important that we handle the change in
14+
a way that gives users of Rust a smooth transition. What we want to avoid is
15+
that existing programs suddenly stop compiling with opaque error messages: we
16+
would prefer to have a gradual period of warnings, with clear guidance as to
17+
what the problem is, how to fix it, and why the change was made. This RFC
18+
describes the procedure that we have been developing for handling breaking
19+
changes that aims to achieve that kind of smooth transition.
20+
21+
One of the key points of this policy is that (a) warnings should be issued
22+
initially rather than hard errors if at all possible and (b) every change that
23+
causes existing code to stop compiling will have an associated tracking issue.
24+
This issue provides a point to collect feedback on the results of that change.
25+
Sometimes changes have unexpectedly large consequences or there may be a way to
26+
avoid the change that was not considered. In those cases, we may decide to
27+
change course and roll back the change, or find another solution (if warnings
28+
are being used, this is particularly easy to do).
29+
30+
### What qualifies as a bug fix?
31+
32+
Note that this RFC does not try to define when a breaking change is permitted.
33+
That is already covered under [RFC 1122][]. This document assumes that the
34+
change being made is in accordance with those policies. Here is a summary of the
35+
conditions from RFC 1122:
36+
37+
- **Soundness changes:** Fixes to holes uncovered in the type system.
38+
- **Compiler bugs:** Places where the compiler is not implementing the specified
39+
semantics found in an RFC or lang-team decision.
40+
- **Underspecified language semantics:** Clarifications to grey areas where the
41+
compiler behaves inconsistently and no formal behavior had been previously
42+
decided.
43+
44+
Please see [the RFC][rfc 1122] for full details!
45+
46+
# Detailed design
47+
48+
[design]: #detailed-design
49+
50+
The procedure for making a breaking change is as follows (each of these steps is
51+
described in more detail below):
52+
53+
0. Do a **crater run** to assess the impact of the change.
54+
1. Make a **special tracking issue** dedicated to the change.
55+
1. Do not report an error right away. Instead, **issue forwards-compatibility
56+
lint warnings**.
57+
- Sometimes this is not straightforward. See the text below for suggestions
58+
on different techniques we have employed in the past.
59+
- For cases where warnings are infeasible:
60+
- Report errors, but make every effort to give a targeted error message
61+
that directs users to the tracking issue
62+
- Submit PRs to all known affected crates that fix the issue
63+
- or, at minimum, alert the owners of those crates to the problem and
64+
direct them to the tracking issue
65+
1. Once the change has been in the wild for at least one cycle, we can
66+
**stabilize the change**, converting those warnings into errors.
67+
68+
Finally, for changes to libsyntax that will affect plugins, the general policy
69+
is to batch these changes. That is discussed below in more detail.
70+
71+
### Tracking issue
72+
73+
Every breaking change should be accompanied by a **dedicated tracking issue**
74+
for that change. The main text of this issue should describe the change being
75+
made, with a focus on what users must do to fix their code. The issue should be
76+
approachable and practical; it may make sense to direct users to an RFC or some
77+
other issue for the full details. The issue also serves as a place where users
78+
can comment with questions or other concerns.
79+
80+
A template for these breaking-change tracking issues can be found below. An
81+
example of how such an issue should look can be [found
82+
here][breaking-change-issue].
83+
84+
The issue should be tagged with (at least) `B-unstable` and `T-compiler`.
85+
86+
### Tracking issue template
87+
88+
This is a template to use for tracking issues:
89+
90+
```
91+
This is the **summary issue** for the `YOUR_LINT_NAME_HERE`
92+
future-compatibility warning and other related errors. The goal of
93+
this page is describe why this change was made and how you can fix
94+
code that is affected by it. It also provides a place to ask questions
95+
or register a complaint if you feel the change should not be made. For
96+
more information on the policy around future-compatibility warnings,
97+
see our [breaking change policy guidelines][guidelines].
98+
99+
[guidelines]: LINK_TO_THIS_RFC
100+
101+
#### What is the warning for?
102+
103+
*Describe the conditions that trigger the warning and how they can be
104+
fixed. Also explain why the change was made.**
105+
106+
#### When will this warning become a hard error?
107+
108+
At the beginning of each 6-week release cycle, the Rust compiler team
109+
will review the set of outstanding future compatibility warnings and
110+
nominate some of them for **Final Comment Period**. Toward the end of
111+
the cycle, we will review any comments and make a final determination
112+
whether to convert the warning into a hard error or remove it
113+
entirely.
114+
```
115+
116+
### Issuing future compatibility warnings
117+
118+
The best way to handle a breaking change is to begin by issuing
119+
future-compatibility warnings. These are a special category of lint warning.
120+
Adding a new future-compatibility warning can be done as follows.
121+
122+
```rust
123+
// 1. Define the lint in `src/librustc/lint/builtin.rs`:
124+
declare_lint! {
125+
pub YOUR_ERROR_HERE,
126+
Warn,
127+
"illegal use of foo bar baz"
128+
}
129+
130+
// 2. Add to the list of HardwiredLints in the same file:
131+
impl LintPass for HardwiredLints {
132+
fn get_lints(&self) -> LintArray {
133+
lint_array!(
134+
..,
135+
YOUR_ERROR_HERE
136+
)
137+
}
138+
}
139+
140+
// 3. Register the lint in `src/librustc_lint/lib.rs`:
141+
store.register_future_incompatible(sess, vec![
142+
...,
143+
FutureIncompatibleInfo {
144+
id: LintId::of(YOUR_ERROR_HERE),
145+
reference: "issue #1234", // your tracking issue here!
146+
},
147+
]);
148+
149+
// 4. Report the lint:
150+
tcx.lint_node(
151+
lint::builtin::YOUR_ERROR_HERE,
152+
path_id,
153+
binding.span,
154+
format!("some helper message here"));
155+
```
156+
157+
#### Helpful techniques
158+
159+
It can often be challenging to filter out new warnings from older, pre-existing
160+
errors. One technique that has been used in the past is to run the older code
161+
unchanged and collect the errors it would have reported. You can then issue
162+
warnings for any errors you would give which do not appear in that original set.
163+
Another option is to abort compilation after the original code completes if
164+
errors are reported: then you know that your new code will only execute when
165+
there were no errors before.
166+
167+
#### Crater and crates.io
168+
169+
We should always do a crater run to assess impact. It is polite and considerate
170+
to at least notify the authors of affected crates the breaking change. If we can
171+
submit PRs to fix the problem, so much the better.
172+
173+
#### Is it ever acceptable to go directly to issuing errors?
174+
175+
Changes that are believed to have negligible impact can go directly to issuing
176+
an error. One rule of thumb would be to check against `crates.io`: if fewer than
177+
10 **total** affected projects are found (**not** root errors), we can move
178+
straight to an error. In such cases, we should still make the "breaking change"
179+
page as before, and we should ensure that the error directs users to this page.
180+
In other words, everything should be the same except that users are getting an
181+
error, and not a warning. Moreover, we should submit PRs to the affected
182+
projects (ideally before the PR implementing the change lands in rustc).
183+
184+
If the impact is not believed to be negligible (e.g., more than 10 crates are
185+
affected), then warnings are required (unless the compiler team agrees to grant
186+
a special exemption in some particular case). If implementing warnings is not
187+
feasible, then we should make an aggressive strategy of migrating crates before
188+
we land the change so as to lower the number of affected crates. Here are some
189+
techniques for approaching this scenario:
190+
191+
1. Issue warnings for subparts of the problem, and reserve the new errors for
192+
the smallest set of cases you can.
193+
2. Try to give a very precise error message that suggests how to fix the problem
194+
and directs users to the tracking issue.
195+
3. It may also make sense to layer the fix:
196+
- First, add warnings where possible and let those land before proceeding to
197+
issue errors.
198+
- Work with authors of affected crates to ensure that corrected versions are
199+
available _before_ the fix lands, so that downstream users can use them.
200+
201+
### Stabilization
202+
203+
After a change is made, we will **stabilize** the change using the same process
204+
that we use for unstable features:
205+
206+
- After a new release is made, we will go through the outstanding tracking
207+
issues corresponding to breaking changes and nominate some of them for **final
208+
comment period** (FCP).
209+
- The FCP for such issues lasts for one cycle. In the final week or two of the
210+
cycle, we will review comments and make a final determination:
211+
212+
- Convert to error: the change should be made into a hard error.
213+
- Revert: we should remove the warning and continue to allow the older code to
214+
compile.
215+
- Defer: can't decide yet, wait longer, or try other strategies.
216+
217+
Ideally, breaking changes should have landed on the **stable branch** of the
218+
compiler before they are finalized.
219+
220+
<a name="guide">
221+
222+
### Removing a lint
223+
224+
Once we have decided to make a "future warning" into a hard error, we need a PR
225+
that removes the custom lint. As an example, here are the steps required to
226+
remove the `overlapping_inherent_impls` compatibility lint. First, convert the
227+
name of the lint to uppercase (`OVERLAPPING_INHERENT_IMPLS`) ripgrep through the
228+
source for that string. We will basically by converting each place where this
229+
lint name is mentioned (in the compiler, we use the upper-case name, and a macro
230+
automatically generates the lower-case string; so searching for
231+
`overlapping_inherent_impls` would not find much).
232+
233+
#### Remove the lint.
234+
235+
The first reference you will likely find is the lint definition [in
236+
`librustc/lint/builtin.rs` that resembles this][defsource]:
237+
238+
[defsource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc/lint/builtin.rs#L171-L175
239+
240+
```rust
241+
declare_lint! {
242+
pub OVERLAPPING_INHERENT_IMPLS,
243+
Deny, // this may also say Warning
244+
"two overlapping inherent impls define an item with the same name were erroneously allowed"
245+
}
246+
```
247+
248+
This `declare_lint!` macro creates the relevant data structures. Remove it. You
249+
will also find that there is a mention of `OVERLAPPING_INHERENT_IMPLS` later in
250+
the file as [part of a `lint_array!`][lintarraysource]; remove it too,
251+
252+
[lintarraysource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc/lint/builtin.rs#L252-L290
253+
254+
Next, you see see [a reference to `OVERLAPPING_INHERENT_IMPLS` in
255+
`librustc_lint/lib.rs`][futuresource]. This defining the lint as a "future
256+
compatibility lint":
257+
258+
```rust
259+
FutureIncompatibleInfo {
260+
id: LintId::of(OVERLAPPING_INHERENT_IMPLS),
261+
reference: "issue #36889 <https://github.com/rust-lang/rust/issues/36889>",
262+
},
263+
```
264+
265+
Remove this too.
266+
267+
#### Add the lint to the list of removed lists.
268+
269+
In `src/librustc_lint/lib.rs` there is a list of "renamed and removed lints".
270+
You can add this lint to the list:
271+
272+
```rust
273+
store.register_removed("overlapping_inherent_impls", "converted into hard error, see #36889");
274+
```
275+
276+
where `#36889` is the tracking issue for your lint.
277+
278+
#### Update the places that issue the lint
279+
280+
Finally, the last class of references you will see are the places that actually
281+
**trigger** the lint itself (i.e., what causes the warnings to appear). These
282+
you do not want to delete. Instead, you want to convert them into errors. In
283+
this case, the [`add_lint` call][addlintsource] looks like this:
284+
285+
```rust
286+
self.tcx.sess.add_lint(lint::builtin::OVERLAPPING_INHERENT_IMPLS,
287+
node_id,
288+
self.tcx.span_of_impl(item1).unwrap(),
289+
msg);
290+
```
291+
292+
We want to convert this into an error. In some cases, there may be an existing
293+
error for this scenario. In others, we will need to allocate a fresh diagnostic
294+
code.
295+
[Instructions for allocating a fresh diagnostic code can be found here.](rustc-diagnostic-code.html)
296+
You may want to mention in the extended description that the compiler behavior
297+
changed on this point, and include a reference to the tracking issue for the
298+
change.
299+
300+
Let's say that we've adopted `E0592` as our code. Then we can change the
301+
`add_lint()` call above to something like:
302+
303+
```rust
304+
struct_span_err!(self.tcx.sess, self.tcx.span_of_impl(item1).unwrap(), msg)
305+
.emit();
306+
```
307+
308+
#### Update tests
309+
310+
Finally, run the test suite. These should be some tests that used to reference
311+
the `overlapping_inherent_impls` lint, those will need to be updated. In
312+
general, if the test used to have `#[deny(overlapping_inherent_impls)]`, that
313+
can just be removed.
314+
315+
```
316+
./x.py test
317+
```
318+
319+
#### All done!
320+
321+
Open a PR. =)
322+
323+
[addlintsource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc_typeck/coherence/inherent.rs#L300-L303
324+
[futuresource]: https://github.com/rust-lang/rust/blob/085d71c3efe453863739c1fb68fd9bd1beff214f/src/librustc_lint/lib.rs#L202-L205
325+
326+
<!-- -Links--------------------------------------------------------------------- -->
327+
328+
[rfc 1122]: https://github.com/rust-lang/rfcs/blob/master/text/1122-language-semver.md
329+
[breaking-change-issue]: https://gist.github.com/nikomatsakis/631ec8b4af9a18b5d062d9d9b7d3d967
330+

0 commit comments

Comments
 (0)