11# Walkthrough: a typical contribution
22
33There are _ a lot_ of ways to contribute to the Rust compiler, including fixing
4- bugs, improving performance, helping design features, providing feedback on
5- existing features, etc. This chapter does not claim to scratch the surface.
6- Instead, it walks through the design and implementation of a new feature. Not
7- all of the steps and processes described here are needed for every
4+ bugs, improving performance, helping design features, providing feedback on existing features, etc.
5+ This chapter does not claim to scratch the surface.
6+ Instead, it walks through the design and implementation of a new feature.
7+ Not all of the steps and processes described here are needed for every
88contribution, and I will try to point those out as they arise.
99
1010In general, if you are interested in making a contribution and aren't sure
1111where to start, please feel free to ask!
1212
1313## Overview
1414
15- The feature I will discuss in this chapter is the ` ? ` Kleene operator for
16- macros. Basically, we want to be able to write something like this:
15+ The feature I will discuss in this chapter is the ` ? ` Kleene operator for macros.
16+ Basically, we want to be able to write something like this:
1717
1818``` rust,ignore
1919macro_rules! foo {
@@ -36,25 +36,30 @@ fn main() {
3636So basically, the ` $(pat)? ` matcher in the macro means "this pattern can occur
37370 or 1 times", similar to other regex syntaxes.
3838
39- There were a number of steps to go from an idea to stable Rust feature. Here is
40- a quick list. We will go through each of these in order below. As I mentioned
41- before, not all of these are needed for every type of contribution.
39+ There were a number of steps to go from an idea to stable Rust feature.
40+ Here is a quick list.
41+ We will go through each of these in order below.
42+ As I mentioned before, not all of these are needed for every type of contribution.
4243
4344- ** Idea discussion/Pre-RFC** A Pre-RFC is an early draft or design discussion
44- of a feature. This stage is intended to flesh out the design space a bit and
45- get a grasp on the different merits and problems with an idea. It's a great
46- way to get early feedback on your idea before presenting it to the wider
47- audience. You can find the original discussion [ here] [ prerfc ] .
45+ of a feature.
46+ This stage is intended to flesh out the design space a bit and
47+ get a grasp on the different merits and problems with an idea.
48+ It's a great way to get early feedback on your idea before presenting it to the wider
49+ audience.
50+ You can find the original discussion [ here] [ prerfc ] .
4851- ** RFC** This is when you formally present your idea to the community for
49- consideration. You can find the RFC [ here] [ rfc ] .
52+ consideration.
53+ You can find the RFC [ here] [ rfc ] .
5054- ** Implementation** Implement your idea unstably in the compiler. You can
5155 find the original implementation [ here] [ impl1 ] .
5256- ** Possibly iterate/refine** As the community gets experience with your
5357 feature on the nightly compiler and in ` std ` , there may be additional
54- feedback about design choice that might be adjusted. This particular feature
55- went [ through] [ impl2 ] a [ number] [ impl3 ] of [ iterations] [ impl4 ] .
58+ feedback about design choice that might be adjusted.
59+ This particular feature went [ through] [ impl2 ] a [ number] [ impl3 ] of [ iterations] [ impl4 ] .
5660- ** Stabilization** When your feature has baked enough, a Rust team member may
57- [ propose to stabilize it] [ merge ] . If there is consensus, this is done.
61+ [ propose to stabilize it] [ merge ] .
62+ If there is consensus, this is done.
5863- ** Relax** Your feature is now a stable Rust feature!
5964
6065[ prerfc ] : https://internals.rust-lang.org/t/pre-rfc-at-most-one-repetition-macro-patterns/6557
@@ -75,58 +80,63 @@ before, not all of these are needed for every type of contribution.
7580
7681[ rfcwhen ] : https://github.com/rust-lang/rfcs#when-you-need-to-follow-this-process
7782
78- An RFC is a document that describes the feature or change you are proposing in
79- detail. Anyone can write an RFC; the process is the same for everyone,
80- including Rust team members.
83+ An RFC is a document that describes the feature or change you are proposing in detail.
84+ Anyone can write an RFC;
85+ the process is the same for everyone, including Rust team members.
8186
82- To open an RFC, open a PR on the
83- [ rust-lang/rfcs] ( https://github.com/rust-lang/rfcs ) repo on GitHub. You can
84- find detailed instructions in the
87+ To open an RFC, open a PR on the [ rust-lang/rfcs] ( https://github.com/rust-lang/rfcs ) repo on GitHub.
88+ You can find detailed instructions in the
8589[ README] ( https://github.com/rust-lang/rfcs#what-the-process-is ) .
8690
8791Before opening an RFC, you should do the research to "flesh out" your idea.
88- Hastily-proposed RFCs tend not to be accepted. You should generally have a good
89- description of the motivation, impact, disadvantages, and potential
92+ Hastily-proposed RFCs tend not to be accepted.
93+ You should generally have a good description of the motivation, impact, disadvantages, and potential
9094interactions with other features.
9195
92- If that sounds like a lot of work, it's because it is. But no fear! Even if
93- you're not a compiler hacker, you can get great feedback by doing a _ pre-RFC_ .
94- This is an _ informal_ discussion of the idea. The best place to do this is
95- internals.rust-lang.org. Your post doesn't have to follow any particular
96- structure. It doesn't even need to be a cohesive idea. Generally, you will get
97- tons of feedback that you can integrate back to produce a good RFC.
96+ If that sounds like a lot of work, it's because it is.
97+ But no fear!
98+ Even if you're not a compiler hacker, you can get great feedback by doing a _ pre-RFC_ .
99+ This is an _ informal_ discussion of the idea.
100+ The best place to do this is internals.rust-lang.org.
101+ Your post doesn't have to follow any particular structure.
102+ It doesn't even need to be a cohesive idea.
103+ Generally, you will get tons of feedback that you can integrate back to produce a good RFC.
98104
99- (Another pro-tip: try searching the RFCs repo and internals for prior related
100- ideas. A lot of times an idea has already been considered and was either
101- rejected or postponed to be tried again later. This can save you and everybody
102- else some time)
105+ (Another pro-tip: try searching the RFCs repo and internals for prior related ideas.
106+ A lot of times an idea has already been considered and was either
107+ rejected or postponed to be tried again later.
108+ This can save you and everybody else some time)
103109
104110In the case of our example, a participant in the pre-RFC thread pointed out a
105- syntax ambiguity and a potential resolution. Also, the overall feedback seemed
106- positive. In this case, the discussion converged pretty quickly, but for some
111+ syntax ambiguity and a potential resolution.
112+ Also, the overall feedback seemed positive.
113+ In this case, the discussion converged pretty quickly, but for some
107114ideas, a lot more discussion can happen (e.g. see [ this RFC] [ nonascii ] which
108- received a whopping 684 comments!). If that happens, don't be discouraged; it
109- means the community is interested in your idea, but it perhaps needs some
115+ received a whopping 684 comments!).
116+ If that happens, don't be discouraged;
117+ it means the community is interested in your idea, but it perhaps needs some
110118adjustments.
111119
112120[ nonascii ] : https://github.com/rust-lang/rfcs/pull/2457
113121
114- The RFC for our ` ? ` macro feature did receive some discussion on the RFC thread
115- too. As with most RFCs, there were a few questions that we couldn't answer by
116- discussion: we needed experience using the feature to decide. Such questions
117- are listed in the "Unresolved Questions" section of the RFC. Also, over the
118- course of the RFC discussion, you will probably want to update the RFC document
122+ The RFC for our ` ? ` macro feature did receive some discussion on the RFC thread too.
123+ As with most RFCs, there were a few questions that we couldn't answer by
124+ discussion: we needed experience using the feature to decide.
125+ Such questions are listed in the "Unresolved Questions" section of the RFC.
126+ Also, over the course of the RFC discussion, you will probably want to update the RFC document
119127itself to reflect the course of the discussion (e.g. new alternatives or prior
120128work may be added or you may decide to change parts of the proposal itself).
121129
122130In the end, when the discussion seems to reach a consensus and die down a bit,
123131a Rust team member may propose to move to "final comment period" (FCP) with one
124- of three possible dispositions. This means that they want the other members of
125- the appropriate teams to review and comment on the RFC. More discussion may
126- ensue, which may result in more changes or unresolved questions being added. At
127- some point, when everyone is satisfied, the RFC enters the FCP, which is the
128- last chance for people to bring up objections. When the FCP is over, the
129- disposition is adopted. Here are the three possible dispositions:
132+ of three possible dispositions.
133+ This means that they want the other members of
134+ the appropriate teams to review and comment on the RFC.
135+ More discussion may ensue, which may result in more changes or unresolved questions being added.
136+ At some point, when everyone is satisfied, the RFC enters the FCP, which is the
137+ last chance for people to bring up objections.
138+ When the FCP is over, the disposition is adopted.
139+ Here are the three possible dispositions:
130140
131141- _ Merge_ : accept the feature. Here is the proposal to merge for our [ ` ? ` macro
132142 feature] [ rfcmerge ] .
@@ -136,14 +146,14 @@ disposition is adopted. Here are the three possible dispositions:
136146 will go a different direction.
137147- _ Postpone_ : there is interest in going this direction but not at the moment.
138148 This happens most often because the appropriate Rust team doesn't have the
139- bandwidth to shepherd the feature through the process to stabilization. Often
140- this is the case when the feature doesn't fit into the team's roadmap.
149+ bandwidth to shepherd the feature through the process to stabilization.
150+ Often this is the case when the feature doesn't fit into the team's roadmap.
141151 Postponed ideas may be revisited later.
142152
143153[ rfcmerge ] : https://github.com/rust-lang/rfcs/pull/2298#issuecomment-360582667
144154
145- When an RFC is merged, the PR is merged into the RFCs repo. A new _ tracking
146- issue_ is created in the [ rust-lang/rust] repo to track progress on the feature
155+ When an RFC is merged, the PR is merged into the RFCs repo.
156+ A new _ tracking issue_ is created in the [ rust-lang/rust] repo to track progress on the feature
147157and discuss unresolved questions, implementation progress and blockers, etc.
148158Here is the tracking issue on for our [ ` ? ` macro feature] [ tracking ] .
149159
@@ -158,93 +168,98 @@ To make a change to the compiler, open a PR against the [rust-lang/rust] repo.
158168[ rust-lang/rust ] : https://github.com/rust-lang/rust
159169
160170Depending on the feature/change/bug fix/improvement, implementation may be
161- relatively-straightforward or it may be a major undertaking. You can always ask
162- for help or mentorship from more experienced compiler devs. Also, you don't
163- have to be the one to implement your feature; but keep in mind that if you
164- don't, it might be a while before someone else does.
171+ relatively-straightforward or it may be a major undertaking.
172+ You can always ask for help or mentorship from more experienced compiler devs.
173+ Also, you don't have to be the one to implement your feature;
174+ but keep in mind that if you don't, it might be a while before someone else does.
165175
166176For the ` ? ` macro feature, I needed to go understand the relevant parts of
167- macro expansion in the compiler. Personally, I find that [ improving the
177+ macro expansion in the compiler.
178+ Personally, I find that [ improving the
168179comments] [ comments ] in the code is a helpful way of making sure I understand
169180it, but you don't have to do that if you don't want to.
170181
171182[ comments ] : https://github.com/rust-lang/rust/pull/47732
172183
173- I then [ implemented] [ impl1 ] the original feature, as described in the RFC. When
174- a new feature is implemented, it goes behind a _ feature gate_ , which means that
175- you have to use ` #![feature(my_feature_name)] ` to use the feature. The feature
176- gate is removed when the feature is stabilized.
184+ I then [ implemented] [ impl1 ] the original feature, as described in the RFC.
185+ When a new feature is implemented, it goes behind a _ feature gate_ , which means that
186+ you have to use ` #![feature(my_feature_name)] ` to use the feature.
187+ The feature gate is removed when the feature is stabilized.
177188
178189** Most bug fixes and improvements** don't require a feature gate. You can just
179190make your changes/improvements.
180191
181- When you open a PR on the [ rust-lang/rust] , a bot will assign your PR to a
182- reviewer. If there is a particular Rust team member you are working with, you can
192+ When you open a PR on the [ rust-lang/rust] , a bot will assign your PR to a reviewer.
193+ If there is a particular Rust team member you are working with, you can
183194request that reviewer by leaving a comment on the thread with `r?
184195@reviewer-github -id` (e.g. ` r? @eddyb`). If you don't know who to request,
185- don't request anyone; the bot will assign someone automatically based on which files you changed.
196+ don't request anyone;
197+ the bot will assign someone automatically based on which files you changed.
186198
187199The reviewer may request changes before they approve your PR, they may mark the PR with label
188200"S-waiting-on-author" after leaving comments, this means that the PR is blocked on you to make
189- some requested changes. When you finished iterating on the changes, you can mark the PR as
201+ some requested changes.
202+ When you finished iterating on the changes, you can mark the PR as
190203` S-waiting-on-review ` again by leaving a comment with ` @rustbot ready ` , this will remove the
191204` S-waiting-on-author ` label and add the ` S-waiting-on-review ` label.
192205
193- Feel free to ask questions or discuss things you don't understand or disagree with. However,
194- recognize that the PR won't be merged unless someone on the Rust team approves
195- it. If a reviewer leave a comment like ` r=me after fixing ... ` , that means they approve the PR and
206+ Feel free to ask questions or discuss things you don't understand or disagree with.
207+ However, recognize that the PR won't be merged unless someone on the Rust team approves
208+ it.
209+ If a reviewer leave a comment like ` r=me after fixing ... ` , that means they approve the PR and
196210you can merge it with comment with ` @bors r=reviewer-github-id ` (e.g. ` @bors r=eddyb ` ) to merge it
197- after fixing trivial issues. Note that ` r=someone ` requires permission and bors could say
198- something like "🔑 Insufficient privileges..." when commenting ` r=someone ` . In that case,
199- you have to ask the reviewer to revisit your PR.
200-
201- When your reviewer approves the PR, it will go into a queue for yet another bot
202- called ` @bors ` . ` @bors ` manages the CI build/merge queue. When your PR reaches
203- the head of the ` @bors ` queue, ` @bors ` will test out the merge by running all
204- tests against your PR on GitHub Actions. This takes a lot of time to
205- finish. If all tests pass, the PR is merged and becomes part of the next
206- nightly compiler!
211+ after fixing trivial issues.
212+ Note that ` r=someone ` requires permission and bors could say
213+ something like "🔑 Insufficient privileges..." when commenting ` r=someone ` .
214+ In that case, you have to ask the reviewer to revisit your PR.
215+
216+ When your reviewer approves the PR, it will go into a queue for yet another bot called ` @bors ` .
217+ ` @bors ` manages the CI build/merge queue.
218+ When your PR reaches the head of the ` @bors ` queue, ` @bors ` will test out the merge by running all
219+ tests against your PR on GitHub Actions.
220+ This takes a lot of time to finish.
221+ If all tests pass, the PR is merged and becomes part of the next nightly compiler!
207222
208223There are a couple of things that may happen for some PRs during the review process
209224
210225- If the change is substantial enough, the reviewer may request an FCP on
211- the PR. This gives all members of the appropriate team a chance to review the
212- changes.
226+ the PR.
227+ This gives all members of the appropriate team a chance to review the changes.
213228- If the change may cause breakage, the reviewer may request a [ crater] run.
214229 This compiles the compiler with your changes and then attempts to compile all
215- crates on crates.io with your modified compiler. This is a great smoke test
230+ crates on crates.io with your modified compiler.
231+ This is a great smoke test
216232 to check if you introduced a change to compiler behavior that affects a large
217233 portion of the ecosystem.
218234- If the diff of your PR is large or the reviewer is busy, your PR may have
219- some merge conflicts with other PRs that happen to get merged first. You
220- should fix these merge conflicts using the normal git procedures.
235+ some merge conflicts with other PRs that happen to get merged first.
236+ You should fix these merge conflicts using the normal git procedures.
221237
222238[ crater ] : ./tests/crater.html
223239
224240If you are not doing a new feature or something like that (e.g. if you are
225- fixing a bug), then that's it! Thanks for your contribution :)
241+ fixing a bug), then that's it!
242+ Thanks for your contribution :)
226243
227244## Refining your implementation
228245
229246As people get experience with your new feature on nightly, slight changes may
230- be proposed and unresolved questions may become resolved. Updates/changes go
231- through the same process for implementing any other changes, as described
247+ be proposed and unresolved questions may become resolved.
248+ Updates/changes go through the same process for implementing any other changes, as described
232249above (i.e. submit a PR, go through review, wait for ` @bors ` , etc).
233250
234- Some changes may be major enough to require an FCP and some review by Rust team
235- members.
251+ Some changes may be major enough to require an FCP and some review by Rust team members.
236252
237253For the ` ? ` macro feature, we went through a few different iterations after the
238254original implementation: [ 1] [ impl2 ] , [ 2] [ impl3 ] , [ 3] [ impl4 ] .
239255
240256Along the way, we decided that ` ? ` should not take a separator, which was
241- previously an unresolved question listed in the RFC. We also changed the
242- disambiguation strategy: we decided to remove the ability to use ` ? ` as a
257+ previously an unresolved question listed in the RFC.
258+ We also changed the disambiguation strategy: we decided to remove the ability to use ` ? ` as a
243259separator token for other repetition operators (e.g. ` + ` or ` * ` ). However,
244260since this was a breaking change, we decided to do it over an edition boundary.
245261Thus, the new feature can be enabled only in edition 2018. These deviations
246- from the original RFC required [ another
247- FCP] ( https://github.com/rust-lang/rust/issues/51934 ) .
262+ from the original RFC required [ another FCP] ( https://github.com/rust-lang/rust/issues/51934 ) .
248263
249264## Stabilization
250265
@@ -264,8 +279,8 @@ The stabilization report for our feature is [here][stabrep].
264279[ stabrep ] : https://github.com/rust-lang/rust/issues/48075#issuecomment-433243048
265280
266281After this, [ a PR is made] [ stab ] to remove the feature gate, enabling the feature by
267- default (on the 2018 edition). A note is added to the [ Release notes ] [ relnotes ]
268- about the feature.
282+ default (on the 2018 edition).
283+ A note is added to the [ Release notes ] [ relnotes ] about the feature.
269284
270285[ stab ] : https://github.com/rust-lang/rust/pull/56245
271286
0 commit comments