From 4b100a1b58767a4fd5884355072e27a42892babc Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Mon, 30 May 2022 12:17:36 +0000 Subject: [PATCH] Check that diagnostics happen in the line that they are annotated for --- Cargo.lock | 45 +++ src/bin/miri.rs | 11 +- tests/compile-fail/intrinsics/assume.rs | 2 +- tests/compile-fail/no_main.stderr | 2 + .../sync/libc_pthread_cond_double_destroy.rs | 2 +- .../libc_pthread_condattr_double_destroy.rs | 2 +- .../sync/libc_pthread_mutex_double_destroy.rs | 2 +- .../libc_pthread_mutexattr_double_destroy.rs | 2 +- .../libc_pthread_rwlock_double_destroy.rs | 2 +- ui_test/Cargo.lock | 45 +++ ui_test/Cargo.toml | 2 + ui_test/README.md | 6 +- ui_test/src/comments.rs | 36 +- ui_test/src/comments/tests.rs | 2 +- ui_test/src/lib.rs | 193 ++++++++--- ui_test/src/rustc_stderr.rs | 152 +++++++++ ui_test/src/tests.rs | 308 ++++++++++++++++-- 17 files changed, 737 insertions(+), 77 deletions(-) create mode 100644 ui_test/src/rustc_stderr.rs diff --git a/Cargo.lock b/Cargo.lock index 5377f9420b7..29b7e0bec26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -193,6 +193,12 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "itoa" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "112c678d4050afce233f4f2852bb2eb519230b3cf12f33585275537d7e41578d" + [[package]] name = "lazy_static" version = "1.4.0" @@ -446,6 +452,12 @@ dependencies = [ "semver", ] +[[package]] +name = "ryu" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3f6f92acf49d1b98f7a81226834412ada05458b7364277387724a237f062695" + [[package]] name = "scopeguard" version = "1.1.0" @@ -458,6 +470,37 @@ version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8cb243bdfdb5936c8dc3c45762a19d12ab4550cdc753bc247637d4ec35a040fd" +[[package]] +name = "serde" +version = "1.0.137" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61ea8d54c77f8315140a05f4c7237403bf38b72704d031543aa1d16abbf517d1" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.137" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f26faba0c3959972377d3b2d306ee9f71faee9714294e41bb777f83f88578be" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.81" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b7ce2b32a1aed03c558dc61a5cd328f15aff2dbc17daad8fb8af04d2100e15c" +dependencies = [ + "itoa", + "ryu", + "serde", +] + [[package]] name = "shell-escape" version = "0.1.5" @@ -500,6 +543,8 @@ dependencies = [ "pretty_assertions", "regex", "rustc_version", + "serde", + "serde_json", ] [[package]] diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 9c4cd0684c6..2cf5bc644db 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -19,7 +19,6 @@ use log::debug; use rustc_data_structures::sync::Lrc; use rustc_driver::Compilation; -use rustc_errors::emitter::{ColorConfig, HumanReadableErrorType}; use rustc_hir::{self as hir, def_id::LOCAL_CRATE, Node}; use rustc_interface::interface::Config; use rustc_middle::{ @@ -28,7 +27,7 @@ use rustc_middle::{ }, ty::{query::ExternProviders, TyCtxt}, }; -use rustc_session::{config::ErrorOutputType, search_paths::PathKind, CtfeBacktrace}; +use rustc_session::{search_paths::PathKind, CtfeBacktrace}; use miri::{BacktraceStyle, ProvenanceMode}; @@ -64,13 +63,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls { let (entry_def_id, entry_type) = if let Some(entry_def) = tcx.entry_fn(()) { entry_def } else { - let output_ty = ErrorOutputType::HumanReadable(HumanReadableErrorType::Default( - ColorConfig::Auto, - )); - rustc_session::early_error( - output_ty, - "miri can only run programs that have a main function", - ); + tcx.sess.fatal("miri can only run programs that have a main function"); }; let mut config = self.miri_config.clone(); diff --git a/tests/compile-fail/intrinsics/assume.rs b/tests/compile-fail/intrinsics/assume.rs index 7b18cab7980..ad193d84991 100644 --- a/tests/compile-fail/intrinsics/assume.rs +++ b/tests/compile-fail/intrinsics/assume.rs @@ -5,6 +5,6 @@ fn main() { unsafe { std::intrinsics::assume(x < 10); std::intrinsics::assume(x > 1); - std::intrinsics::assume(x > 42); //~ `assume` intrinsic called with `false` + std::intrinsics::assume(x > 42); //~ ERROR `assume` intrinsic called with `false` } } diff --git a/tests/compile-fail/no_main.stderr b/tests/compile-fail/no_main.stderr index 52591a8d6da..88bdfb4e387 100644 --- a/tests/compile-fail/no_main.stderr +++ b/tests/compile-fail/no_main.stderr @@ -1,2 +1,4 @@ error: miri can only run programs that have a main function +error: aborting due to previous error + diff --git a/tests/compile-fail/sync/libc_pthread_cond_double_destroy.rs b/tests/compile-fail/sync/libc_pthread_cond_double_destroy.rs index c376618357d..18be75b308c 100644 --- a/tests/compile-fail/sync/libc_pthread_cond_double_destroy.rs +++ b/tests/compile-fail/sync/libc_pthread_cond_double_destroy.rs @@ -17,6 +17,6 @@ fn main() { libc::pthread_cond_destroy(cond.as_mut_ptr()); libc::pthread_cond_destroy(cond.as_mut_ptr()); - //~^ Undefined Behavior: using uninitialized data, but this operation requires initialized memory + //~^ ERROR Undefined Behavior: using uninitialized data, but this operation requires initialized memory } } diff --git a/tests/compile-fail/sync/libc_pthread_condattr_double_destroy.rs b/tests/compile-fail/sync/libc_pthread_condattr_double_destroy.rs index 44af51a3e87..1543a5841ad 100644 --- a/tests/compile-fail/sync/libc_pthread_condattr_double_destroy.rs +++ b/tests/compile-fail/sync/libc_pthread_condattr_double_destroy.rs @@ -14,6 +14,6 @@ fn main() { libc::pthread_condattr_destroy(attr.as_mut_ptr()); libc::pthread_condattr_destroy(attr.as_mut_ptr()); - //~^ Undefined Behavior: using uninitialized data, but this operation requires initialized memory + //~^ ERROR Undefined Behavior: using uninitialized data, but this operation requires initialized memory } } diff --git a/tests/compile-fail/sync/libc_pthread_mutex_double_destroy.rs b/tests/compile-fail/sync/libc_pthread_mutex_double_destroy.rs index 08abc0ca12c..3710810cd2c 100644 --- a/tests/compile-fail/sync/libc_pthread_mutex_double_destroy.rs +++ b/tests/compile-fail/sync/libc_pthread_mutex_double_destroy.rs @@ -18,6 +18,6 @@ fn main() { libc::pthread_mutex_destroy(mutex.as_mut_ptr()); libc::pthread_mutex_destroy(mutex.as_mut_ptr()); - //~^ Undefined Behavior: using uninitialized data, but this operation requires initialized memory + //~^ ERROR Undefined Behavior: using uninitialized data, but this operation requires initialized memory } } diff --git a/tests/compile-fail/sync/libc_pthread_mutexattr_double_destroy.rs b/tests/compile-fail/sync/libc_pthread_mutexattr_double_destroy.rs index 69ca3ad512f..c232780ee2e 100644 --- a/tests/compile-fail/sync/libc_pthread_mutexattr_double_destroy.rs +++ b/tests/compile-fail/sync/libc_pthread_mutexattr_double_destroy.rs @@ -14,6 +14,6 @@ fn main() { libc::pthread_mutexattr_destroy(attr.as_mut_ptr()); libc::pthread_mutexattr_destroy(attr.as_mut_ptr()); - //~^ Undefined Behavior: using uninitialized data, but this operation requires initialized memory + //~^ ERROR Undefined Behavior: using uninitialized data, but this operation requires initialized memory } } diff --git a/tests/compile-fail/sync/libc_pthread_rwlock_double_destroy.rs b/tests/compile-fail/sync/libc_pthread_rwlock_double_destroy.rs index d20c78155fd..055bb1af489 100644 --- a/tests/compile-fail/sync/libc_pthread_rwlock_double_destroy.rs +++ b/tests/compile-fail/sync/libc_pthread_rwlock_double_destroy.rs @@ -11,6 +11,6 @@ fn main() { libc::pthread_rwlock_destroy(&mut lock); libc::pthread_rwlock_destroy(&mut lock); - //~^ Undefined Behavior: using uninitialized data, but this operation requires initialized memory + //~^ ERROR Undefined Behavior: using uninitialized data, but this operation requires initialized memory } } diff --git a/ui_test/Cargo.lock b/ui_test/Cargo.lock index 185af43ac0b..5a4cdb89271 100644 --- a/ui_test/Cargo.lock +++ b/ui_test/Cargo.lock @@ -148,6 +148,12 @@ dependencies = [ "libc", ] +[[package]] +name = "itoa" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "112c678d4050afce233f4f2852bb2eb519230b3cf12f33585275537d7e41578d" + [[package]] name = "lazy_static" version = "1.4.0" @@ -240,6 +246,12 @@ dependencies = [ "semver", ] +[[package]] +name = "ryu" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f3f6f92acf49d1b98f7a81226834412ada05458b7364277387724a237f062695" + [[package]] name = "scopeguard" version = "1.1.0" @@ -252,6 +264,37 @@ version = "1.0.9" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8cb243bdfdb5936c8dc3c45762a19d12ab4550cdc753bc247637d4ec35a040fd" +[[package]] +name = "serde" +version = "1.0.137" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61ea8d54c77f8315140a05f4c7237403bf38b72704d031543aa1d16abbf517d1" +dependencies = [ + "serde_derive", +] + +[[package]] +name = "serde_derive" +version = "1.0.137" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1f26faba0c3959972377d3b2d306ee9f71faee9714294e41bb777f83f88578be" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "serde_json" +version = "1.0.81" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9b7ce2b32a1aed03c558dc61a5cd328f15aff2dbc17daad8fb8af04d2100e15c" +dependencies = [ + "itoa", + "ryu", + "serde", +] + [[package]] name = "syn" version = "1.0.95" @@ -273,6 +316,8 @@ dependencies = [ "pretty_assertions", "regex", "rustc_version", + "serde", + "serde_json", ] [[package]] diff --git a/ui_test/Cargo.toml b/ui_test/Cargo.toml index 66d35fdd222..92c00915cbf 100644 --- a/ui_test/Cargo.toml +++ b/ui_test/Cargo.toml @@ -12,3 +12,5 @@ regex = "1.5.5" pretty_assertions = "1.2.1" crossbeam = "0.8.1" lazy_static = "1.4.0" +serde = { version = "1.0", features = ["derive"] } +serde_json = "1.0" diff --git a/ui_test/README.md b/ui_test/README.md index fdd94a74823..b3c9a3378cf 100644 --- a/ui_test/README.md +++ b/ui_test/README.md @@ -11,9 +11,9 @@ Note that the space after `//`, when it is present, is *not* optional -- it must * `// stderr-per-bitwidth` produces one stderr file per bitwidth, as they may differ significantly sometimes * `// error-pattern: XXX` make sure the stderr output contains `XXX` * `//~ ERROR: XXX` make sure the stderr output contains `XXX` for an error in the line where this comment is written - * NOTE: it is not checked at present that it is actually in the line where the error occurred, or that it is truly an ERROR/WARNING/HELP/NOTE, but you should treat it as such until that becomes true. - * Also supports `HELP` or `WARN` for different kind of message - * if the all caps note is left out, any message is matched + * Also supports `HELP`, `WARN` or `NOTE` for different kind of message + * if one of those levels is specified explicitly, *all* diagnostics of this level or higher need an annotation. If you want to avoid this, just leave out the all caps level note entirely. + * If the all caps note is left out, a message of any level is matched. Leaving it out is not allowed for `ERROR` levels. * This checks the output *before* normalization, so you can check things that get normalized away, but need to be careful not to accidentally have a pattern that differs between platforms. * `// revisions: XXX YYY` runs the test once for each space separated name in the list diff --git a/ui_test/src/comments.rs b/ui_test/src/comments.rs index e6e45de4160..cc9870a63be 100644 --- a/ui_test/src/comments.rs +++ b/ui_test/src/comments.rs @@ -2,6 +2,8 @@ use std::path::Path; use regex::Regex; +use crate::rustc_stderr::Level; + #[cfg(test)] mod tests; @@ -33,7 +35,11 @@ pub(crate) struct Comments { pub(crate) struct ErrorMatch { pub matched: String, pub revision: Option, + pub level: Option, + /// The line where the message was defined, for reporting issues with it (e.g. in case it wasn't found). pub definition_line: usize, + /// The line this pattern is expecting to find a message in. + pub line: usize, } impl Comments { @@ -47,9 +53,13 @@ impl Comments { pub(crate) fn parse(path: &Path, content: &str) -> Self { let mut this = Self::default(); let error_pattern_regex = - Regex::new(r"//(\[(?P[^\]]+)\])?~[|^]*\s*(ERROR|HELP|WARN)?:?(?P.*)") + Regex::new(r"//(\[(?P[^\]]+)\])?~(?P\||[\^]+)?\s*(?PERROR|HELP|WARN|NOTE)?:?(?P.*)") .unwrap(); + + // The line that a `|` will refer to + let mut fallthrough_to = None; for (l, line) in content.lines().enumerate() { + let l = l + 1; // enumerate starts at 0, but line numbers start at 1 if let Some(revisions) = line.strip_prefix("// revisions:") { assert_eq!( this.revisions, @@ -113,7 +123,29 @@ impl Comments { let matched = captures["text"].trim().to_string(); let revision = captures.name("revision").map(|rev| rev.as_str().to_string()); - this.error_matches.push(ErrorMatch { matched, revision, definition_line: l }); + + let level = captures.name("level").map(|rev| rev.as_str().parse().unwrap()); + + let match_line = match captures.name("offset").map(|rev| rev.as_str()) { + Some("|") => fallthrough_to.expect("`//~|` pattern without preceding line"), + Some(pat) => { + debug_assert!(pat.chars().all(|c| c == '^')); + l - pat.len() + } + None => l, + }; + + fallthrough_to = Some(match_line); + + this.error_matches.push(ErrorMatch { + matched, + revision, + level, + definition_line: l, + line: match_line, + }); + } else { + fallthrough_to = None; } } this diff --git a/ui_test/src/comments/tests.rs b/ui_test/src/comments/tests.rs index 0140fdf4a9c..ef966224142 100644 --- a/ui_test/src/comments/tests.rs +++ b/ui_test/src/comments/tests.rs @@ -13,7 +13,7 @@ fn main() { "; let comments = Comments::parse(Path::new(""), s); println!("parsed comments: {:#?}", comments); - assert_eq!(comments.error_matches[0].definition_line, 4); + assert_eq!(comments.error_matches[0].definition_line, 5); assert_eq!(comments.error_matches[0].revision, None); assert_eq!( comments.error_matches[0].matched, diff --git a/ui_test/src/lib.rs b/ui_test/src/lib.rs index 6052efe02e0..4a9cdd386ac 100644 --- a/ui_test/src/lib.rs +++ b/ui_test/src/lib.rs @@ -8,10 +8,12 @@ use colored::*; use comments::ErrorMatch; use crossbeam::queue::SegQueue; use regex::Regex; +use rustc_stderr::{Level, Message}; use crate::comments::Comments; mod comments; +mod rustc_stderr; #[cfg(test)] mod tests; @@ -100,7 +102,8 @@ pub fn run_tests(config: Config) { for revision in comments.revisions.clone().unwrap_or_else(|| vec![String::new()]) { - let (m, errors) = run_test(&path, &config, &target, &revision, &comments); + let (m, errors, stderr) = + run_test(&path, &config, &target, &revision, &comments); // Using a single `eprintln!` to prevent messages from threads from getting intermingled. let mut msg = format!("{} ", path.display()); @@ -113,7 +116,13 @@ pub fn run_tests(config: Config) { succeeded.fetch_add(1, Ordering::Relaxed); } else { eprintln!("{msg}{}", "FAILED".red().bold()); - failures.lock().unwrap().push((path.clone(), m, revision, errors)); + failures.lock().unwrap().push(( + path.clone(), + m, + revision, + errors, + stderr, + )); } } } @@ -128,7 +137,7 @@ pub fn run_tests(config: Config) { let ignored = ignored.load(Ordering::Relaxed); let filtered = filtered.load(Ordering::Relaxed); if !failures.is_empty() { - for (path, miri, revision, errors) in &failures { + for (path, miri, revision, errors, stderr) in &failures { eprintln!(); eprint!("{}", path.display().to_string().underline()); if !revision.is_empty() { @@ -138,32 +147,63 @@ pub fn run_tests(config: Config) { eprintln!(); eprintln!("command: {:?}", miri); eprintln!(); - let mut dump_stderr = None; + // `None` means never dump, as we already dumped it for an `OutputDiffers` + // `Some(false)` means there's no reason to dump, as all errors are independent of the stderr + // `Some(true)` means that there was a pattern in the .rs file that was not found in the output. + let mut dump_stderr = Some(false); for error in errors { match error { Error::ExitStatus(mode, exit_status) => eprintln!("{mode:?} got {exit_status}"), - Error::PatternNotFound { stderr, pattern, definition_line } => { + Error::PatternNotFound { pattern, definition_line } => { eprintln!("`{pattern}` {} in stderr output", "not found".red()); eprintln!( "expected because of pattern here: {}:{definition_line}", path.display().to_string().bold() ); - dump_stderr = Some(stderr.clone()) + dump_stderr = dump_stderr.map(|_| true); + } + Error::NoPatternsFound => { + eprintln!("{}", "no error patterns found in failure test".red()); } - Error::NoPatternsFound => - eprintln!("{}", "no error patterns found in failure test".red()), Error::PatternFoundInPassTest => eprintln!("{}", "error pattern found in success test".red()), Error::OutputDiffers { path, actual, expected } => { - dump_stderr = None; + if path.extension().unwrap() == "stderr" { + dump_stderr = None; + } eprintln!("actual output differed from expected {}", path.display()); eprintln!("{}", pretty_assertions::StrComparison::new(expected, actual)); eprintln!() } + Error::ErrorsWithoutPattern { path: None, msgs } => { + eprintln!( + "There were {} unmatched diagnostics that occurred outside the testfile and had not pattern", + msgs.len(), + ); + for Message { level, message } in msgs { + eprintln!(" {level:?}: {message}") + } + } + Error::ErrorsWithoutPattern { path: Some((path, line)), msgs } => { + eprintln!( + "There were {} unmatched diagnostics at {}:{line}", + msgs.len(), + path.display() + ); + for Message { level, message } in msgs { + eprintln!(" {level:?}: {message}") + } + } + Error::ErrorPatternWithoutErrorAnnotation(path, line) => { + eprintln!( + "Annotation at {}:{line} matched an error diagnostic but did not have `ERROR` before its message", + path.display() + ); + } } eprintln!(); } - if let Some(stderr) = dump_stderr { + if let Some(true) = dump_stderr { eprintln!("actual stderr:"); eprintln!("{}", stderr); eprintln!(); @@ -195,7 +235,6 @@ enum Error { /// Got an invalid exit status for the given mode. ExitStatus(Mode, ExitStatus), PatternNotFound { - stderr: String, pattern: String, definition_line: usize, }, @@ -209,6 +248,11 @@ enum Error { actual: String, expected: String, }, + ErrorsWithoutPattern { + msgs: Vec, + path: Option<(PathBuf, usize)>, + }, + ErrorPatternWithoutErrorAnnotation(PathBuf, usize), } type Errors = Vec; @@ -219,7 +263,7 @@ fn run_test( target: &str, revision: &str, comments: &Comments, -) -> (Command, Errors) { +) -> (Command, Errors, String) { // Run miri let mut miri = Command::new(&config.program); miri.args(config.args.iter()); @@ -227,6 +271,7 @@ fn run_test( if !revision.is_empty() { miri.arg(format!("--cfg={revision}")); } + miri.arg("--error-format=json"); for arg in &comments.compile_flags { miri.arg(arg); } @@ -235,7 +280,7 @@ fn run_test( } let output = miri.output().expect("could not execute miri"); let mut errors = config.mode.ok(output.status); - check_test_result( + let stderr = check_test_result( path, config, target, @@ -245,7 +290,7 @@ fn run_test( &output.stdout, &output.stderr, ); - (miri, errors) + (miri, errors, stderr) } fn check_test_result( @@ -257,11 +302,9 @@ fn check_test_result( errors: &mut Errors, stdout: &[u8], stderr: &[u8], -) { +) -> String { // Always remove annotation comments from stderr. - let annotations = Regex::new(r"\s*//~.*").unwrap(); - let stderr = std::str::from_utf8(stderr).unwrap(); - let stderr = annotations.replace_all(stderr, ""); + let diagnostics = rustc_stderr::process(path, stderr); let stdout = std::str::from_utf8(stdout).unwrap(); // Check output files (if any) let revised = |extension: &str| { @@ -273,7 +316,7 @@ fn check_test_result( }; // Check output files against actual output check_output( - &stderr, + &diagnostics.rendered, path, errors, revised("stderr"), @@ -293,50 +336,126 @@ fn check_test_result( comments, ); // Check error annotations in the source against output - check_annotations(&stderr, errors, config, revision, comments); + check_annotations( + diagnostics.messages, + diagnostics.messages_from_unknown_file_or_line, + path, + errors, + config, + revision, + comments, + ); + diagnostics.rendered } fn check_annotations( - unnormalized_stderr: &str, + mut messages: Vec>, + mut messages_from_unknown_file_or_line: Vec, + path: &Path, errors: &mut Errors, config: &Config, revision: &str, comments: &Comments, ) { - let mut found_annotation = false; if let Some((ref error_pattern, definition_line)) = comments.error_pattern { - if !unnormalized_stderr.contains(error_pattern) { + let mut found = false; + + // first check the diagnostics messages outside of our file. We check this first, so that + // you can mix in-file annotations with // error-pattern annotations, even if there is overlap + // in the messages. + if let Some(i) = messages_from_unknown_file_or_line + .iter() + .position(|msg| msg.message.contains(error_pattern)) + { + messages_from_unknown_file_or_line.remove(i); + found = true; + } + + // if nothing was found, check the ones inside our file. We permit this because some tests may have + // flaky line numbers for their messages. + if !found { + for line in &mut messages { + if let Some(i) = line.iter().position(|msg| msg.message.contains(error_pattern)) { + line.remove(i); + found = true; + break; + } + } + } + + if !found { errors.push(Error::PatternNotFound { - stderr: unnormalized_stderr.to_string(), pattern: error_pattern.to_string(), definition_line, }); } - found_annotation = true; } - for &ErrorMatch { ref matched, revision: ref rev, definition_line } in &comments.error_matches { - // FIXME: check that the error happens on the marked line + // The order on `Level` is such that `Error` is the highest level. + // We will ensure that *all* diagnostics of level at least `lowest_annotation_level` + // are matched. + let mut lowest_annotation_level = Level::Error; + for &ErrorMatch { ref matched, revision: ref rev, definition_line, line, level } in + &comments.error_matches + { if let Some(rev) = rev { if rev != revision { continue; } } - - if !unnormalized_stderr.contains(matched) { - errors.push(Error::PatternNotFound { - stderr: unnormalized_stderr.to_string(), - pattern: matched.to_string(), - definition_line, - }); + if let Some(level) = level { + // If we found a diagnostic with a level annotation, make sure that all + // diagnostics of that level have annotations, even if we don't end up finding a matching diagnostic + // for this pattern. + lowest_annotation_level = std::cmp::min(lowest_annotation_level, level); } - found_annotation = true; + + if let Some(msgs) = messages.get_mut(line) { + let found = msgs.iter().position(|msg| { + msg.message.contains(matched) + // in case there is no level on the annotation, match any level. + && level.map_or(true, |level| { + msg.level == level + }) + }); + if let Some(found) = found { + let msg = msgs.remove(found); + if msg.level == Level::Error && level.is_none() { + errors + .push(Error::ErrorPatternWithoutErrorAnnotation(path.to_path_buf(), line)); + } + continue; + } + } + + errors.push(Error::PatternNotFound { pattern: matched.to_string(), definition_line }); } - match (config.mode, found_annotation) { + + let filter = |msgs: Vec| -> Vec<_> { + msgs.into_iter().filter(|msg| msg.level >= lowest_annotation_level).collect() + }; + + let messages_from_unknown_file_or_line = filter(messages_from_unknown_file_or_line); + if !messages_from_unknown_file_or_line.is_empty() { + errors.push(Error::ErrorsWithoutPattern { + path: None, + msgs: messages_from_unknown_file_or_line, + }); + } + + for (line, msgs) in messages.into_iter().enumerate() { + let msgs = filter(msgs); + if !msgs.is_empty() { + errors + .push(Error::ErrorsWithoutPattern { path: Some((path.to_path_buf(), line)), msgs }); + } + } + + match (config.mode, comments.error_pattern.is_some() || !comments.error_matches.is_empty()) { (Mode::Pass, true) | (Mode::Panic, true) => errors.push(Error::PatternFoundInPassTest), (Mode::Fail, false) => errors.push(Error::NoPatternsFound), _ => {} - }; + } } fn check_output( diff --git a/ui_test/src/rustc_stderr.rs b/ui_test/src/rustc_stderr.rs new file mode 100644 index 00000000000..ac76a78a3cb --- /dev/null +++ b/ui_test/src/rustc_stderr.rs @@ -0,0 +1,152 @@ +use std::{ + fmt::Write, + path::{Path, PathBuf}, +}; + +use regex::Regex; + +#[derive(serde::Deserialize, Debug)] +struct RustcMessage { + rendered: Option, + spans: Vec, + level: String, + message: String, + children: Vec, +} + +#[derive(Copy, Clone, Debug, PartialOrd, Ord, PartialEq, Eq)] +pub(crate) enum Level { + Error = 4, + Warn = 3, + Help = 2, + Note = 1, + /// Only used for "For more information about this error, try `rustc --explain EXXXX`". + FailureNote = 0, +} + +#[derive(Debug)] +pub(crate) struct Message { + pub(crate) level: Level, + pub(crate) message: String, +} + +/// Information about macro expansion. +#[derive(serde::Deserialize, Debug)] +struct Expansion { + span: Span, +} + +#[derive(serde::Deserialize, Debug)] +struct Span { + line_start: usize, + file_name: PathBuf, + expansion: Option>, +} + +impl std::str::FromStr for Level { + type Err = String; + fn from_str(s: &str) -> Result { + match s { + "ERROR" | "error" => Ok(Self::Error), + "WARN" | "warning" => Ok(Self::Warn), + "HELP" | "help" => Ok(Self::Help), + "NOTE" | "note" => Ok(Self::Note), + "failure-note" => Ok(Self::FailureNote), + _ => Err(format!("unknown level `{s}`")), + } + } +} + +#[derive(Debug)] +pub(crate) struct Diagnostics { + /// Rendered and concatenated version of all diagnostics. + /// This is equivalent to non-json diagnostics. + pub rendered: String, + /// Per line, a list of messages for that line. + pub messages: Vec>, + /// Messages not on any line (usually because they are from libstd) + pub messages_from_unknown_file_or_line: Vec, +} + +impl RustcMessage { + fn line(&self, file: &Path) -> Option { + self.spans.iter().find_map(|span| span.line(file)) + } + + /// Put the message and its children into the line-indexed list. + fn insert_recursive( + self, + file: &Path, + messages: &mut Vec>, + messages_from_unknown_file_or_line: &mut Vec, + line: Option, + ) { + let line = self.line(file).or(line); + let msg = Message { level: self.level.parse().unwrap(), message: self.message }; + if let Some(line) = line { + if messages.len() <= line { + messages.resize_with(line + 1, Vec::new); + } + messages[line].push(msg); + // All other messages go into the general bin, unless they are specifically of the + // "aborting due to X previous errors" variety, as we never want to match those. They + // only count the number of errors and provide no useful information about the tests. + } else if !(msg.message.starts_with("aborting due to") + && msg.message.contains("previous error")) + { + messages_from_unknown_file_or_line.push(msg); + } + for child in self.children { + child.insert_recursive(file, messages, messages_from_unknown_file_or_line, line) + } + } +} + +impl Span { + /// Returns a line number *in the given file*, if possible. + fn line(&self, file: &Path) -> Option { + if self.file_name == file { + Some(self.line_start) + } else { + self.expansion.as_ref()?.span.line(file) + } + } +} + +pub(crate) fn filter_annotations_from_rendered(rendered: &str) -> std::borrow::Cow<'_, str> { + let annotations = Regex::new(r"\s*//~.*").unwrap(); + annotations.replace_all(&rendered, "") +} + +pub(crate) fn process(file: &Path, stderr: &[u8]) -> Diagnostics { + let stderr = std::str::from_utf8(&stderr).unwrap(); + let mut rendered = String::new(); + let mut messages = vec![]; + let mut messages_from_unknown_file_or_line = vec![]; + for line in stderr.lines() { + if line.starts_with("{") { + match serde_json::from_str::(line) { + Ok(msg) => { + write!( + rendered, + "{}", + filter_annotations_from_rendered(msg.rendered.as_ref().unwrap()) + ) + .unwrap(); + msg.insert_recursive( + file, + &mut messages, + &mut messages_from_unknown_file_or_line, + None, + ); + } + Err(err) => + panic!("failed to parse rustc JSON output at line: {}\nerr:{}", line, err), + } + } else { + // FIXME: do we want to throw interpreter stderr into a separate file? + writeln!(rendered, "{}", line).unwrap(); + } + } + Diagnostics { rendered, messages, messages_from_unknown_file_or_line } +} diff --git a/ui_test/src/tests.rs b/ui_test/src/tests.rs index d0ef1195d88..7e08a68be7b 100644 --- a/ui_test/src/tests.rs +++ b/ui_test/src/tests.rs @@ -1,5 +1,8 @@ use std::path::{Path, PathBuf}; +use crate::rustc_stderr::Level; +use crate::rustc_stderr::Message; + use super::*; fn config() -> Config { @@ -29,25 +32,292 @@ fn main() { let comments = Comments::parse(&path, s); let mut errors = vec![]; let config = config(); - // Crucially, the intended error string *does* appear in this output, as a quote of the comment itself. - let stderr = br" -error: Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated) - --> tests/compile-fail/validity/dangling_ref1.rs:6:29 - | -LL | let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address $HEX is unallocated) - | ^^^^^^^^^^^^^^^^^^^^^^^ type validation failed: encountered a dangling reference (address 0x10 is unallocated) - | - = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior - = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information - - = note: inside `main` at tests/compile-fail/validity/dangling_ref1.rs:6:29 -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace -error: aborting due to previous error - "; - check_test_result(&path, &config, "", "", &comments, &mut errors, /*stdout*/ br"", stderr); - // The "OutputDiffers" is because we cannot open the .rs file + let messages = vec![ + vec![], vec![], vec![], vec![], vec![], + vec![ + Message { + message:"Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + } + ] + ]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); match &errors[..] { - [Error::OutputDiffers { .. }, Error::PatternNotFound { .. }] => {} - _ => panic!("not the expected error: {:#?}", errors), + [ + Error::PatternNotFound { definition_line: 5, .. }, + Error::ErrorsWithoutPattern { path: Some((_, 5)), .. }, + ] => {} + _ => panic!("{:#?}", errors), + } +} + +#[test] +fn find_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address 0x10 is unallocated) +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + { + let messages = vec![vec![], vec![], vec![], vec![], vec![], vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + } + ] + ]; + let mut errors = vec![]; + check_annotations( + messages, + vec![], + Path::new("moobar"), + &mut errors, + &config, + "", + &comments, + ); + match &errors[..] { + [] => {} + _ => panic!("{:#?}", errors), + } + } + + // only difference to above is a wrong line number + { + let messages = vec![vec![], vec![], vec![], vec![], vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + } + ] + ]; + let mut errors = vec![]; + check_annotations( + messages, + vec![], + Path::new("moobar"), + &mut errors, + &config, + "", + &comments, + ); + match &errors[..] { + [ + Error::PatternNotFound { definition_line: 5, .. }, + Error::ErrorsWithoutPattern { path: Some((_, 4)), .. }, + ] => {} + _ => panic!("not the expected error: {:#?}", errors), + } + } + + // only difference to first is a wrong level + { + let messages = vec![ + vec![], vec![], vec![], vec![], vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Note, + } + ] + ]; + let mut errors = vec![]; + check_annotations( + messages, + vec![], + Path::new("moobar"), + &mut errors, + &config, + "", + &comments, + ); + match &errors[..] { + // Note no `ErrorsWithoutPattern`, because there are no `//~NOTE` in the test file, so we ignore them + [Error::PatternNotFound { definition_line: 5, .. }] => {} + _ => panic!("not the expected error: {:#?}", errors), + } + } +} + +#[test] +fn duplicate_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address 0x10 is unallocated) + //~^ ERROR encountered a dangling reference (address 0x10 is unallocated) +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + let messages = vec![ + vec![], vec![], vec![], vec![], vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + } + ] + ]; + let mut errors = vec![]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); + match &errors[..] { + [Error::PatternNotFound { definition_line: 6, .. }] => {} + _ => panic!("{:#?}", errors), + } +} + +#[test] +fn missing_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address 0x10 is unallocated) +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + let messages = vec![ + vec![], vec![], vec![], vec![], vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + }, + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + } + ] + ]; + let mut errors = vec![]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); + match &errors[..] { + [Error::ErrorsWithoutPattern { path: Some((_, 5)), .. }] => {} + _ => panic!("{:#?}", errors), + } +} + +#[test] +fn missing_warn_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address 0x10 is unallocated) + //~^ WARN cake +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + let messages= vec![ + vec![], + vec![], + vec![], + vec![], + vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + }, + Message { + message: "kaboom".to_string(), + level: Level::Warn, + }, + Message { + message: "cake".to_string(), + level: Level::Warn, + }, + ], + ]; + let mut errors = vec![]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); + match &errors[..] { + [Error::ErrorsWithoutPattern { path: Some((_, 5)), msgs, .. }] => + match &msgs[..] { + [Message { message, level: Level::Warn }] if message == "kaboom" => {} + _ => panic!("{:#?}", msgs), + }, + _ => panic!("{:#?}", errors), + } +} + +#[test] +fn missing_implicit_warn_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ ERROR encountered a dangling reference (address 0x10 is unallocated) + //~^ cake +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + let messages = vec![ + vec![], + vec![], + vec![], + vec![], + vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + }, + Message { + message: "kaboom".to_string(), + level: Level::Warn, + }, + Message { + message: "cake".to_string(), + level: Level::Warn, + }, + ], + ]; + let mut errors = vec![]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); + match &errors[..] { + [] => {} + _ => panic!("{:#?}", errors), + } +} + +#[test] +fn implicit_err_pattern() { + let s = r" +use std::mem; + +fn main() { + let _x: &i32 = unsafe { mem::transmute(16usize) }; //~ encountered a dangling reference (address 0x10 is unallocated) +} + "; + let comments = Comments::parse(Path::new(""), s); + let config = config(); + let messages = vec![ + vec![], + vec![], + vec![], + vec![], + vec![], + vec![ + Message { + message: "Undefined Behavior: type validation failed: encountered a dangling reference (address 0x10 is unallocated)".to_string(), + level: Level::Error, + }, + ], + ]; + let mut errors = vec![]; + check_annotations(messages, vec![], Path::new("moobar"), &mut errors, &config, "", &comments); + match &errors[..] { + [Error::ErrorPatternWithoutErrorAnnotation(_, 5)] => {} + _ => panic!("{:#?}", errors), } }