rework the README.md for rustc and add other readmes #44505

nikomatsakis · 2017-09-11T20:29:32Z

OK, so, long ago I committed to the idea of trying to write some high-level documentation for rustc. This has proved to be much harder for me to get done than I thought it would! This PR is far from as complete as I had hoped, but I wanted to open it so that people can give me feedback on the conventions that it establishes. If this seems like a good way forward, we can land it and I will open an issue with a good check-list of things to write (and try to take down some of them myself).

Here are the conventions I established on which I would like feedback.

Use README.md files. First off, I'm aiming to keep most of the high-level docs in README.md files, rather than entries on forge. My thought is that such files are (a) more discoverable than forge and (b) closer to the code, and hence can be edited in a single PR. However, since they are not in the code, they will naturally get out of date, so the intention is to focus on the highest-level details, which are least likely to bitrot. I've included a few examples of common functions and so forth, but never tried to (e.g.) exhaustively list the names of functions and so forth.
- I would like to use the tidy scripts to try and check that these do not go out of date. Future work.

librustc/README.md as the main entrypoint. This seems like the most natural place people will look first. It lays out how the crates are structured and is intended to give pointers to the main data structures of the compiler (I didn't update that yet; the existing material is terribly dated).

A glossary listing abbreviations and things. It's much harder to read code if you don't know what some obscure set of letters like infcx stands for.

Major modules each have their own README.md that documents the high-level idea. For example, I wrote some stuff about hir and ty. Both of them have many missing topics, but I think that is roughly the level of depth that would be good. The idea is to give people a "feeling" for what the code does.

What is missing primarily here is lots of content. =) Here are some things I'd like to see:

A description of what a QUERY is and how to define one
- Some comments for librustc/ty/maps.rs
An overview of how compilation proceeds now (i.e., the hybrid demand-driven and forward model) and how we would like to see it going in the future (all demand-driven)
Some coverage of how incremental will work under red-green
An updated list of the major IRs in use of the compiler (AST, HIR, TypeckTables, MIR) and major bits of interesting code (typeck, borrowck, etc)
More advice on how to use x.py, or at least pointers to that
Good choice for config.toml
How to use RUST_LOG and other debugging flags (e.g., -Zverbose, -Ztreat-err-as-bug)
Helpful conventions for debug! statement formatting

cc @rust-lang/compiler @mgattozzi

rust-highfive · 2017-09-11T20:29:37Z

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

steveklabnik

Wonderful! Always glad to see more docs, and these are great.

I have some small nits on formatting, take 'em or leave 'em.

steveklabnik · 2017-09-11T21:00:30Z

src/librustc/hir/README.md

+
+
+
+


you may want to cut this down to one newline

steveklabnik · 2017-09-11T21:01:05Z

src/librustc/hir/mod.rs

+///
+/// Example:
+///
+/// ```rust


conventionally the explicit rust here is only used in markdown files, not source files

steveklabnik · 2017-09-11T21:01:40Z

src/librustc/hir/mod.rs

+/// (which is an expression), but also the argument patterns, since
+/// those are something that the caller doesn't really care about.
+///
+/// Example:


Conventionally, this would be

/// # Examples

steveklabnik · 2017-09-11T21:02:31Z

src/librustc/ty/README.md

+
+As you can see, the `TyCtxt` type takes three lifetime parameters.
+These lifetimes are perhaps the most complex thing to understand about
+the tcx. During rust compilation, we allocate most of our memory in


maybe "Rust"?

steveklabnik · 2017-09-11T21:03:40Z

src/librustc/ty/README.md

+allocated in the global arena). However, the lifetime `'tcx` is always
+a safe approximation, so that is what you get back.
+
+NB. Because types are interned, it is possible to compare them for


you may or may not want to consider using a > for this note

steveklabnik · 2017-09-11T21:03:52Z

src/librustc/ty/README.md

+
+
+
+


extra newlines here too

eddyb · 2017-09-11T21:49:29Z

r? @steveklabnik

petrochenkov · 2017-09-11T22:24:05Z

src/librustc/hir/README.md

+to a `&hir::Item` (e.g. for the mod `foo`), you do not immediately
+gain access to the contents of the function `bar()`. Instead, you only
+gain access to the **id** for `bar()`, and you must some function to
+lookup the contents of `bar()` given its id; this gives us a change to


change -> chance?

petrochenkov · 2017-09-11T22:32:06Z

src/librustc/hir/README.md

+of a function/closure or the definition of a constant. Bodies are
+associated with an **owner**, which is typically some kind of item
+(e.g., a `fn()` or `const`), but could also be a closure expression
+(e.g., `|x, y| x + y`). You can use the HIR map to find find the body


typo: find find

mgattozzi · 2017-09-12T01:30:54Z

@nikomatsakis I'm glad you finally got around to this! I'll take a look at this later when I get the chance!

qmx · 2017-09-12T02:17:01Z

src/librustc_driver/README.md

+the code from the other crates within rustc. This crate itself does
+not contain any of the "main logic" of the compiler (though it does
+have some code related to pretty printing or other minor compiler
+options).


This definition of driver as being the "main" function for the compiler is really nice. What about giving more emphasis to it on the top-level README file?

tbg · 2017-09-12T03:22:49Z

src/librustc/README.md

+better than others).
+
+The compiler process
+====================


Unable to comment below this part of the diff, but between the analysis oasses and translation to LLVM there's also HAIR and MIR lowering. Would be nice to mention those as well to see how they fit into the big picture. (I believe HAIR is where the typeck tables are terminated (as in flushed out), for example?).

Yeah, I think this part is basically all dated. I just pulled it unedited from before.

tbg · 2017-09-12T03:25:23Z

src/librustc/hir/README.md

+integration with incremental compilation. This way, if you gain access
+to a `&hir::Item` (e.g. for the mod `foo`), you do not immediately
+gain access to the contents of the function `bar()`. Instead, you only
+gain access to the **id** for `bar()`, and you must some function to


and you must X some function

tbg · 2017-09-12T03:28:06Z

src/librustc/hir/mod.rs

@@ -413,6 +413,9 @@ pub struct WhereEqPredicate {

 pub type CrateConfig = HirVec<P<MetaItem>>;

+/// The top-level data structure that stores the entire contents of
+/// the crate currently being compiled.
+///


Does this newline serve a purpose?

tbg

Definitely helpful. Left a few comments but none of them major.

tbg · 2017-09-12T03:31:49Z

src/librustc/ty/README.md

+pub type Ty<'tcx> = &'tcx TyS<'tcx>;
+```
+
+The `TyS` struct defines the actual details of how a type is


I've wondered what the S in TyS is for. And similarly the s in sty. Maybe this is one place to mention it.

tbg · 2017-09-12T03:33:55Z

src/librustc/ty/context.rs

-/// generates so that so that it can be reused and doesn't have to be redone
-/// later on.
+/// The central data structure of the compiler. Keeps track of all the
+/// information that typechecker generates so that so that it can be


nit: "the typechecker" or "typechecking"?

tbg · 2017-09-12T03:34:32Z

src/librustc/ty/context.rs

-/// later on.
+/// The central data structure of the compiler. Keeps track of all the
+/// information that typechecker generates so that so that it can be
+/// reused and doesn't have to be redone later on.


nit: nothing after "reused" seems to add anything.

@eddyb

bring TyCtxt into scope got comments both from @eddyb and @nikomatsakis (via rust-lang#44505) that we should always put `TyCtxt` in scope should I just go and import it at other places in the codebase or we just keep doing small improvements?

@eddyb

bring TyCtxt into scope got comments both from @eddyb and @nikomatsakis (via rust-lang#44505) that we should always put `TyCtxt` in scope should I just go and import it at other places in the codebase or we just keep doing small improvements?

@eddyb

bring TyCtxt into scope got comments both from @eddyb and @nikomatsakis (via rust-lang#44505) that we should always put `TyCtxt` in scope should I just go and import it at other places in the codebase or we just keep doing small improvements?

@eddyb

bring TyCtxt into scope got comments both from @eddyb and @nikomatsakis (via rust-lang#44505) that we should always put `TyCtxt` in scope should I just go and import it at other places in the codebase or we just keep doing small improvements?

nikomatsakis · 2017-09-18T13:36:48Z

@eddyb take a look at the last two commits -- I made an attempt to document the query system (I also broke it up into multiple files, to try and hide some of the "plumbing"). I did not rename anything, because I didn't want to run around and update paths for now (though I think it would maybe be best to move this code into queries instead of ty::maps::queries).

alexcrichton · 2017-09-18T17:53:03Z

@bors: r+

bors · 2017-09-18T17:53:04Z

📌 Commit a8a28ce has been approved by alexcrichton

alexcrichton · 2017-09-18T17:53:12Z

@bors: p=2

bors · 2017-09-18T23:45:51Z

🔒 Merge conflict

bors · 2017-09-18T23:49:51Z

☔ The latest upstream changes (presumably #44678) made this pull request unmergeable. Please resolve the merge conflicts.

arielb1 · 2017-09-19T10:42:26Z

src/librustc/README.md

+    - LLVM then runs its various optimizations, which produces a number of `.o` files
+      (one for each "codegen unit").
+6. **Linking**
+    - Finally, those `.o` files are linke together.


arielb1 · 2017-09-19T10:45:15Z

src/librustc/README.md

@@ -226,9 +180,13 @@ pointers for understanding them better.
  found in `src/librustc_mir`.
 - obligation -- something that must be proven by the trait system; see `librustc/traits`.
 - local crate -- the crate currently being compiled.
+- node-id or `NodeId` -- an index identifying a particular node in the


better: gradually being replaced with HirId.

arielb1 · 2017-09-19T10:47:16Z

src/librustc/README.md

 - cx -- we tend to use "cx" as an abbrevation for context. See also tcx, infcx, etc.
+- `DefId` -- an index identifying a **definition** (see `librustc/hir/def_id.rs`).


an index identifying a definition (see librustc/hir/def_id.rs). Uniquely identifies a DefPath.

arielb1 · 2017-09-19T10:48:02Z

src/librustc/README.md

 - HIR -- the **High-level IR**, created by lowering and desugaring the AST. See `librustc/hir`.
+- `HirId` -- identifies a particular node in the HIR by combining a
+  def-id with an "intra-definition offset".
 - `'gcx` -- the lifetime of the global arena (see `librustc/ty`).


ICE - internal compiler error. When the compiler crashes.

This takes way longer than I thought it would. =)

nikomatsakis · 2017-09-19T16:37:38Z

@bors r=steveklabnik p=22

(Besides the fact that it'd be really nice to have these docs be easier to find, this PR is actually kinda' conflict prone due to breaking up the maps module.)

bors · 2017-09-19T16:37:39Z

📌 Commit 638958b has been approved by steveklabnik

bors · 2017-09-19T16:38:17Z

💡 This pull request was already approved, no need to approve it again.

There's another pull request that is currently being tested, blocking this pull request: Optimize drain_filter #44355

bors · 2017-09-19T16:38:18Z

📌 Commit 638958b has been approved by steveklabnik

mgattozzi · 2017-09-19T20:10:36Z

@nikomatsakis I think everyone already covered my thoughts. I think this is a good first stab at it and addresses things I've brought up to you during Meetups so I'm hoping we can continue to do things like this and make compiler hacking more accessible! :D

bors · 2017-09-19T22:44:04Z

⌛ Testing commit 638958b with merge f60bc3a...

@mgattozzi

rework the README.md for rustc and add other readmes OK, so, long ago I committed to the idea of trying to write some high-level documentation for rustc. This has proved to be much harder for me to get done than I thought it would! This PR is far from as complete as I had hoped, but I wanted to open it so that people can give me feedback on the conventions that it establishes. If this seems like a good way forward, we can land it and I will open an issue with a good check-list of things to write (and try to take down some of them myself). Here are the conventions I established on which I would like feedback. **Use README.md files**. First off, I'm aiming to keep most of the high-level docs in `README.md` files, rather than entries on forge. My thought is that such files are (a) more discoverable than forge and (b) closer to the code, and hence can be edited in a single PR. However, since they are not *in the code*, they will naturally get out of date, so the intention is to focus on the highest-level details, which are least likely to bitrot. I've included a few examples of common functions and so forth, but never tried to (e.g.) exhaustively list the names of functions and so forth. - I would like to use the tidy scripts to try and check that these do not go out of date. Future work. **librustc/README.md as the main entrypoint.** This seems like the most natural place people will look first. It lays out how the crates are structured and **is intended** to give pointers to the main data structures of the compiler (I didn't update that yet; the existing material is terribly dated). **A glossary listing abbreviations and things.** It's much harder to read code if you don't know what some obscure set of letters like `infcx` stands for. **Major modules each have their own README.md that documents the high-level idea.** For example, I wrote some stuff about `hir` and `ty`. Both of them have many missing topics, but I think that is roughly the level of depth that would be good. The idea is to give people a "feeling" for what the code does. What is missing primarily here is lots of content. =) Here are some things I'd like to see: - A description of what a QUERY is and how to define one - Some comments for `librustc/ty/maps.rs` - An overview of how compilation proceeds now (i.e., the hybrid demand-driven and forward model) and how we would like to see it going in the future (all demand-driven) - Some coverage of how incremental will work under red-green - An updated list of the major IRs in use of the compiler (AST, HIR, TypeckTables, MIR) and major bits of interesting code (typeck, borrowck, etc) - More advice on how to use `x.py`, or at least pointers to that - Good choice for `config.toml` - How to use `RUST_LOG` and other debugging flags (e.g., `-Zverbose`, `-Ztreat-err-as-bug`) - Helpful conventions for `debug!` statement formatting cc @rust-lang/compiler @mgattozzi

bors · 2017-09-20T01:03:43Z

☀️ Test successful - status-appveyor, status-travis
Approved by: steveklabnik
Pushing f60bc3a to master...

rust-highfive assigned eddyb Sep 11, 2017

steveklabnik reviewed Sep 11, 2017

View reviewed changes

rust-highfive assigned steveklabnik and unassigned eddyb Sep 11, 2017

petrochenkov reviewed Sep 11, 2017

View reviewed changes

qmx reviewed Sep 12, 2017

View reviewed changes

tbg reviewed Sep 12, 2017

View reviewed changes

tbg approved these changes Sep 12, 2017

View reviewed changes

aidanhs added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Sep 13, 2017

qmx mentioned this pull request Sep 14, 2017

bring TyCtxt into scope #44560

Merged

nikomatsakis force-pushed the lotsa-comments branch from 9443a19 to 0b7b668 Compare September 15, 2017 20:32

nikomatsakis force-pushed the lotsa-comments branch from 0b7b668 to 6fa6583 Compare September 18, 2017 13:35

nikomatsakis changed the title ~~[WIP] rework the README.md for rustc and add other readmes~~ rework the README.md for rustc and add other readmes Sep 18, 2017

nikomatsakis force-pushed the lotsa-comments branch from 6fa6583 to b27b709 Compare September 18, 2017 14:23

arielb1 reviewed Sep 19, 2017

View reviewed changes

arielb1 mentioned this pull request Sep 19, 2017

Tracking issue for RFC 1937: ? in main #43301

Closed

12 tasks

nikomatsakis added 7 commits September 19, 2017 09:00

rework the README.md for rustc and add other readmes

44e45d9

This takes way longer than I thought it would. =)

apply various nits

73a4e8d

promote maps into its own directory

76eac36

split maps into submodules, document

70db841

revamp the Compiler Process section to be more up to date

f130e7d

define span

032fdef

start writing some typeck docs (incomplete)

38813cf

nikomatsakis force-pushed the lotsa-comments branch from a8a28ce to 38813cf Compare September 19, 2017 14:39

incorporate suggestions from arielb1

638958b

arielb1 added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 19, 2017

bors merged commit 638958b into rust-lang:master Sep 20, 2017

This was referenced Sep 20, 2017

incr.comp.: Move task result fingerprinting into DepGraph. #44696

Merged

Move effect-checking to MIR #44700

Merged

remove or encapsulate the remaining non-query data in tcx #44501

Merged

		- cx -- we tend to use "cx" as an abbrevation for context. See also tcx, infcx, etc.
		- `DefId` -- an index identifying a definition (see `librustc/hir/def_id.rs`).

rework the README.md for rustc and add other readmes #44505

rework the README.md for rustc and add other readmes #44505

Conversation

nikomatsakis commented Sep 11, 2017 • edited Loading

rust-highfive commented Sep 11, 2017

steveklabnik left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eddyb commented Sep 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mgattozzi commented Sep 12, 2017

qmx Sep 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tbg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Sep 18, 2017

alexcrichton commented Sep 18, 2017

bors commented Sep 18, 2017

alexcrichton commented Sep 18, 2017

bors commented Sep 18, 2017

bors commented Sep 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arielb1 Sep 19, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Sep 19, 2017 • edited Loading

bors commented Sep 19, 2017

bors commented Sep 19, 2017

bors commented Sep 19, 2017

mgattozzi commented Sep 19, 2017

bors commented Sep 19, 2017

bors commented Sep 20, 2017

nikomatsakis commented Sep 11, 2017 •

edited

Loading

qmx Sep 12, 2017 •

edited

Loading

arielb1 Sep 19, 2017 •

edited

Loading

nikomatsakis commented Sep 19, 2017 •

edited

Loading