src/doc/rustc-dev-guide/src/walkthrough.md

   1 # Walkthrough: a typical contribution
   2
   3 There 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
   8 contribution, and I will try to point those out as they arise.
   9
  10 In general, if you are interested in making a contribution and aren't sure
  11 where to start, please feel free to ask!
  12
  13 ## Overview
  14
  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:
  17
  18 ```rust,ignore
  19 macro_rules! foo {
  20     ($arg:ident $(, $optional_arg:ident)?) => {
  21         println!("{}", $arg);
  22
  23         $(
  24             println!("{}", $optional_arg);
  25         )?
  26     }
  27 }
  28
  29 fn main() {
  30     let x = 0;
  31     foo!(x); // ok! prints "0"
  32     foo!(x, x); // ok! prints "0 0"
  33 }
  34 ```
  35
  36 So basically, the `$(pat)?` matcher in the macro means "this pattern can occur
  37 0 or 1 times", similar to other regex syntaxes.
  38
  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.
  42
  43 - **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 the wider
  47   audience. You can find the original discussion [here][prerfc].
  48 - **RFC**  This is when you formally present your idea to the community for
  49   consideration. You can find the RFC [here][rfc].
  50 - **Implementation** Implement your idea unstably in the compiler. You can
  51   find the original implementation [here][impl1].
  52 - **Possibly iterate/refine** As the community gets experience with your
  53   feature on the nightly compiler and in `libstd`, 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].
  56 - **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.
  58 - **Relax** Your feature is now a stable rust feature!
  59
  60 [prerfc]: https://internals.rust-lang.org/t/pre-rfc-at-most-one-repetition-macro-patterns/6557
  61 [rfc]: https://github.com/rust-lang/rfcs/pull/2298
  62 [impl1]: https://github.com/rust-lang/rust/pull/47752
  63 [impl2]: https://github.com/rust-lang/rust/pull/49719
  64 [impl3]: https://github.com/rust-lang/rust/pull/51336
  65 [impl4]: https://github.com/rust-lang/rust/pull/51587
  66 [merge]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433177613
  67
  68 ## Pre-RFC and RFC
  69
  70 > NOTE: In general, if you are not proposing a _new_ feature or substantial
  71 > change to rust or the ecosystem, you don't need to follow the RFC process.
  72 > Instead, you can just jump to [implementation](#impl).
  73 >
  74 > You can find the official guidelines for when to open an RFC [here][rfcwhen].
  75
  76 [rfcwhen]: https://github.com/rust-lang/rfcs#when-you-need-to-follow-this-process
  77
  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.
  81
  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
  85 [README](https://github.com/rust-lang/rfcs#what-the-process-is).
  86
  87 Before 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
  90 interactions with other features.
  91
  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.
  98
  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)
 103
 104 In 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
 107 ideas, 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
 110 adjustments.
 111
 112 [nonascii]: https://github.com/rust-lang/rfcs/pull/2457
 113
 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
 119 itself to reflect the course of the discussion (e.g. new alternatives or prior
 120 work may be added or you may decide to change parts of the proposal itself).
 121
 122 In the end, when the discussion seems to reach a consensus and die down a bit,
 123 a 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:
 130
 131 - _Merge_: accept the feature. Here is the proposal to merge for our [`?` macro
 132   feature][rfcmerge].
 133 - _Close_: this feature in its current form is not a good fit for rust. Don't
 134   be discouraged if this happens to your RFC, and don't take it personally.
 135   This is not a reflection on you, but rather a community decision that rust
 136   will go a different direction.
 137 - _Postpone_: there is interest in going this direction but not at the moment.
 138   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.
 141   Postponed ideas may be revisited later.
 142
 143 [rfcmerge]: https://github.com/rust-lang/rfcs/pull/2298#issuecomment-360582667
 144
 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
 147 and discuss unresolved questions, implementation progress and blockers, etc.
 148 Here is the tracking issue on for our [`?` macro feature][tracking].
 149
 150 [tracking]: https://github.com/rust-lang/rust/issues/48075
 151
 152 <a name="impl"></a>
 153
 154 ## Implementation
 155
 156 To make a change to the compiler, open a PR against the [rust-lang/rust] repo.
 157
 158 [rust-lang/rust]: https://github.com/rust-lang/rust
 159
 160 Depending 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.
 165
 166 For the `?` macro feature, I needed to go understand the relevant parts of
 167 macro expansion in the compiler. Personally, I find that [improving the
 168 comments][comments] in the code is a helpful way of making sure I understand
 169 it, but you don't have to do that if you don't want to.
 170
 171 [comments]: https://github.com/rust-lang/rust/pull/47732
 172
 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.
 177
 178 **Most bug fixes and improvements** don't require a feature gate. You can just
 179 make your changes/improvements.
 180
 181 When you open a PR on the [rust-lang/rust], a bot will assign your PR to a
 182 review. If there is a particular rust team member you are working with, you can
 183 request that reviewer by leaving a comment on the thread with `r?
 184 @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.
 186
 187 The reviewer may request changes before they approve your PR. Feel free to ask
 188 questions or discuss things you don't understand or disagree with. However,
 189 recognize that the PR won't be merged unless someone on the rust team approves
 190 it.
 191
 192 When your review approves the PR, it will go into a queue for yet another bot
 193 called `@bors`.  `@bors` manages the CI build/merge queue. When your PR reaches
 194 the head of the `@bors` queue, `@bors` will test out the merge by running all
 195 tests against your PR on Travis CI. This takes a lot of time to
 196 finish. If all tests pass, the PR is merged and becomes part of the next
 197 nightly compiler!
 198
 199 There are a couple of things that may happen for some PRs during the review process
 200
 201 - If the change is substantial enough, the reviewer may request an FCP on
 202   the PR. This gives all members of the appropriate team a chance to review the
 203   changes.
 204 - If the change may cause breakage, the reviewer may request a [crater] run.
 205   This compiles the compiler with your changes and then attempts to compile all
 206   crates on crates.io with your modified compiler. This is a great smoke test
 207   to check if you introduced a change to compiler behavior that affects a large
 208   portion of the ecosystem.
 209 - If the diff of your PR is large or the reviewer is busy, your PR may have
 210   some merge conflicts with other PRs that happen to get merged first. You
 211   should fix these merge conflicts using the normal git procedures.
 212
 213 [crater]: ./tests/intro.html#crater
 214
 215 If you are not doing a new feature or something like that (e.g. if you are
 216 fixing a bug), then that's it! Thanks for your contribution :)
 217
 218 ## Refining your implementation
 219
 220 As people get experience with your new feature on nightly, slight changes may
 221 be proposed and unresolved questions may become resolved. Updates/changes go
 222 through the same process for implementing any other changes, as described
 223 above (i.e. submit a PR, go through review, wait for `@bors`, etc).
 224
 225 Some changes may be major enough to require an FCP and some review by rust team
 226 members.
 227
 228 For the `?` macro feature, we went through a few different iterations after the
 229 original implementation: [1][impl2], [2][impl3], [3][impl4].
 230
 231 Along the way, we decided that `?` should not take a separator, which was
 232 previously an unresolved question listed in the RFC. We also changed the
 233 disambiguation strategy: we decided to remove the ability to use `?` as a
 234 separator token for other repetition operators (e.g. `+` or `*`). However,
 235 since this was a breaking change, we decided to do it over an edition boundary.
 236 Thus, the new feature can be enabled only in edition 2018. These deviations
 237 from the original RFC required [another
 238 FCP](https://github.com/rust-lang/rust/issues/51934).
 239
 240 ## Stabilization
 241
 242 Finally, after the feature had baked for a while on nightly, a language team member
 243 [moved to stabilize it][stabilizefcp].
 244
 245 [stabilizefcp]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433177613
 246
 247 A _stabilization report_ needs to be written that includes
 248
 249 - brief description of the behavior and any deviations from the RFC
 250 - which edition(s) are affected and how
 251 - links to a few tests to show the interesting aspects
 252
 253 The stabilization report for our feature is [here][stabrep].
 254
 255 [stabrep]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433243048
 256
 257 After this, [a PR is made][stab] to remove the feature gate, enabling the feature by
 258 default (on the 2018 edition). A note is added to the [Release notes][relnotes]
 259 about the feature.
 260
 261 [stab]: https://github.com/rust-lang/rust/pull/56245
 262
 263 Steps to stabilize the feature can be found at [Stabilizing Features](./stabilization_guide.md).
 264
 265 [relnotes]: https://github.com/rust-lang/rust/blob/master/RELEASES.md