From 07eb29fb1e78942981bcae5ca2730d86299e40f2 Mon Sep 17 00:00:00 2001
From: Mark Mansi <markm@cs.wisc.edu>
Date: Sat, 10 Nov 2018 22:06:15 -0600
Subject: [PATCH 1/8] start on walkthrough
---
src/walkthrough.md | 147 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
diff --git a/src/walkthrough.md b/src/walkthrough.md
index ab158177a..680f2b831 100644
--- a/src/walkthrough.md
+++ b/src/walkthrough.md
@@ -1 +1,148 @@
# Walkthrough: a typical contribution
+
+There are _a lot_ of ways to contribute to the rust compiler, including fixing
+bugs, improving performance, helping design features, providing feedback on
+existing features, etc. This chapter does not claim to scratch the surface.
+Instead, it walks through the design and implementation of a new feature. Not
+all of the steps and processes described here are needed for every
+contribution, and I will try to point those out as they arise.
+
+In general, if you are interested in making a contribution and aren't sure
+where to start, please feel free to ask!
+
+## Overview
+
+The feature I will discuss in this chapter is the `?` Kleene operator for
+macros. Basically, we want to be able to write something like this:
+
+```rust
+macro_rules! foo {
+ ($arg:ident $(, $optional_arg:ident)?) => {
+ println!("{}", $arg);
+
+ $(
+ println!("{}", $optional_arg);
+ )?
+ }
+}
+
+fn main() {
+ let x = 0;
+ foo!(x); // ok! prints "0"
+ foo!(x, x); // ok! prints "0 0"
+}
+```
+
+So basically, the `$(pat)?` matcher in the macro means "this pattern can occur
+0 or 1 times", similar to other regex syntaxes.
+
+There were a number of steps to go from an idea to stable rust feature. Here is
+a quick list. We will go through each of these in order below. As I mentioned
+before, not all of these are needed for every type of contribution.
+
+- **Idea discussion/Pre-RFC** A Pre-RFC is an early draft or design discussion
+ of a feature. This stage is intended to flesh out the design space a bit and
+ get a grasp on the different merits and problems with an idea. It's a great
+ way to get early feedback on your idea before presenting it the wider
+ audience. You can find the original discussion [here][prerfc].
+- **RFC** This is when you formally present your idea to the community for
+ consideration. You can find the RFC [here][rfc].
+- **Implementation** Implement your idea unstabley in the compiler. You can
+ find the original implementation [here][impl1].
+- **Possibly iterate/refine** As the community gets experience with your
+ feature on the nightly compiler and in `libstd`, there may be additional
+ feedback about design choice that might be adjusted. This particular feature
+ went [through][impl2] a [number][impl3] of [iterations][impl4].
+- **Stabilization** When your feature has baked enough, a rust team member may
+ [propose to stabilize it][merge]. If there is consensus, this is done.
+- **Relax** Your feature is now a stable rust feature!
+
+[prerfc]: https://internals.rust-lang.org/t/pre-rfc-at-most-one-repetition-macro-patterns/6557
+[rfc]: https://github.com/rust-lang/rfcs/pull/2298
+[impl1]: https://github.com/rust-lang/rust/pull/47752
+[impl2]: https://github.com/rust-lang/rust/pull/49719
+[impl3]: https://github.com/rust-lang/rust/pull/51336
+[impl4]: https://github.com/rust-lang/rust/pull/51587
+[merge]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433177613
+
+## Pre-RFC and RFC
+
+> NOTE: In general, if you are not proposing a _new_ feature or substantial
+> change to rust or the ecosystem, you don't need to follow the RFC process.
+> Instead, you can just jump to [implementation](#impl).
+>
+> You can find the official guidelines for when to open an RFC [here][rfcwhen].
+
+[rfcwhen]: https://github.com/rust-lang/rfcs#when-you-need-to-follow-this-process
+
+An RFC is a document that describes the feature or change you are proposing in
+detail. Anyone can write an RFC; the process is the same for everyone,
+including rust team members.
+
+To open an RFC, open a PR on the
+[rust-lang/rfcs](https://github.com/rust-lang/rfcs) repo on GitHub. You can
+find detailed instructions in the
+[README](https://github.com/rust-lang/rfcs#what-the-process-is).
+
+Before opening an RFC, you should do the research to "flesh out" your idea.
+Hastily-proposed RFCs tend not to be accepted. You should generally have a good
+description of the motivation, impact, disadvantages, and potential
+interactions with other features.
+
+If that sounds like a lot of work, it's because it is. But no fear! Even if
+you're not a compiler hacker, you can get great feedback by doing a _pre-RFC_.
+This is an _informal_ discussion of the idea. The best place to do this is
+internals.rust-lang.org. Your post doesn't have to follow any particular
+structure. It doesn't even need to be a cohesive idea. Generally, you will get
+tons of feedback that you can integrate back to produce a good RFC.
+
+(Another pro-tip: try searching the RFCs repo and internals for prior related
+ideas. A lot of times an idea has already been considered and was either
+rejected or postponed to be tried again later. This can save you and everybody
+else some time)
+
+In the case of our example, a participant in the pre-RFC thread pointed out a
+syntax ambiguity and a potential resolution. Also, the overall feedback seemed
+positive. In this case, the discussion converged pretty quickly, but for some
+ideas, a lot more discussion can happen (e.g. see [this RFC][nonascii] which
+received a whopping 684 comments!). If that happens, don't be discouraged; it
+means the community is interested in your idea, but it perhaps needs some
+adjustments.
+
+[nonascii]: https://github.com/rust-lang/rfcs/pull/2457
+
+The RFC for our `?` macro feature did receive some discussion on the RFC thread
+too. As with most RFCs, there were a few questions that we couldn't answer by
+discussion: we needed experience using the feature to decide. Such questions
+are listed in the "Unresolved Questions" section of the RFC. Also, over the
+course of the RFC discussion, you will probably want to update the RFC document
+itself to reflect the course of the discussion (e.g. new alternatives or prior
+work may be added or you may decide to change parts of the proposal itself).
+
+In the end, when the discussion seems to reach a consensus and die down a bit,
+a rust team member may propose [to merge the RFC][rfcmerge]. This means that
+they want the other members of the appropriate teams to review and comment on
+the RFC. More changes may be proposed. At some point, when everyone is
+satisfied, the RFC enters the "final comment period" (FCP), which is the last
+chance for people to bring up objections. When the FCP is over, the RFC is
+"merged" (or accepted).
+
+[rfcmerge]: https://github.com/rust-lang/rfcs/pull/2298#issuecomment-360582667
+
+Some other possible outcomes might be for a team member to propose to
+
+- _Close_: this feature in its current form is not a good fit for rust. Don't
+ be discouraged if this happens to your RFC, and don't take it personally.
+ This is not a reflection on you, but rather a community decision that rust
+ will go a different direction.
+- _Postpone_: there is interest in going this direction but not at the moment.
+ This happens most often because the appropriate rust team doesn't have the
+ bandwidth to shepherd the feature through the process to stabilization. Often
+ this is the case when the feature doesn't fit into the team's roadmap.
+ Postponed ideas may be revisited later.
+
+<a name="impl"></a>
+
+## Implementation
+
+TODO
From a5b158f044cac668197588da5fb4694c2f8c38c3 Mon Sep 17 00:00:00 2001
From: Mark Mansi <markm@cs.wisc.edu>
Date: Sat, 10 Nov 2018 22:41:27 -0600
Subject: [PATCH 2/8] add a bit more
---
src/walkthrough.md | 67 +++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 66 insertions(+), 1 deletion(-)
diff --git a/src/walkthrough.md b/src/walkthrough.md
index 680f2b831..738828bbe 100644
--- a/src/walkthrough.md
+++ b/src/walkthrough.md
@@ -15,7 +15,7 @@ where to start, please feel free to ask!
The feature I will discuss in this chapter is the `?` Kleene operator for
macros. Basically, we want to be able to write something like this:
-```rust
+```rust,ignore
macro_rules! foo {
($arg:ident $(, $optional_arg:ident)?) => {
println!("{}", $arg);
@@ -145,4 +145,69 @@ Some other possible outcomes might be for a team member to propose to
## Implementation
+To make a change to the compiler, open a PR against the [rust-lang/rust] repo.
+
+[rust-lang/rust]: https://github.com/rust-lang/rust
+
+Depending on the feature/change/bug fix/improvement, implementation may be
+relatively-straightforward or it may be a major undertaking. You can always ask
+for help or mentorship from more experienced compiler devs. Also, you don't
+have to be the one to implement your feature; but keep in mind that if you
+don't it might be a while before someone else does.
+
+For the `?` macro feature, I needed to go understand the relevant parts of
+macro expansion in the compiler. Personally, I find that [improving the
+comments][comments] in the code is a helpful way of making sure I understand
+it, but you don't have to do that if you don't want to.
+
+[comments]: https://github.com/rust-lang/rust/pull/47732
+
+I then [implemented][impl1] the original feature, as described in the RFC. When
+a new feature is implemented, it goes behind a _feature gate_, which means that
+you have to use `#![feature(my_feature_name)]` to use the feature. The feature
+gate is removed when the feature is stabilized.
+
+**Most bug fixes and improvements** don't require a feature gate. You can just
+make your changes/improvements.
+
+When you open a PR on the [rust-lang/rust], a bot will assign your PR to a
+review. If there is a particular rust team member you are working with, you can
+request that reviewer by leaving a comment on the thread with `r?
+@reviewer-github-id` (e.g. `r? @eddyb`). If you don't know who to request,
+don't request anyone; the bot will assign someone automatically.
+
+The reviewer may request changes before they approve your PR. Feel free to ask
+questions or discuss things you don't understand or disagree with. However,
+recognize that the PR won't be merged unless someone on the rust team approves
+it.
+
+When your review approves the PR, it will go into a queue for yet another bot
+called `@bors`. `@bors` manages the CI build/merge queue. When your PR reaches
+the head of the `@bors` queue, `@bors` will test out the merge by running all
+tests against your PR on Travis CI. This takes about 2 hours as of this
+writing. If all tests pass, the PR is merged and becomes part of the next
+nightly compiler!
+
+There are a couple of things that may happen for some PRs during the review process
+
+- If the change is substantial enough, the reviewer may request an FCP on
+ the PR. This gives all members of the appropriate team a chance to review the
+ changes.
+- If the change may cause breakage, the reviewer may request a [crater] run.
+ This compiles the compiler with your changes and then attempts to compile all
+ crates on crates.io with your modified compiler. This is a great smoke test
+ to check if you introduced a change to compiler behavior that affects a large
+ portion of the ecosystem.
+- If the diff of your PR is large or the reviewer is busy, your PR may have
+ some merge conflicts with other PRs. You should fix these merge conflicts
+ using the normal git procedures.
+
+[crater]: ./tests/intro.html#crater
+
+## Refining your implementation
+
+TODO
+
+## Stabilization
+
TODO
From 0f3cb5ef9849f29166e8053052d16d112871c3a5 Mon Sep 17 00:00:00 2001
From: Mark Mansi <markm@cs.wisc.edu>
Date: Sun, 11 Nov 2018 16:32:11 -0600
Subject: [PATCH 3/8] add a bit more
---
src/walkthrough.md | 45 ++++++++++++++++++++++++++++++++++-----------
1 file changed, 34 insertions(+), 11 deletions(-)
diff --git a/src/walkthrough.md b/src/walkthrough.md
index 738828bbe..8cd511d92 100644
--- a/src/walkthrough.md
+++ b/src/walkthrough.md
@@ -120,17 +120,16 @@ itself to reflect the course of the discussion (e.g. new alternatives or prior
work may be added or you may decide to change parts of the proposal itself).
In the end, when the discussion seems to reach a consensus and die down a bit,
-a rust team member may propose [to merge the RFC][rfcmerge]. This means that
-they want the other members of the appropriate teams to review and comment on
-the RFC. More changes may be proposed. At some point, when everyone is
+a rust team member may propose to move to FCP with one of three possible dispositions.
+This means that they want the other members of the appropriate teams to review
+and comment on the RFC. More discussion may ensue, which may result in more changes
+or unresolved questions being added. At some point, when everyone is
satisfied, the RFC enters the "final comment period" (FCP), which is the last
-chance for people to bring up objections. When the FCP is over, the RFC is
-"merged" (or accepted).
-
-[rfcmerge]: https://github.com/rust-lang/rfcs/pull/2298#issuecomment-360582667
-
-Some other possible outcomes might be for a team member to propose to
+chance for people to bring up objections. When the FCP is over, the disposition is
+adopted. Here are the three possible dispositions:
+- _Merge_: accept the feature. Here is the proposal to merge for our [`?` macro
+ feature][rfcmerge].
- _Close_: this feature in its current form is not a good fit for rust. Don't
be discouraged if this happens to your RFC, and don't take it personally.
This is not a reflection on you, but rather a community decision that rust
@@ -141,6 +140,15 @@ Some other possible outcomes might be for a team member to propose to
this is the case when the feature doesn't fit into the team's roadmap.
Postponed ideas may be revisited later.
+[rfcmerge]: https://github.com/rust-lang/rfcs/pull/2298#issuecomment-360582667
+
+When an RFC is merged, the PR is merged into the RFCs repo. A new _tracking
+issue_ is created in the [rust-lang/rust] repo to track progress on the feature
+and discuss unresolved questions, implementation progress and blockers, etc.
+Here is the tracking issue on for our [`?` macro feature][tracking].
+
+[tracking]: https://github.com/rust-lang/rust/issues/48075
+
<a name="impl"></a>
## Implementation
@@ -199,15 +207,30 @@ There are a couple of things that may happen for some PRs during the review proc
to check if you introduced a change to compiler behavior that affects a large
portion of the ecosystem.
- If the diff of your PR is large or the reviewer is busy, your PR may have
- some merge conflicts with other PRs. You should fix these merge conflicts
- using the normal git procedures.
+ some merge conflicts with other PRs that happen to get merged first. You
+ should fix these merge conflicts using the normal git procedures.
[crater]: ./tests/intro.html#crater
+If you are not doing a new feature or something like that (e.g. if you are
+fixing a bug), then that's it! Thanks for your contribution :)
+
## Refining your implementation
+As people get experience with your new feature on nightly, slight changes may
+be proposed and unresolved questions may become resolved. Updates/changes go
+through the same process for implementing any other changes, as described
+above (i.e. submit a PR, go through review, wait for `@bors`, etc).
+
+Some changes may be major enough to require an FCP and some review by rust team
+members.
+
+For the `?` macro feature, we went through a few different iterations after the
+original implementation: [1][impl2], [2][impl3], [3][impl4].
+
TODO
+
## Stabilization
TODO
From ad8ebf263ce6d42c05c7dbb7be05e01cb3887ef3 Mon Sep 17 00:00:00 2001
From: Mark Mansi <markm@cs.wisc.edu>
Date: Mon, 19 Nov 2018 15:34:02 -0600
Subject: [PATCH 4/8] finish walkthrough
---
src/walkthrough.md | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/src/walkthrough.md b/src/walkthrough.md
index 8cd511d92..4eae1397d 100644
--- a/src/walkthrough.md
+++ b/src/walkthrough.md
@@ -228,9 +228,34 @@ members.
For the `?` macro feature, we went through a few different iterations after the
original implementation: [1][impl2], [2][impl3], [3][impl4].
-TODO
-
+Along the way, we decided that `?` should not take a separator, which was
+previously an unresolved question listed in the RFC. We also changed the
+disambiguation strategy: we decided to remove the ability to use `?` as a
+separator token for other repetition operators (e.g. `+` or `*`). However,
+since this was a breaking change, we decided to do it over an edition boundary.
+Thus, the new feature can be enabled only in edition 2018. These deviations
+from the original RFC required [another
+FCP](https://github.com/rust-lang/rust/issues/51934).
## Stabilization
-TODO
+Finally, after the feature had baked for a while on nightly, a language team member
+[moved to stabilize it][stabilizefcp].
+
+[stabilizefcp]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433177613
+
+A _stabilization report_ needs to be written that includes
+
+- brief description of the behavior and any deviations from the RFC
+- which edition(s) are affected and how
+- links to a few tests to show the interesting aspects
+
+The stabilization report for our feature is [here][stabrep].
+
+[stabrep]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433243048
+
+After this, a PR is made to remove the feature gate, enabling the feature by
+default (on the 2018 edition). A note is added to the [Release notes][relnotes]
+about the feature.
+
+[relnotes]: https://github.com/rust-lang/rust/blob/master/RELEASES.md
From 49c3d4a36eebab51f5c767f4798daed72e780b56 Mon Sep 17 00:00:00 2001
From: Mark Mansi <markm@cs.wisc.edu>
Date: Mon, 26 Nov 2018 11:06:41 -0600
Subject: [PATCH 5/8] add link to stabilization pr
---
src/walkthrough.md | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/walkthrough.md b/src/walkthrough.md
index 4eae1397d..e77a0f690 100644
--- a/src/walkthrough.md
+++ b/src/walkthrough.md
@@ -254,8 +254,16 @@ The stabilization report for our feature is [here][stabrep].
[stabrep]: https://github.com/rust-lang/rust/issues/48075#issuecomment-433243048
-After this, a PR is made to remove the feature gate, enabling the feature by
+After this, [a PR is made][stab] to remove the feature gate, enabling the feature by
default (on the 2018 edition). A note is added to the [Release notes][relnotes]
about the feature.
+[stab]: https://github.com/rust-lang/rust/pull/56245
+
+TODO: currently, we have a [forge article][feature-stab] about stabilization, but
+we really ought to move that to the guide (in fact, we probably should have a whole
+chapter about feature gates and stabilization).
+
+[feature-stab]: https://forge.rust-lang.org/stabilization-guide.html
+
[relnotes]: https://github.com/rust-lang/rust/blob/master/RELEASES.md
From 3940e94a5e24e454f662ba5c13c1dceed50ff6a4 Mon Sep 17 00:00:00 2001
From: Who? Me?! <mark-i-m@users.noreply.github.com>
Date: Mon, 26 Nov 2018 15:24:39 -0600
Subject: [PATCH 6/8] fix relative link (#242)
---
src/traits/lowering-module.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/traits/lowering-module.md b/src/traits/lowering-module.md
index c9c4accc1..939484051 100644
--- a/src/traits/lowering-module.md
+++ b/src/traits/lowering-module.md
@@ -63,4 +63,4 @@ error: aborting due to previous error
[chalkify]: https://github.com/rust-lang/rust/tree/master/src/test/ui/chalkify
[lower_impl]: https://github.com/rust-lang/rust/tree/master/src/test/ui/chalkify/lower_impl.rs
[the stderr file]: https://github.com/rust-lang/rust/tree/master/src/test/ui/chalkify/lower_impl.stderr
-[ui test]: https://rust-lang-nursery.github.io/rustc-guide/tests/adding.html#guide-to-the-ui-tests
+[ui test]: ../tests/adding.html#guide-to-the-ui-tests
From 2f4e10ce5bf34bc2832b60f5ae4ff344333c3110 Mon Sep 17 00:00:00 2001
From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
Date: Sun, 2 Dec 2018 01:10:43 +0400
Subject: [PATCH 7/8] Fix failing tidy (line endings on Windows) (#246)
This happens every time a new doc submodule is added to https://github.com/rust-lang/rust.
cc https://github.com/rust-lang/book/pull/549 https://github.com/rust-lang-nursery/reference/pull/36 https://github.com/rust-lang/rust-by-example/pull/1018
---
.gitattributes | 2 ++
1 file changed, 2 insertions(+)
create mode 100644 .gitattributes
diff --git a/.gitattributes b/.gitattributes
new file mode 100644
index 000000000..d56abbf30
--- /dev/null
+++ b/.gitattributes
@@ -0,0 +1,2 @@
+# Set the default behavior, in case people don't have core.autocrlf set.
+* text=auto eol=lf
From 344c4e437ba4cfa5c14db643ec4d6b68dcd164c5 Mon Sep 17 00:00:00 2001
From: yui-knk <spiketeika@gmail.com>
Date: Sat, 1 Dec 2018 22:11:05 +0900
Subject: [PATCH 8/8] Fix the file path where AST is defined
---
src/high-level-overview.md | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/high-level-overview.md b/src/high-level-overview.md
index 733c523c9..93b5592c3 100644
--- a/src/high-level-overview.md
+++ b/src/high-level-overview.md
@@ -96,7 +96,7 @@ take:
1. **Parsing input**
- this processes the `.rs` files and produces the AST
("abstract syntax tree")
- - the AST is defined in `syntax/ast.rs`. It is intended to match the lexical
+ - the AST is defined in `src/libsyntax/ast.rs`. It is intended to match the lexical
syntax of the Rust language quite closely.
2. **Name resolution, macro expansion, and configuration**
- once parsing is complete, we process the AST recursively, resolving