Merge pull request #33 from oli-obk/revise_revisions

Revise revisions

Author: oli-obk

README.md

@@ -32,7 +32,9 @@ their command specifies, or the test will fail without even being run.
 * `//@error-pattern: XXX` make sure the stderr output contains `XXX`
 * `//@revisions: XXX YYY` runs the test once for each space separated name in the list
     * emits one stderr file per revision
-    * `//~` comments can be restricted to specific revisions by adding the revision name before the `~` in square brackets: `//[XXX]~`
+    * `//~` comments can be restricted to specific revisions by adding the revision name after the `~` in square brackets: `//~[XXX]`
+    * `//@` comments can be restricted to specific revisions by adding the revision name after the `@` in square brackets: `//@[XXX]`
+        * Note that you cannot add revisions to the `revisions` command.
 * `//@compile-flags: XXX` appends `XXX` to the command line arguments passed to the rustc driver
     * you can specify this multiple times, and all the flags will accumulate
 * `//@rustc-env: XXX=YYY` sets the env var `XXX` to `YYY` for the rustc driver execution.

src/lib.rs

@@ -15,7 +15,7 @@ use regex::bytes::Regex;
 use rustc_stderr::{Level, Message};
 use std::collections::VecDeque;
 use std::ffi::OsString;
-use std::fmt::{Display, Write as _};
+use std::fmt::Display;
 use std::io::Write as _;
 use std::num::NonZeroUsize;
 use std::path::{Path, PathBuf};
@@ -488,28 +488,22 @@ fn parse_and_test_file(path: PathBuf, config: &Config) -> Vec<TestRun> {
             }]
         }
     };
-    // Ignore file if only/ignore rules do (not) apply
-    if !test_file_conditions(&comments, config) {
-        return vec![TestRun {
-            result: TestResult::Ignored,
-            path,
-            revision: "".into(),
-        }];
-    }
     // Run the test for all revisions
     comments
         .revisions
         .clone()
         .unwrap_or_else(|| vec![String::new()])
         .into_iter()
         .map(|revision| {
-            let (command, errors, stderr) = run_test(&path, config, &revision, &comments);
-
-            // Using a single `eprintln!` to prevent messages from threads from getting intermingled.
-            let mut msg = format!("{}", path.display());
-            if !revision.is_empty() {
-                write!(msg, " (revision `{revision}`) ").unwrap();
+            // Ignore file if only/ignore rules do (not) apply
+            if !test_file_conditions(&comments, config, &revision) {
+                return TestRun {
+                    result: TestResult::Ignored,
+                    path: path.clone(),
+                    revision,
+                };
             }
+            let (command, errors, stderr) = run_test(&path, config, &revision, &comments);
             let result = if errors.is_empty() {
                 TestResult::Ok
             } else {
@@ -569,11 +563,19 @@ fn build_command(path: &Path, config: &Config, revision: &str, comments: &Commen
     if !revision.is_empty() {
         cmd.arg(format!("--cfg={revision}"));
     }
-    for arg in &comments.compile_flags {
+    for arg in comments
+        .for_revision(revision)
+        .flat_map(|r| r.compile_flags.iter())
+    {
         cmd.arg(arg);
     }
     cmd.args(config.trailing_args.iter());
-    cmd.envs(comments.env_vars.iter().map(|(k, v)| (k, v)));
+    cmd.envs(
+        comments
+            .for_revision(revision)
+            .flat_map(|r| r.env_vars.iter())
+            .map(|(k, v)| (k, v)),
+    );
 
     cmd
 }
@@ -627,6 +629,7 @@ fn check_test_result(
         &config.stderr_filters,
         config,
         comments,
+        revision,
     );
     check_output(
         stdout,
@@ -636,6 +639,7 @@ fn check_test_result(
         &config.stdout_filters,
         config,
         comments,
+        revision,
     );
     // Check error annotations in the source against output
     check_annotations(
@@ -659,7 +663,17 @@ fn check_annotations(
     revision: &str,
     comments: &Comments,
 ) {
-    if let Some((ref error_pattern, definition_line)) = comments.error_pattern {
+    let error_pattern = comments.find_one_for_revision(
+        revision,
+        |r| r.error_pattern.as_ref(),
+        |(_, line)| {
+            errors.push(Error::InvalidComment {
+                msg: "same revision defines pattern twice".into(),
+                line: *line,
+            })
+        },
+    );
+    if let Some((error_pattern, definition_line)) = error_pattern {
         // 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.
@@ -671,7 +685,7 @@ fn check_annotations(
         } else {
             errors.push(Error::PatternNotFound {
                 pattern: error_pattern.clone(),
-                definition_line,
+                definition_line: *definition_line,
             });
         }
     }
@@ -680,20 +694,17 @@ fn check_annotations(
     // We will ensure that *all* diagnostics of level at least `lowest_annotation_level`
     // are matched.
     let mut lowest_annotation_level = Level::Error;
+    let mut seen_error_match = false;
     for &ErrorMatch {
         ref pattern,
-        revision: ref rev,
         definition_line,
         line,
         level,
-    } in &comments.error_matches
+    } in comments
+        .for_revision(revision)
+        .flat_map(|r| r.error_matches.iter())
     {
-        if let Some(rev) = rev {
-            if rev != revision {
-                continue;
-            }
-        }
-
+        seen_error_match = true;
         // 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.
@@ -715,18 +726,21 @@ fn check_annotations(
         });
     }
 
-    let filter = |msgs: Vec<Message>| -> Vec<_> {
-        msgs.into_iter()
-            .filter(|msg| {
-                msg.level
-                    >= comments
-                        .require_annotations_for_level
-                        .unwrap_or(lowest_annotation_level)
+    let filter = |mut msgs: Vec<Message>, errors: &mut Vec<_>| -> Vec<_> {
+        let error = |_| {
+            errors.push(Error::InvalidComment {
+                msg: "`require_annotations_for_level` specified twice for same revision".into(),
+                line: 0,
             })
-            .collect()
+        };
+        let required_annotation_level = comments
+            .find_one_for_revision(revision, |r| r.require_annotations_for_level, error)
+            .unwrap_or(lowest_annotation_level);
+        msgs.retain(|msg| msg.level >= required_annotation_level);
+        msgs
     };
 
-    let messages_from_unknown_file_or_line = filter(messages_from_unknown_file_or_line);
+    let messages_from_unknown_file_or_line = filter(messages_from_unknown_file_or_line, errors);
     if !messages_from_unknown_file_or_line.is_empty() {
         errors.push(Error::ErrorsWithoutPattern {
             path: None,
@@ -735,7 +749,7 @@ fn check_annotations(
     }
 
     for (line, msgs) in messages.into_iter().enumerate() {
-        let msgs = filter(msgs);
+        let msgs = filter(msgs, errors);
         if !msgs.is_empty() {
             errors.push(Error::ErrorsWithoutPattern {
                 path: Some((path.to_path_buf(), line)),
@@ -744,10 +758,7 @@ fn check_annotations(
         }
     }
 
-    match (
-        config.mode,
-        comments.error_pattern.is_some() || !comments.error_matches.is_empty(),
-    ) {
+    match (config.mode, error_pattern.is_some() || seen_error_match) {
         (Mode::Pass, true) | (Mode::Panic, true) => errors.push(Error::PatternFoundInPassTest),
         (
             Mode::Fail {
@@ -767,10 +778,11 @@ fn check_output(
     filters: &Filter,
     config: &Config,
     comments: &Comments,
+    revision: &str,
 ) {
     let target = config.target.as_ref().unwrap();
-    let output = normalize(path, output, filters, comments);
-    let path = output_path(path, comments, kind, target);
+    let output = normalize(path, output, filters, comments, revision);
+    let path = output_path(path, comments, kind, target, revision);
     match config.output_conflict_handling {
         OutputConflictHandling::Bless => {
             if output.is_empty() {
@@ -793,8 +805,17 @@ fn check_output(
     }
 }
 
-fn output_path(path: &Path, comments: &Comments, kind: String, target: &str) -> PathBuf {
-    if comments.stderr_per_bitwidth {
+fn output_path(
+    path: &Path,
+    comments: &Comments,
+    kind: String,
+    target: &str,
+    revision: &str,
+) -> PathBuf {
+    if comments
+        .for_revision(revision)
+        .any(|r| r.stderr_per_bitwidth)
+    {
         return path.with_extension(format!("{}bit.{kind}", get_pointer_width(target)));
     }
     path.with_extension(kind)
@@ -810,11 +831,18 @@ fn test_condition(condition: &Condition, config: &Config) -> bool {
 }
 
 /// Returns whether according to the in-file conditions, this file should be run.
-fn test_file_conditions(comments: &Comments, config: &Config) -> bool {
-    if comments.ignore.iter().any(|c| test_condition(c, config)) {
+fn test_file_conditions(comments: &Comments, config: &Config, revision: &str) -> bool {
+    if comments
+        .for_revision(revision)
+        .flat_map(|r| r.ignore.iter())
+        .any(|c| test_condition(c, config))
+    {
         return false;
     }
-    comments.only.iter().all(|c| test_condition(c, config))
+    comments
+        .for_revision(revision)
+        .flat_map(|r| r.only.iter())
+        .all(|c| test_condition(c, config))
 }
 
 // Taken 1:1 from compiletest-rs
@@ -830,7 +858,13 @@ fn get_pointer_width(triple: &str) -> u8 {
     }
 }
 
-fn normalize(path: &Path, text: &[u8], filters: &Filter, comments: &Comments) -> Vec<u8> {
+fn normalize(
+    path: &Path,
+    text: &[u8],
+    filters: &Filter,
+    comments: &Comments,
+    revision: &str,
+) -> Vec<u8> {
     // Useless paths
     let mut text = text.replace(&path.parent().unwrap().display().to_string(), "$DIR");
     if let Some(lib_path) = option_env!("RUSTC_LIB_PATH") {
@@ -841,7 +875,10 @@ fn normalize(path: &Path, text: &[u8], filters: &Filter, comments: &Comments) ->
         text = regex.replace_all(&text, *replacement).into_owned();
     }
 
-    for (from, to) in &comments.normalize_stderr {
+    for (from, to) in comments
+        .for_revision(revision)
+        .flat_map(|r| r.normalize_stderr.iter())
+    {
         text = from.replace_all(&text, to).into_owned();
     }
     text