refactor ignore- and only- to use decisions

Author: pietroalbini

src/tools/compiletest/src/header.rs

@@ -660,13 +660,6 @@ impl Config {
         }
     }
 
-    fn has_cfg_prefix(&self, line: &str, prefix: &str) -> bool {
-        // returns whether this line contains this prefix or not. For prefix
-        // "ignore", returns true if line says "ignore-x86_64", "ignore-arch",
-        // "ignore-android" etc.
-        line.starts_with(prefix) && line.as_bytes().get(prefix.len()) == Some(&b'-')
-    }
-
     fn parse_name_directive(&self, line: &str, directive: &str) -> bool {
         // Ensure the directive is a whole word. Do not match "ignore-x86" when
         // the line says "ignore-x86_64".
@@ -878,7 +871,7 @@ pub fn make_test_description<R: Read>(
                         // The ignore reason must be a &'static str, so we have to leak memory to
                         // create it. This is fine, as the header is parsed only at the start of
                         // compiletest so it won't grow indefinitely.
-                        ignore_message = Some(Box::leak(Box::<str>::from(reason)));
+                        ignore_message = Some(&*Box::leak(Box::<str>::from(reason)));
                     }
                     IgnoreDecision::Error { message } => {
                         eprintln!("error: {}: {message}", path.display());
@@ -889,46 +882,8 @@ pub fn make_test_description<R: Read>(
             };
         }
 
-        {
-            let parsed = parse_cfg_name_directive(config, ln, "ignore");
-            ignore = match parsed.outcome {
-                MatchOutcome::Match => {
-                    let reason = parsed.pretty_reason.unwrap();
-                    // The ignore reason must be a &'static str, so we have to leak memory to
-                    // create it. This is fine, as the header is parsed only at the start of
-                    // compiletest so it won't grow indefinitely.
-                    ignore_message = Some(Box::leak(Box::<str>::from(match parsed.comment {
-                        Some(comment) => format!("ignored {reason} ({comment})"),
-                        None => format!("ignored {reason}"),
-                    })) as &str);
-                    true
-                }
-                MatchOutcome::NoMatch => ignore,
-                MatchOutcome::External => ignore,
-                MatchOutcome::Invalid => panic!("invalid line in {}: {ln}", path.display()),
-            };
-        }
-
-        if config.has_cfg_prefix(ln, "only") {
-            let parsed = parse_cfg_name_directive(config, ln, "only");
-            ignore = match parsed.outcome {
-                MatchOutcome::Match => ignore,
-                MatchOutcome::NoMatch => {
-                    let reason = parsed.pretty_reason.unwrap();
-                    // The ignore reason must be a &'static str, so we have to leak memory to
-                    // create it. This is fine, as the header is parsed only at the start of
-                    // compiletest so it won't grow indefinitely.
-                    ignore_message = Some(Box::leak(Box::<str>::from(match parsed.comment {
-                        Some(comment) => format!("only executed {reason} ({comment})"),
-                        None => format!("only executed {reason}"),
-                    })) as &str);
-                    true
-                }
-                MatchOutcome::External => ignore,
-                MatchOutcome::Invalid => panic!("invalid line in {}: {ln}", path.display()),
-            };
-        }
-
+        decision!(cfg::handle_ignore(config, ln));
+        decision!(cfg::handle_only(config, ln));
         decision!(needs::handle_needs(&needs_cache, config, ln));
         decision!(ignore_llvm(config, ln));
         decision!(ignore_cdb(config, ln));

src/tools/compiletest/src/header/cfg.rs

@@ -1,8 +1,43 @@
 use crate::common::{CompareMode, Config, Debugger};
+use crate::header::IgnoreDecision;
 use std::collections::HashSet;
 
 const EXTRA_ARCHS: &[&str] = &["spirv"];
 
+pub(super) fn handle_ignore(config: &Config, line: &str) -> IgnoreDecision {
+    let parsed = parse_cfg_name_directive(config, line, "ignore");
+    match parsed.outcome {
+        MatchOutcome::NoMatch => IgnoreDecision::Continue,
+        MatchOutcome::Match => IgnoreDecision::Ignore {
+            reason: match parsed.comment {
+                Some(comment) => format!("ignored {} ({comment})", parsed.pretty_reason.unwrap()),
+                None => format!("ignored {}", parsed.pretty_reason.unwrap()),
+            },
+        },
+        MatchOutcome::Invalid => IgnoreDecision::Error { message: format!("invalid line: {line}") },
+        MatchOutcome::External => IgnoreDecision::Continue,
+        MatchOutcome::NotADirective => IgnoreDecision::Continue,
+    }
+}
+
+pub(super) fn handle_only(config: &Config, line: &str) -> IgnoreDecision {
+    let parsed = parse_cfg_name_directive(config, line, "only");
+    match parsed.outcome {
+        MatchOutcome::Match => IgnoreDecision::Continue,
+        MatchOutcome::NoMatch => IgnoreDecision::Ignore {
+            reason: match parsed.comment {
+                Some(comment) => {
+                    format!("only executed {} ({comment})", parsed.pretty_reason.unwrap())
+                }
+                None => format!("only executed {}", parsed.pretty_reason.unwrap()),
+            },
+        },
+        MatchOutcome::Invalid => IgnoreDecision::Error { message: format!("invalid line: {line}") },
+        MatchOutcome::External => IgnoreDecision::Continue,
+        MatchOutcome::NotADirective => IgnoreDecision::Continue,
+    }
+}
+
 /// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
 /// or `normalize-stderr-32bit`.
 pub(super) fn parse_cfg_name_directive<'a>(
@@ -11,10 +46,10 @@ pub(super) fn parse_cfg_name_directive<'a>(
     prefix: &str,
 ) -> ParsedNameDirective<'a> {
     if !line.as_bytes().starts_with(prefix.as_bytes()) {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
     if line.as_bytes().get(prefix.len()) != Some(&b'-') {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
     let line = &line[prefix.len() + 1..];
 
@@ -24,7 +59,7 @@ pub(super) fn parse_cfg_name_directive<'a>(
     // Some of the matchers might be "" depending on what the target information is. To avoid
     // problems we outright reject empty directives.
     if name == "" {
-        return ParsedNameDirective::invalid();
+        return ParsedNameDirective::not_a_directive();
     }
 
     let mut outcome = MatchOutcome::Invalid;
@@ -218,8 +253,13 @@ pub(super) struct ParsedNameDirective<'a> {
 }
 
 impl ParsedNameDirective<'_> {
-    fn invalid() -> Self {
-        Self { name: None, pretty_reason: None, comment: None, outcome: MatchOutcome::NoMatch }
+    fn not_a_directive() -> Self {
+        Self {
+            name: None,
+            pretty_reason: None,
+            comment: None,
+            outcome: MatchOutcome::NotADirective,
+        }
     }
 }
 
@@ -233,6 +273,8 @@ pub(super) enum MatchOutcome {
     Invalid,
     /// The directive is handled by other parts of our tooling.
     External,
+    /// The line is not actually a directive.
+    NotADirective,
 }
 
 trait CustomContains {