Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check that diagnostics happen in the line that they are annotated for #2167

Merged
merged 1 commit into from
Jun 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 2 additions & 9 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand All @@ -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};

Expand Down Expand Up @@ -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();

Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/intrinsics/assume.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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`
}
}
2 changes: 2 additions & 0 deletions tests/compile-fail/no_main.stderr
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
error: miri can only run programs that have a main function

error: aborting due to previous error

Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
45 changes: 45 additions & 0 deletions ui_test/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ui_test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
6 changes: 3 additions & 3 deletions ui_test/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
36 changes: 34 additions & 2 deletions ui_test/src/comments.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ use std::path::Path;

use regex::Regex;

use crate::rustc_stderr::Level;

#[cfg(test)]
mod tests;

Expand Down Expand Up @@ -33,7 +35,11 @@ pub(crate) struct Comments {
pub(crate) struct ErrorMatch {
pub matched: String,
pub revision: Option<String>,
pub level: Option<Level>,
/// 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 {
Expand All @@ -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<revision>[^\]]+)\])?~[|^]*\s*(ERROR|HELP|WARN)?:?(?P<text>.*)")
Regex::new(r"//(\[(?P<revision>[^\]]+)\])?~(?P<offset>\||[\^]+)?\s*(?P<level>ERROR|HELP|WARN|NOTE)?:?(?P<text>.*)")
.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,
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion ui_test/src/comments/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ fn main() {
";
let comments = Comments::parse(Path::new("<dummy>"), 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,
Expand Down
Loading