diff --git a/src/SUMMARY.md b/src/SUMMARY.md index a2f40ed35..981eb22c9 100644 --- a/src/SUMMARY.md +++ b/src/SUMMARY.md @@ -2,7 +2,10 @@ - [About this guide](./about-this-guide.md) - [How to build the compiler and run what you built](./how-to-build-and-run.md) -- [Using the compiler testing framework](./running-tests.md) +- [Coding conventions](./conventions.md) +- [The compiler testing framework](./tests/intro.md) + - [Running tests](./tests/running.md) + - [Adding new tests](./tests/adding.md) - [Walkthrough: a typical contribution](./walkthrough.md) - [High-level overview of the compiler source](./high-level-overview.md) - [Queries: demand-driven compilation](./query.md) diff --git a/src/conventions.md b/src/conventions.md new file mode 100644 index 000000000..f7ef86f11 --- /dev/null +++ b/src/conventions.md @@ -0,0 +1,146 @@ +This file offers some tips on the coding conventions for rustc. This +chapter covers [formatting](#formatting), [coding for correctness](#cc), +[using crates from crates.io](#cio), and some tips on +[structuring your PR for easy review](#er). + + + +# Formatting and the tidy script + +rustc is slowly moving towards the [Rust standard coding style][fmt]; +at the moment, however, it follows a rather more *chaotic* style. We +do have some mandatory formatting conventions, which are automatically +enforced by a script we affectionately call the "tidy" script. The +tidy script runs automatically when you do `./x.py test`. + +[fmt]: https://github.com/rust-lang-nursery/fmt-rfcs + + + +### Copyright notice + +All files must begin with the following copyright notice: + +``` +// Copyright 2012-2013 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. +``` + +The year at the top is not meaningful: copyright protections are in +fact automatic from the moment of authorship. We do not typically edit +the years on existing files. When creating a new file, you can use the +current year if you like, but you don't have to. + +## Line length + +Lines should be at most 100 characters. It's even better if you can +keep things to 80. + +**Ignoring the line length limit.** Sometimes -- in particular for +tests -- it can be necessary to exempt yourself from this limit. In +that case, you can add a comment towards the top of the file (after +the copyright notice) like so: + +``` +// ignore-tidy-linelength +``` + +## Tabs vs spaces + +Prefer 4-space indent. + + + +# Coding for correctness + +Beyond formatting, there are a few other tips that are worth +following. + +## Prefer exhaustive matches + +Using `_` in a match is convenient, but it means that when new +variants are added to the enum, they may not get handled correctly. +Ask yourself: if a new variant were added to this enum, what's the +chance that it would want to use the `_` code, versus having some +other treatment? Unless the answer is "low", then prefer an +exhaustive match. (The same advice applies to `if let` and `while +let`, which are effectively tests for a single variant.) + +## Use "TODO" comments for things you don't want to forget + +As a useful tool to yourself, you can insert a `// TODO` comment +for something that you want to get back to before you land your PR: + +```rust,ignore +fn do_something() { + if something_else { + unimplemented!(): // TODO write this + } +} +``` + +The tidy script will report an error for a `// TODO` comment, so this +code would not be able to land until the TODO is fixed (or removed). + +This can also be useful in a PR as a way to signal from one commit that you are +leaving a bug that a later commit will fix: + +```rust,ignore +if foo { + return true; // TODO wrong, but will be fixed in a later commit +} +``` + + + +# Using crates from crates.io + +It is allowed to use crates from crates.io, though external +dependencies should not be added gratuitously. All such crates must +have a suitably permissive license. There is an automatic check which +inspects the Cargo metadata to ensure this. + + + +# How to structure your PR + +How you prepare the commits in your PR can make a big difference for the reviewer. +Here are some tips. + +**Isolate "pure refactorings" into their own commit.** For example, if +you rename a method, then put that rename into its own commit, along +with the renames of all the uses. + +**More commits is usually better.** If you are doing a large change, +it's almost always better to break it up into smaller steps that can +be independently understood. The one thing to be aware of is that if +you introduce some code following one strategy, then change it +dramatically (versus adding to it) in a later commit, that +'back-and-forth' can be confusing. + +**If you run rustfmt and the file was not already formatted, isolate +that into its own commit.** This is really the same as the previous +rule, but it's worth highlighting. It's ok to rustfmt files, but since +we do not currently run rustfmt all the time, that can introduce a lot +of noise into your commit. Please isolate that into its own +commit. This also makes rebases a lot less painful, since rustfmt +tends to cause a lot of merge conflicts, and having those isolated +into their own commit makes htem easier to resolve. + +**No merges.** We do not allow merge commits into our history, other +than those by bors. If you get a merge conflict, rebase instead via a +command like `git rebase -i rust-lang/master` (presuming you use the +name `rust-lang` for your remote). + +**Individual commits do not have to build (but it's nice).** We do not +require that every intermediate commit successfully builds -- we only +expect to be able to bisect at a PR level. However, if you *can* make +individual commits build, that is always helpful. + diff --git a/src/how-to-build-and-run.md b/src/how-to-build-and-run.md index 2657da84c..7b6088c7c 100644 --- a/src/how-to-build-and-run.md +++ b/src/how-to-build-and-run.md @@ -132,6 +132,6 @@ Here are a few other useful x.py commands. We'll cover some of them in detail in - `./x.py clean` – clean up the build directory (`rm -rf build` works too, but then you have to rebuild LLVM) - `./x.py build --stage 1` – builds everything using the stage 1 compiler, not just up to libstd - `./x.py build` – builds the stage2 compiler -- Running tests (see the section [running tests](./running-tests.html) for more details): +- Running tests (see the [section on running tests](./tests/running.html) for more details): - `./x.py test --stage 1 src/libstd` – runs the `#[test]` tests from libstd - `./x.py test --stage 1 src/test/run-pass` – runs the `run-pass` test suite diff --git a/src/running-tests.md b/src/running-tests.md deleted file mode 100644 index 02c9de840..000000000 --- a/src/running-tests.md +++ /dev/null @@ -1 +0,0 @@ -# Using the compiler testing framework diff --git a/src/tests/adding.md b/src/tests/adding.md new file mode 100644 index 000000000..276189f59 --- /dev/null +++ b/src/tests/adding.md @@ -0,0 +1,304 @@ +# Adding new tests + +**In general, we expect every PR that fixes a bug in rustc to come +accompanied by a regression test of some kind.** This test should fail +in master but pass after the PR. These tests are really useful for +preventing us from repeating the mistakes of the past. + +To add a new test, the first thing you generally do is to create a +file, typically a Rust source file. Test files have a particular +structure: + +- They always begin with the [copyright notice](./conventions.html#copyright); +- then they should have some kind of + [comment explaining what the test is about](#explanatory_comment); +- next, they can have one or more [header commands](#header_commands), which are special + comments that the test interpreter knows how to interpret. +- finally, they have the Rust source. This may have various [error + annotations](#error_annotations) which indicate expected compilation errors or + warnings. + +Depending on the test suite, there may be some other details to be aware of: + - For [the `ui` test suite](#ui), you need to generate reference output files. + +## What kind of test should I add? + +It can be difficult to know what kind of test to use. Here are some +rough heuristics: + +- Some tests have specialized needs: + - need to run gdb or lldb? use the `debuginfo` test suite + - need to inspect LLVM IR or MIR IR? use the `codegen` or `mir-opt` test suites + - need to run rustdoc? Prefer a `rustdoc` test + - need to inspect the resulting binary in some way? Then use `run-make` +- For most other things, [a `ui` (or `ui-fulldeps`) test](#ui) is to be preferred: + - `ui` tests subsume both run-pass, compile-fail, and parse-fail tests + - in the case of warnings or errors, `ui` tests capture the full output, + which makes it easier to review but also helps prevent "hidden" regressions + in the output + +## Naming your test + +We have not traditionally had a lot of structure in the names of +tests. Moreover, for a long time, the rustc test runner did not +support subdirectories (it now does), so test suites like +[`src/test/run-pass`] have a huge mess of files in them. This is not +considered an ideal setup. + +[`src/test/run-pass`]: https://github.com/rust-lang/rust/tree/master/src/test/run-pass/ + +For regression tests -- basically, some random snippet of code that +came in from the internet -- we often just name the test after the +issue. For example, `src/test/run-pass/issue-12345.rs`. If possible, +though, it is better if you can put the test into a directory that +helps identify what piece of code is being tested here (e.g., +`borrowck/issue-12345.rs` is much better), or perhaps give it a more +meaningful name. Still, **do include the issue number somewhere**. + +When writing a new feature, **create a subdirectory to store your +tests**. For example, if you are implementing RFC 1234 ("Widgets"), +then it might make sense to put the tests in directories like: + +- `src/test/ui/rfc1234-widgets/` +- `src/test/run-pass/rfc1234-widgets/` +- etc + +In other cases, there may already be a suitable directory. (The proper +directory structure to use is actually an area of active debate.) + + + +## Comment explaining what the test is about + +When you create a test file, **include a comment summarizing the point +of the test immediately after the copyright notice**. This should +highlight which parts of the test are more important, and what the bug +was that the test is fixing. Citing an issue number is often very +helpful. + +This comment doesn't have to be super extensive. Just something like +"Regression test for #18060: match arms were matching in the wrong +order." might already be enough. + +These comments are very useful to others later on when your test +breaks, since they often can highlight what the problem is. They are +also useful if for some reason the tests need to be refactored, since +they let others know which parts of the test were important (often a +test must be rewritten because it no longer tests what is was meant to +test, and then it's useful to know what it *was* meant to test +exactly). + + + +## Header commands: configuring rustc + +Header commands are special comments that the test runner knows how to +interpret. They must appear before the Rust source in the test. They +are normally put after the short comment that explains the point of +this test. For example, this test uses the `// compile-flags` command +to specify a custom flag to give to rustc when the test is compiled: + +```rust +// Copyright 2017 The Rust Project Developers. blah blah blah. +// ... +// except according to those terms. + +// Test the behavior of `0 - 1` when overflow checks are disabled. + +// compile-flags: -Coverflow-checks=off + +fn main() { + let x = 0 - 1; + ... +} +``` + +### Ignoring tests + +These are used to ignore the test in some situations, which means the test won't +be compiled or run. + +* `ignore-X` where `X` is a target detail or stage will ignore the test accordingly (see below) +* `ignore-pretty` will not compile the pretty-printed test (this is done to test the pretty-printer, but might not always work) +* `ignore-test` always ignores the test +* `ignore-lldb` and `ignore-gdb` will skip a debuginfo test on that debugger. + +Some examples of `X` in `ignore-X`: + +* Architecture: `aarch64`, `arm`, `asmjs`, `mips`, `wasm32`, `x86_64`, `x86`, ... +* OS: `android`, `emscripten`, `freebsd`, `ios`, `linux`, `macos`, `windows`, ... +* Environment (fourth word of the target triple): `gnu`, `msvc`, `musl`. +* Pointer width: `32bit`, `64bit`. +* Stage: `stage0`, `stage1`, `stage2`. + +### Other Header Commands + +Here is a list of other header commands. This list is not +exhaustive. Header commands can generally be found by browsing the +`TestProps` structure found in [`header.rs`] from the compiletest +source. + +* `min-{gdb,lldb}-version` +* `min-llvm-version` +* `must-compile-successfully` for UI tests, indicates that the test is supposed + to compile, as opposed to the default where the test is supposed to error out. +* `compile-flags` passes extra command-line args to the compiler, + e.g. `compile-flags -g` which forces debuginfo to be enabled. +* `should-fail` indicates that the test should fail; used for "meta testing", + where we test the compiletest program itself to check that it will generate + errors in appropriate scenarios. This header is ignored for pretty-printer tests. +* `gate-test-X` where `X` is a feature marks the test as "gate test" for feature X. + Such tests are supposed to ensure that the compiler errors when usage of a gated + feature is attempted without the proper `#![feature(X)]` tag. + Each unstable lang feature is required to have a gate test. + +[`header.rs`]: https://github.com/rust-lang/rust/tree/master/src/tools/compiletest/src/header.rs + + + +## Error annotations + +Error annotations specify the errors that the compiler is expected to +emit. They are "attached" to the line in source where the error is +located. + +* `~`: Associates the following error level and message with the + current line +* `~|`: Associates the following error level and message with the same + line as the previous comment +* `~^`: Associates the following error level and message with the + previous line. Each caret (`^`) that you add adds a line to this, so + `~^^^^^^^` is seven lines up. + +The error levels that you can have are: + +1. `ERROR` +2. `WARNING` +3. `NOTE` +4. `HELP` and `SUGGESTION`* + +\* **Note**: `SUGGESTION` must follow immediately after `HELP`. + +## Revisions + +Certain classes of tests support "revisions" (as of the time of this +writing, this includes run-pass, compile-fail, run-fail, and +incremental, though incremental tests are somewhat +different). Revisions allow a single test file to be used for multiple +tests. This is done by adding a special header at the top of the file: + +``` +// revisions: foo bar baz +``` + +This will result in the test being compiled (and tested) three times, +once with `--cfg foo`, once with `--cfg bar`, and once with `--cfg +baz`. You can therefore use `#[cfg(foo)]` etc within the test to tweak +each of these results. + +You can also customize headers and expected error messages to a particular +revision. To do this, add `[foo]` (or `bar`, `baz`, etc) after the `//` +comment, like so: + +``` +// A flag to pass in only for cfg `foo`: +//[foo]compile-flags: -Z verbose + +#[cfg(foo)] +fn test_foo() { + let x: usize = 32_u32; //[foo]~ ERROR mismatched types +} +``` + +Note that not all headers have meaning when customized to a revision. +For example, the `ignore-test` header (and all "ignore" headers) +currently only apply to the test as a whole, not to particular +revisions. The only headers that are intended to really work when +customized to a revision are error patterns and compiler flags. + + + +## Guide to the UI tests + +The UI tests are intended to capture the compiler's complete output, +so that we can test all aspects of the presentation. They work by +compiling a file (e.g., [`ui/hello_world/main.rs`][hw-main]), +capturing the output, and then applying some normalization (see +below). This normalized result is then compared against reference +files named `ui/hello_world/main.stderr` and +`ui/hello_world/main.stdout`. If either of those files doesn't exist, +the output must be empty (that is actually the case for +[this particular test][hw]). If the test run fails, we will print out +the current output, but it is also saved in +`build//test/ui/hello_world/main.stdout` (this path is +printed as part of the test failure message), so you can run `diff` +and so forth. + +[hw-main]: https://github.com/rust-lang/rust/blob/master/src/test/ui/hello_world/main.rs +[hw]: https://github.com/rust-lang/rust/blob/master/src/test/ui/hello_world/ + +### Tests that do not result in compile errors + +By default, a UI test is expected **not to compile** (in which case, +it should contain at least one `//~ ERROR` annotation). However, you +can also make UI tests where compilation is expected to succeed, and +you can even run the resulting program. Just add one of the following +[header commands](#header_commands): + +- `// must-compile-successfully` -- compilation should succeed but do not run the resulting binary +- `// run-pass` -- compilation should succeed and we should run the resulting binary + +### Editing and updating the reference files + +If you have changed the compiler's output intentionally, or you are +making a new test, you can use the script `ui/update-references.sh` to +update the references. When you run the test framework, it will report +various errors: in those errors is a command you can use to run the +`ui/update-references.sh` script, which will then copy over the files +from the build directory and use them as the new reference. You can +also just run `ui/update-all-references.sh`. In both cases, you can run +the script with `--help` to get a help message. + +### Normalization + +The normalization applied is aimed at eliminating output difference +between platforms, mainly about filenames: + +- the test directory is replaced with `$DIR` +- all backslashes (`\`) are converted to forward slashes (`/`) (for Windows) +- all CR LF newlines are converted to LF + +Sometimes these built-in normalizations are not enough. In such cases, you +may provide custom normalization rules using the header commands, e.g. + +``` +// normalize-stdout-test: "foo" -> "bar" +// normalize-stderr-32bit: "fn\(\) \(32 bits\)" -> "fn\(\) \($$PTR bits\)" +// normalize-stderr-64bit: "fn\(\) \(64 bits\)" -> "fn\(\) \($$PTR bits\)" +``` + +This tells the test, on 32-bit platforms, whenever the compiler writes +`fn() (32 bits)` to stderr, it should be normalized to read `fn() ($PTR bits)` +instead. Similar for 64-bit. The replacement is performed by regexes using +default regex flavor provided by `regex` crate. + +The corresponding reference file will use the normalized output to test both +32-bit and 64-bit platforms: + +``` +... + | + = note: source type: fn() ($PTR bits) + = note: target type: u16 (16 bits) +... +``` + +Please see [`ui/transmute/main.rs`][mrs] and [`main.stderr`][] for a concrete usage example. + +[mrs]: https://github.com/rust-lang/rust/blob/master/src/test/ui/transmute/main.rs +[`main.stderr`]: https://github.com/rust-lang/rust/blob/master/src/test/ui/transmute/main.stderr + +Besides `normalize-stderr-32bit` and `-64bit`, one may use any target +information or stage supported by `ignore-X` here as well (e.g. +`normalize-stderr-windows` or simply `normalize-stderr-test` for unconditional +replacement). diff --git a/src/tests/intro.md b/src/tests/intro.md new file mode 100644 index 000000000..615673206 --- /dev/null +++ b/src/tests/intro.md @@ -0,0 +1,57 @@ +# Using the compiler testing framework + +The compiler has an extensive testing framework, masterminded by the +compiletest tool (sources in the [`src/tools/compiletest`]). This +section gives a brief overview of how the testing framework is setup, +and then gets into some of the details on +[how to run tests](./tests/running.html#ui) as well as +[how to add new tests](./tests/adding.html). + +[`src/tools/compiletest`]: https://github.com/rust-lang/rust/tree/master/src/tools/compiletest + +## Test suites + +The tests are located in the tree in the [`src/test`] +directory. Immediately within you will see a series of subdirectories +(e.g. `ui`, `run-make`, and so forth). Each of those directories is +called a **test suite** -- they house a group of tests that are run in +a distinct mode. + +[`src/test`]: https://github.com/rust-lang/rust/tree/master/src/test + +Here is a brief summary of the test suites as of this writing and what +they mean. In some cases, the test suites are linked to parts of the manual +that give more details. + +- [`ui`](./tests/adding.html#ui) -- tests that check the exact stdout/stderr from compilation + and/or running the test +- `run-pass` -- tests that are expected to compile and execute successfully (no panics) + - `run-pass-valgrind` -- tests that ought to run with valrind +- `run-fail` -- tests that are expected to compile but then panic during execution +- `compile-fail` -- tests that are expected to fail compilation. +- `parse-fail` -- tests that are expected to fail to parse +- `pretty` -- tests targeting the Rust "pretty printer", which + generates valid Rust code from the AST +- `debuginfo` -- tests that run in gdb or lldb and query the debug info +- `codegen` -- tests that compile and then test the generated LLVM + code to make sure that the optimizations we want are taking effect. +- `mir-opt` -- tests that check parts of the generated MIR to make + sure we are building things correctly or doing the optimizations we + expect. +- `incremental` -- tests for incremental compilation, checking that + when certain modifications are performed, we are able to reuse the + results from previous compilations. +- `run-make` -- tests that basically just execute a `Makefile`; the + ultimate in flexibility but quite annoying to write. +- `rustdoc` -- tests for rustdoc, making sure that the generated files contain + the expected documentation. +- `*-fulldeps` -- same as above, but indicates that the test depends on things other + than `libstd` (and hence those things must be built) + +## Further reading + +The following blog posts may also be of interest: + +- brson's classic ["How Rust is tested"][howtest] + +[howtest]: https://brson.github.io/2017/07/10/how-rust-is-tested diff --git a/src/tests/running.md b/src/tests/running.md new file mode 100644 index 000000000..602aa51d9 --- /dev/null +++ b/src/tests/running.md @@ -0,0 +1,72 @@ +# Running tests + +You can run the tests using `x.py`. The most basic command -- which +you will almost never want to use! -- is as follows: + +``` +./x.py test +``` + +This will build the full stage 2 compiler and then run the whole test +suite. You probably don't want to do this very often, because it takes +a very long time, and anyway bors / travis will do it for you. (Often, +I will run this command in the background after opening a PR that I +think is done, but rarely otherwise. -nmatsakis) + +## Tidy + +When you run the full suite of tests via `./x.py test`, the first +thing that executes is a "tidy suite" that checks for long lines and +other formatting conventions. There is more information in the +[section on coding conventions](./conventions.html#formatting). + +## Running a subset of the test suites + +When working on a specific PR, you will usually want to run a smaller +set of tests, and with a stage 1 build. For example, a good "smoke +test" that can be used after modifying rustc to see if things are +generally working correctly would be the following: + +```bash +./x.py test --stage 1 src/test/{ui,compile-fail,run-pass} +``` + +This will run the `ui`, `compile-fail`, and `run-pass` test suites, and +only with the stage 1 build. Of course, the choice of test suites is somewhat +arbitrary, and may not suit the task you are doing. For example, if you are hacking +on debuginfo, you may be better off with the debuginfo test suite: + +```bash +./x.py test --stage 1 src/test/debuginfo +``` + +**Warning:** Note that bors only runs the tests with the full stage 2 +build; therefore, while the tests **usually** work fine with stage 1, +there are some limitations. In particular, the stage1 compiler doesn't +work well with procedural macros or custom derive tests. + +## Running an individual test + +Another common thing that people want to do is to run an **individual +test**, often the test they are trying to fix. One way to do this is +to invoke `x.py` with the `--test-args` option: + +``` +./x.py test --stage 1 src/test/ui --test-args issue-1234 +``` + +Under the hood, the test runner invokes the standard rust test runner +(the same one you get with `#[test]`), so this command would wind up +filtering for tests that include "issue-1234" in the name. + +Often, though, it's easier to just run the test by hand. Most tests are +just `rs` files, so you can do something like + +``` +rustc +stage1 src/test/ui/issue-1234.rs +``` + +This is much faster, but doesn't always work. For example, some tests +include directives that specify specific compiler flags, or which rely +on other crates, and they may not run the same without those options. +