Auto merge of rust-lang#2167 - rust-lang:gesundheit, r=RalfJung

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

fixes rust-lang#2131

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();
 

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`
     }
 }

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
+

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
     }
 }

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
     }
 }

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
     }
 }

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
     }
 }

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
     }
 }

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"

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

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<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 {
@@ -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,
@@ -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

ui_test/src/comments/tests.rs

@@ -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,