Skip to content

Commit

Permalink
Auto merge of #49900 - pnkfelix:compare-mode-nll-followup-3, r=nikoma…
Browse files Browse the repository at this point in the history
…tsakis

Add src/test/ui regression testing for NLL

This PR changes `x.py test` so that when you are running the `ui` test suite, it will also always run `compiletest` in the new `--compare-mode=nll`, which just double-checks that when running under the experimental NLL mode, the output matches the `<source-name>.nll.stderr` file, if present.

In order to reduce the chance of a developer revolt in response to this change, this PR also includes some changes to make the `--compare-mode=nll` more user-friendly:

 1. It now generates nll-specific .stamp files, and uses them (so that repeated runs can reuse previously cached results).
 2. Each line of terminal output distinguishes whether we are running under `--compare-mode=nll` by printing with the prefix `[ui (nll)]` instead of just the prefix `[ui]`.

Subtask of #48879
  • Loading branch information
bors committed Apr 19, 2018
2 parents 5fe6b58 + 33bcb4e commit 8a28d94
Show file tree
Hide file tree
Showing 79 changed files with 327 additions and 268 deletions.
45 changes: 42 additions & 3 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,7 @@ impl Step for RustdocUi {
target: self.target,
mode: "ui",
suite: "rustdoc-ui",
compare_mode: None,
})
}
}
Expand Down Expand Up @@ -590,19 +591,44 @@ macro_rules! default_test {
}
}

macro_rules! default_test_with_compare_mode {
($name:ident { path: $path:expr, mode: $mode:expr, suite: $suite:expr,
compare_mode: $compare_mode:expr }) => {
test_with_compare_mode!($name { path: $path, mode: $mode, suite: $suite, default: true,
host: false, compare_mode: $compare_mode });
}
}

macro_rules! host_test {
($name:ident { path: $path:expr, mode: $mode:expr, suite: $suite:expr }) => {
test!($name { path: $path, mode: $mode, suite: $suite, default: true, host: true });
}
}

macro_rules! test {
($name:ident { path: $path:expr, mode: $mode:expr, suite: $suite:expr, default: $default:expr,
host: $host:expr }) => {
test_definitions!($name { path: $path, mode: $mode, suite: $suite, default: $default,
host: $host, compare_mode: None });
}
}

macro_rules! test_with_compare_mode {
($name:ident { path: $path:expr, mode: $mode:expr, suite: $suite:expr, default: $default:expr,
host: $host:expr, compare_mode: $compare_mode:expr }) => {
test_definitions!($name { path: $path, mode: $mode, suite: $suite, default: $default,
host: $host, compare_mode: Some($compare_mode) });
}
}

macro_rules! test_definitions {
($name:ident {
path: $path:expr,
mode: $mode:expr,
suite: $suite:expr,
default: $default:expr,
host: $host:expr
host: $host:expr,
compare_mode: $compare_mode:expr
}) => {
#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
pub struct $name {
Expand Down Expand Up @@ -634,16 +660,18 @@ macro_rules! test {
target: self.target,
mode: $mode,
suite: $suite,
compare_mode: $compare_mode,
})
}
}
}
}

default_test!(Ui {
default_test_with_compare_mode!(Ui {
path: "src/test/ui",
mode: "ui",
suite: "ui"
suite: "ui",
compare_mode: "nll"
});

default_test!(RunPass {
Expand Down Expand Up @@ -804,6 +832,7 @@ struct Compiletest {
target: Interned<String>,
mode: &'static str,
suite: &'static str,
compare_mode: Option<&'static str>,
}

impl Step for Compiletest {
Expand All @@ -823,6 +852,7 @@ impl Step for Compiletest {
let target = self.target;
let mode = self.mode;
let suite = self.suite;
let compare_mode = self.compare_mode;

// Skip codegen tests if they aren't enabled in configuration.
if !builder.config.codegen_tests && suite == "codegen" {
Expand Down Expand Up @@ -1044,6 +1074,15 @@ impl Step for Compiletest {
suite, mode, &compiler.host, target));
let _time = util::timeit(&builder);
try_run(builder, &mut cmd);

if let Some(compare_mode) = compare_mode {
cmd.arg("--compare-mode").arg(compare_mode);
let _folder = builder.fold_output(|| format!("test_{}_{}", suite, compare_mode));
builder.info(&format!("Check compiletest suite={} mode={} compare_mode={} ({} -> {})",
suite, mode, compare_mode, &compiler.host, target));
let _time = util::timeit(&builder);
try_run(builder, &mut cmd);
}
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/librustc/session/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1259,6 +1259,8 @@ options! {DebuggingOptions, DebuggingSetter, basic_debugging_options,
useful for profiling / PGO."),
relro_level: Option<RelroLevel> = (None, parse_relro_level, [TRACKED],
"choose which RELRO level to use"),
nll_subminimal_causes: bool = (false, parse_bool, [UNTRACKED],
"when tracking region error causes, accept subminimal results for faster execution."),
disable_nll_user_type_assert: bool = (false, parse_bool, [UNTRACKED],
"disable user provided type assertion in NLL"),
trans_time_graph: bool = (false, parse_bool, [UNTRACKED],
Expand Down
14 changes: 12 additions & 2 deletions src/librustc_mir/borrow_check/nll/region_infer/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use rustc_data_structures::fx::FxHashMap;
use rustc_data_structures::indexed_vec::Idx;
use rustc_data_structures::indexed_vec::IndexVec;
use rustc::mir::{BasicBlock, Location, Mir};
use rustc::ty::RegionVid;
use rustc::ty::{self, RegionVid};
use syntax::codemap::Span;

use super::{Cause, CauseExt, TrackCauses};
Expand Down Expand Up @@ -263,7 +263,17 @@ impl RegionValues {
if let Some(causes) = &mut self.causes {
let cause = make_cause(causes);
let old_cause = causes.get_mut(&(r, i)).unwrap();
if cause < **old_cause {
// #49998: compare using root cause alone to avoid
// useless traffic from similar outlives chains.

let overwrite = if ty::tls::with(|tcx| {
tcx.sess.opts.debugging_opts.nll_subminimal_causes
}) {
cause.root_cause() < old_cause.root_cause()
} else {
cause < **old_cause
};
if overwrite {
*old_cause = Rc::new(cause);
return true;
}
Expand Down
78 changes: 0 additions & 78 deletions src/test/ui/borrowck/borrowck-closures-two-mut.nll.stderr

This file was deleted.

4 changes: 2 additions & 2 deletions src/test/ui/borrowck/issue-45983.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/issue-45983.rs:17:27
|
LL | give_any(|y| x = Some(y));
Expand All @@ -16,7 +16,7 @@ error[E0594]: cannot assign to immutable item `x`
LL | give_any(|y| x = Some(y));
| ^^^^^^^^^^^ cannot mutate
|
= note: Value not mutable causing this error: `x`
= note: the value which is causing this path not to be mutable is...: `x`

error[E0596]: cannot borrow immutable item `x` as mutable
--> $DIR/issue-45983.rs:17:14
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/issue-7573.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/issue-7573.rs:27:31
|
LL | let mut lines_to_use: Vec<&CrateId> = Vec::new();
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/regions-escape-bound-fn-2.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/regions-escape-bound-fn-2.rs:18:27
|
LL | with_int(|y| x = Some(y));
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/borrowck/regions-escape-bound-fn.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/regions-escape-bound-fn.rs:18:22
|
LL | with_int(|y| x = Some(y));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/regions-escape-unboxed-closure.rs:16:27
|
LL | with_int(&mut |y| x = Some(y));
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/expect-region-supply-region.rs:28:13
|
LL | f = Some(x); //~ ERROR borrowed data cannot be stored outside of its closure
| ^^^^^^^

warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/expect-region-supply-region.rs:38:13
|
LL | f = Some(x); //~ ERROR borrowed data cannot be stored outside of its closure
| ^^^^^^^

warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/expect-region-supply-region.rs:47:33
|
LL | closure_expecting_bound(|x: &'x u32| {
| ^^^^^^^

warning: not reporting region error due to -Znll
warning: not reporting region error due to nll
--> $DIR/expect-region-supply-region.rs:52:13
|
LL | f = Some(x);
Expand Down
11 changes: 9 additions & 2 deletions src/test/ui/did_you_mean/issue-34126.nll.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
error[E0596]: cannot borrow immutable item `self` as mutable
--> $DIR/issue-34126.rs:16:18
|
LL | self.run(&mut self); //~ ERROR cannot borrow
| ^^^^^^^^^ cannot borrow as mutable

error[E0502]: cannot borrow `self` as mutable because it is also borrowed as immutable
--> $DIR/issue-34126.rs:16:18
|
Expand All @@ -8,6 +14,7 @@ LL | self.run(&mut self); //~ ERROR cannot borrow
| immutable borrow occurs here
| borrow later used here

error: aborting due to previous error
error: aborting due to 2 previous errors

For more information about this error, try `rustc --explain E0502`.
Some errors occurred: E0502, E0596.
For more information about an error, try `rustc --explain E0502`.
2 changes: 1 addition & 1 deletion src/test/ui/did_you_mean/issue-35937.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0596]: cannot borrow immutable item `f.v` as mutable
LL | f.v.push("cat".to_string()); //~ ERROR cannot borrow
| ^^^ cannot borrow as mutable
|
= note: Value not mutable causing this error: `f`
= note: the value which is causing this path not to be mutable is...: `f`

error[E0384]: cannot assign twice to immutable variable `s.x`
--> $DIR/issue-35937.rs:26:5
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/did_you_mean/issue-38147-1.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0596]: cannot borrow immutable item `*self.s` as mutable
LL | self.s.push('x'); //~ ERROR cannot borrow data mutably
| ^^^^^^ cannot borrow as mutable
|
= note: Value not mutable causing this error: `*self`
= note: the value which is causing this path not to be mutable is...: `*self`

error: aborting due to previous error

Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/did_you_mean/issue-38147-4.nll.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ error[E0596]: cannot borrow immutable item `*f.s` as mutable
LL | f.s.push('x'); //~ ERROR cannot borrow data mutably
| ^^^ cannot borrow as mutable
|
= note: Value not mutable causing this error: `*f`
= note: the value which is causing this path not to be mutable is...: `*f`

error: aborting due to previous error

Expand Down
Loading

0 comments on commit 8a28d94

Please sign in to comment.