Auto merge of #41968 - kennytm:fix-unreadable-json-test-output-36516, r=nikomatsakis

Introduce 'run-pass' header to 'ui' tests in compiletest. Fix issue #36516.

<del>`ui-run` test is a combination of `ui` test and `run-pass` test. It is used to test lint output.</del>

Added support of `// run-pass` header to `ui` tests.

The compiler message of each test must match the corresponding `*.stderr` file like the traditional `ui` tests. Additionally, the compiled output must be executed successfully like the `run-pass` test.

12 `run-pass`/`run-pass-fulldeps` tests are moved to `ui`/`ui-fulldeps` plus the headers. After this move, no `run-pass`/`run-pass-fulldeps` tests should rely on the compiler's JSON message. This allows us to stop passing `--error-format json` in run-pass tests, thus fixing #36516.

src/test/codegen/fastcall-inreg.rs

@@ -37,7 +37,7 @@
 // ignore-tce
 // ignore-thumb
 // ignore-thumbeb
-// ignore-x86_64 no-ignore-x86
+// ignore-x86_64
 // ignore-xcore
 // ignore-nvptx
 // ignore-nvptx64

src/test/run-pass/i128-ffi.rs

@@ -15,7 +15,8 @@
 // ignore-windows
 
 // Ignore 32 bit targets:
-// ignore-x86, ignore-arm
+// ignore-x86
+// ignore-arm
 
 // ignore-emscripten
 

src/test/ui-fulldeps/deprecated-derive.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 #![feature(rustc_private)]
 #![allow(dead_code)]
 

src/test/ui-fulldeps/deprecated-derive.stderr

@@ -0,0 +1,6 @@
+warning: derive(Encodable) is deprecated in favor of derive(RustcEncodable)
+  --> $DIR/deprecated-derive.rs:18:10
+   |
+18 | #[derive(Encodable)]
+   |          ^^^^^^^^^
+

src/test/ui-fulldeps/lint-group-plugin.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
 // aux-build:lint_group_plugin_test.rs
 // ignore-stage1
 #![feature(plugin)]

src/test/ui-fulldeps/lint-group-plugin.stderr

@@ -0,0 +1,16 @@
+warning: item is named 'lintme'
+  --> $DIR/lint-group-plugin.rs:18:1
+   |
+18 | fn lintme() { } //~ WARNING item is named 'lintme'
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(test_lint)] on by default
+
+warning: item is named 'pleaselintme'
+  --> $DIR/lint-group-plugin.rs:19:1
+   |
+19 | fn pleaselintme() { } //~ WARNING item is named 'pleaselintme'
+   | ^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(please_lint)] on by default
+

src/test/ui-fulldeps/lint-plugin-cmdline-allow.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
 // aux-build:lint_plugin_test.rs
 // ignore-stage1
 // compile-flags: -A test-lint

src/test/ui-fulldeps/lint-plugin-cmdline-allow.stderr

@@ -0,0 +1,8 @@
+warning: function is never used: `lintme`
+  --> $DIR/lint-plugin-cmdline-allow.rs:19:1
+   |
+19 | fn lintme() { }
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(dead_code)] on by default
+

src/test/ui-fulldeps/lint-plugin-cmdline-load.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
 // aux-build:lint_plugin_test.rs
 // ignore-stage1
 // compile-flags: -Z extra-plugins=lint_plugin_test

src/test/ui-fulldeps/lint-plugin-cmdline-load.stderr

@@ -0,0 +1,8 @@
+warning: item is named 'lintme'
+  --> $DIR/lint-plugin-cmdline-load.rs:18:1
+   |
+18 | fn lintme() { } //~ WARNING item is named 'lintme'
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(test_lint)] on by default
+

src/test/ui-fulldeps/lint-plugin.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
 // aux-build:lint_plugin_test.rs
 // ignore-stage1
 #![feature(plugin)]

src/test/ui-fulldeps/lint-plugin.stderr

@@ -0,0 +1,8 @@
+warning: item is named 'lintme'
+  --> $DIR/lint-plugin.rs:18:1
+   |
+18 | fn lintme() { } //~ WARNING item is named 'lintme'
+   | ^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(test_lint)] on by default
+

src/test/ui/deprecated-macro_escape-inner.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 mod foo {
     #![macro_escape] //~ WARNING macro_escape is a deprecated synonym for macro_use
     //~^ HELP consider an outer attribute

src/test/ui/deprecated-macro_escape-inner.stderr

@@ -0,0 +1,8 @@
+warning: macro_escape is a deprecated synonym for macro_use
+  --> $DIR/deprecated-macro_escape-inner.rs:14:5
+   |
+14 |     #![macro_escape] //~ WARNING macro_escape is a deprecated synonym for macro_use
+   |     ^^^^^^^^^^^^^^^^
+   |
+   = help: consider an outer attribute, #[macro_use] mod ...
+

src/test/ui/deprecated-macro_escape.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 #[macro_escape] //~ WARNING macro_escape is a deprecated synonym for macro_use
 mod foo {
 }

src/test/ui/deprecated-macro_escape.stderr

@@ -0,0 +1,6 @@
+warning: macro_escape is a deprecated synonym for macro_use
+  --> $DIR/deprecated-macro_escape.rs:13:1
+   |
+13 | #[macro_escape] //~ WARNING macro_escape is a deprecated synonym for macro_use
+   | ^^^^^^^^^^^^^^^
+

src/test/ui/deriving-meta-empty-trait-list.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 #![allow(dead_code)]
 
 #[derive]   //~ WARNING empty trait list in `derive`

src/test/ui/deriving-meta-empty-trait-list.stderr

@@ -0,0 +1,12 @@
+warning: empty trait list in `derive`
+  --> $DIR/deriving-meta-empty-trait-list.rs:15:1
+   |
+15 | #[derive]   //~ WARNING empty trait list in `derive`
+   | ^^^^^^^^^
+
+warning: empty trait list in `derive`
+  --> $DIR/deriving-meta-empty-trait-list.rs:18:1
+   |
+18 | #[derive()] //~ WARNING empty trait list in `derive`
+   | ^^^^^^^^^^^
+

src/test/ui/enum-size-variance.rs

@@ -7,7 +7,13 @@
 // <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
-//
+
+// run-pass
+// ignore-x86
+// ignore-arm
+// ignore-emscripten
+// ^ ignore 32-bit targets, as the error message is target-dependent. see PR #41968.
+
 #![warn(variant_size_differences)]
 #![allow(dead_code)]
 

src/test/ui/enum-size-variance.stderr

@@ -0,0 +1,12 @@
+warning: enum variant is more than three times larger (32 bytes) than the next largest
+  --> $DIR/enum-size-variance.rs:32:5
+   |
+32 |     L(isize, isize, isize, isize), //~ WARNING three times larger
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+note: lint level defined here
+  --> $DIR/enum-size-variance.rs:17:9
+   |
+17 | #![warn(variant_size_differences)]
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^
+

src/test/ui/issue-19100.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 #![allow(non_snake_case)]
 #![allow(dead_code)]
 #![allow(unused_variables)]

src/test/ui/issue-19100.stderr

@@ -0,0 +1,16 @@
+warning[E0170]: pattern binding `Bar` is named the same as one of the variants of the type `Foo`
+  --> $DIR/issue-19100.rs:27:1
+   |
+27 | Bar if true
+   | ^^^
+   |
+   = help: if you meant to match on a variant, consider making the path in the pattern qualified: `Foo::Bar`
+
+warning[E0170]: pattern binding `Baz` is named the same as one of the variants of the type `Foo`
+  --> $DIR/issue-19100.rs:32:1
+   |
+32 | Baz if false
+   | ^^^
+   |
+   = help: if you meant to match on a variant, consider making the path in the pattern qualified: `Foo::Baz`
+

src/test/ui/path-lookahead.rs

@@ -8,6 +8,8 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
+
 // Parser test for #37765
 
 fn with_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `with_parens`

src/test/ui/path-lookahead.stderr

@@ -0,0 +1,28 @@
+warning: unnecessary parentheses around `return` value
+  --> $DIR/path-lookahead.rs:16:10
+   |
+16 |   return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
+   |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: #[warn(unused_parens)] on by default
+
+warning: function is never used: `with_parens`
+  --> $DIR/path-lookahead.rs:15:1
+   |
+15 | / fn with_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `with_parens`
+16 | |   return (<T as ToString>::to_string(&arg)); //~WARN unnecessary parentheses around `return` value
+17 | | }
+   | |_^
+   |
+   = note: #[warn(dead_code)] on by default
+
+warning: function is never used: `no_parens`
+  --> $DIR/path-lookahead.rs:19:1
+   |
+19 | / fn no_parens<T: ToString>(arg: T) -> String { //~WARN function is never used: `no_parens`
+20 | |   return <T as ToString>::to_string(&arg);
+21 | | }
+   | |_^
+   |
+   = note: #[warn(dead_code)] on by default
+

src/test/ui/test-should-panic-attr.rs

@@ -8,6 +8,7 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
+// run-pass
 // compile-flags: --test
 
 #[test]

src/test/ui/test-should-panic-attr.stderr

@@ -0,0 +1,40 @@
+warning: attribute must be of the form: `#[should_panic]` or `#[should_panic(expected = "error message")]`
+  --> $DIR/test-should-panic-attr.rs:15:1
+   |
+15 | #[should_panic = "foo"]
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: Errors in this attribute were erroneously allowed and will become a hard error in a future release.
+
+warning: argument must be of the form: `expected = "error message"`
+  --> $DIR/test-should-panic-attr.rs:22:1
+   |
+22 | #[should_panic(expected)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: Errors in this attribute were erroneously allowed and will become a hard error in a future release.
+
+warning: argument must be of the form: `expected = "error message"`
+  --> $DIR/test-should-panic-attr.rs:29:1
+   |
+29 | #[should_panic(expect)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: Errors in this attribute were erroneously allowed and will become a hard error in a future release.
+
+warning: argument must be of the form: `expected = "error message"`
+  --> $DIR/test-should-panic-attr.rs:36:1
+   |
+36 | #[should_panic(expected(foo, bar))]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: Errors in this attribute were erroneously allowed and will become a hard error in a future release.
+
+warning: argument must be of the form: `expected = "error message"`
+  --> $DIR/test-should-panic-attr.rs:43:1
+   |
+43 | #[should_panic(expected = "foo", bar)]
+   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = note: Errors in this attribute were erroneously allowed and will become a hard error in a future release.
+

src/tools/compiletest/src/header.rs

@@ -80,13 +80,12 @@ impl EarlyProps {
                 return false;
             }
 
-            if !line.contains("ignore-gdb-version") &&
-               config.parse_name_directive(line, "ignore-gdb") {
+            if config.parse_name_directive(line, "ignore-gdb") {
                 return true;
             }
 
             if let Some(actual_version) = config.gdb_version {
-                if line.contains("min-gdb-version") {
+                if line.starts_with("min-gdb-version") {
                     let (start_ver, end_ver) = extract_gdb_version_range(line);
 
                     if start_ver != end_ver {
@@ -95,7 +94,7 @@ impl EarlyProps {
                     // Ignore if actual version is smaller the minimum required
                     // version
                     actual_version < start_ver
-                } else if line.contains("ignore-gdb-version") {
+                } else if line.starts_with("ignore-gdb-version") {
                     let (min_version, max_version) = extract_gdb_version_range(line);
 
                     if max_version < min_version {
@@ -119,20 +118,21 @@ impl EarlyProps {
         fn extract_gdb_version_range(line: &str) -> (u32, u32) {
             const ERROR_MESSAGE: &'static str = "Malformed GDB version directive";
 
-            let range_components = line.split(' ')
-                                       .flat_map(|word| word.split('-'))
-                                       .filter(|word| word.len() > 0)
-                                       .skip_while(|word| extract_gdb_version(word).is_none())
-                                       .collect::<Vec<&str>>();
+            let range_components = line.split(&[' ', '-'][..])
+                                       .filter(|word| !word.is_empty())
+                                       .map(extract_gdb_version)
+                                       .skip_while(Option::is_none)
+                                       .take(3) // 3 or more = invalid, so take at most 3.
+                                       .collect::<Vec<Option<u32>>>();
 
             match range_components.len() {
                 1 => {
-                    let v = extract_gdb_version(range_components[0]).unwrap();
+                    let v = range_components[0].unwrap();
                     (v, v)
                 }
                 2 => {
-                    let v_min = extract_gdb_version(range_components[0]).unwrap();
-                    let v_max = extract_gdb_version(range_components[1]).expect(ERROR_MESSAGE);
+                    let v_min = range_components[0].unwrap();
+                    let v_max = range_components[1].expect(ERROR_MESSAGE);
                     (v_min, v_max)
                 }
                 _ => panic!(ERROR_MESSAGE),
@@ -149,10 +149,10 @@ impl EarlyProps {
             }
 
             if let Some(ref actual_version) = config.lldb_version {
-                if line.contains("min-lldb-version") {
-                    let min_version = line.trim()
-                        .split(' ')
-                        .last()
+                if line.starts_with("min-lldb-version") {
+                    let min_version = line.trim_right()
+                        .rsplit(' ')
+                        .next()
                         .expect("Malformed lldb version directive");
                     // Ignore if actual version is smaller the minimum required
                     // version
@@ -167,10 +167,10 @@ impl EarlyProps {
 
         fn ignore_llvm(config: &Config, line: &str) -> bool {
             if let Some(ref actual_version) = config.llvm_version {
-                if line.contains("min-llvm-version") {
-                    let min_version = line.trim()
-                        .split(' ')
-                        .last()
+                if line.starts_with("min-llvm-version") {
+                    let min_version = line.trim_right()
+                        .rsplit(' ')
+                        .next()
                         .expect("Malformed llvm version directive");
                     // Ignore if actual version is smaller the minimum required
                     // version
@@ -233,6 +233,9 @@ pub struct TestProps {
     pub must_compile_successfully: bool,
     // rustdoc will test the output of the `--test` option
     pub check_test_line_numbers_match: bool,
+    // The test must be compiled and run successfully. Only used in UI tests for
+    // now.
+    pub run_pass: bool,
 }
 
 impl TestProps {
@@ -258,6 +261,7 @@ impl TestProps {
             incremental_dir: None,
             must_compile_successfully: false,
             check_test_line_numbers_match: false,
+            run_pass: false,
         }
     }
 
@@ -368,6 +372,10 @@ impl TestProps {
             if !self.check_test_line_numbers_match {
                 self.check_test_line_numbers_match = config.parse_check_test_line_numbers_match(ln);
             }
+
+            if !self.run_pass {
+                self.run_pass = config.parse_run_pass(ln);
+            }
         });
 
         for key in vec!["RUST_TEST_NOCAPTURE", "RUST_TEST_THREADS"] {
@@ -405,14 +413,14 @@ fn iter_header(testfile: &Path, cfg: Option<&str>, it: &mut FnMut(&str)) {
                     None => false,
                 };
                 if matches {
-                    it(&ln[close_brace + 1..]);
+                    it(ln[(close_brace + 1) ..].trim_left());
                 }
             } else {
                 panic!("malformed condition directive: expected `//[foo]`, found `{}`",
                        ln)
             }
         } else if ln.starts_with("//") {
-            it(&ln[2..]);
+            it(ln[2..].trim_left());
         }
     }
     return;
@@ -485,6 +493,10 @@ impl Config {
         self.parse_name_directive(line, "check-test-line-numbers-match")
     }
 
+    fn parse_run_pass(&self, line: &str) -> bool {
+        self.parse_name_directive(line, "run-pass")
+    }
+
     fn parse_env(&self, line: &str, name: &str) -> Option<(String, String)> {
         self.parse_name_value_directive(line, name).map(|nv| {
             // nv is either FOO or FOO=BAR
@@ -516,15 +528,18 @@ impl Config {
     }
 
     fn parse_name_directive(&self, line: &str, directive: &str) -> bool {
-        // This 'no-' rule is a quick hack to allow pretty-expanded and
-        // no-pretty-expanded to coexist
-        line.contains(directive) && !line.contains(&("no-".to_owned() + directive))
+        // Ensure the directive is a whole word. Do not match "ignore-x86" when
+        // the line says "ignore-x86_64".
+        line.starts_with(directive) && match line.as_bytes().get(directive.len()) {
+            None | Some(&b' ') | Some(&b':') => true,
+            _ => false
+        }
     }
 
     pub fn parse_name_value_directive(&self, line: &str, directive: &str) -> Option<String> {
-        let keycolon = format!("{}:", directive);
-        if let Some(colon) = line.find(&keycolon) {
-            let value = line[(colon + keycolon.len())..line.len()].to_owned();
+        let colon = directive.len();
+        if line.starts_with(directive) && line.as_bytes().get(colon) == Some(&b':') {
+            let value = line[(colon + 1) ..].to_owned();
             debug!("{}: {}", directive, value);
             Some(expand_variables(value, self))
         } else {

src/tools/compiletest/src/runtest.rs

@@ -214,10 +214,10 @@ impl<'test> TestCx<'test> {
             self.fatal_proc_rec("compilation failed!", &proc_res);
         }
 
+        // FIXME(#41968): Move this check to tidy?
         let expected_errors = errors::load_errors(&self.testpaths.file, self.revision);
-        if !expected_errors.is_empty() {
-            self.check_expected_errors(expected_errors, &proc_res);
-        }
+        assert!(expected_errors.is_empty(),
+                "run-pass tests with expected warnings should be moved to ui/");
 
         let proc_res = self.exec_compiled_test();
 
@@ -1394,7 +1394,6 @@ actual:\n\
         match self.config.mode {
             CompileFail |
             ParseFail |
-            RunPass |
             Incremental => {
                 // If we are extracting and matching errors in the new
                 // fashion, then you want JSON mode. Old-skool error
@@ -1422,6 +1421,7 @@ actual:\n\
 
                 args.push(dir_opt);
             }
+            RunPass |
             RunFail |
             RunPassValgrind |
             Pretty |
@@ -2254,6 +2254,14 @@ actual:\n\
             self.fatal_proc_rec(&format!("{} errors occurred comparing output.", errors),
                                 &proc_res);
         }
+
+        if self.props.run_pass {
+            let proc_res = self.exec_compiled_test();
+
+            if !proc_res.status.success() {
+                self.fatal_proc_rec("test run failed!", &proc_res);
+            }
+        }
     }
 
     fn run_mir_opt_test(&self) {