From 55c07678cd80772be719416427f2986dd3524022 Mon Sep 17 00:00:00 2001 From: Pietro Albini Date: Fri, 14 Apr 2023 11:57:22 +0200 Subject: [PATCH] refactor ignore- and only- to use decisions --- src/tools/compiletest/src/header.rs | 51 ++---------------------- src/tools/compiletest/src/header/cfg.rs | 52 ++++++++++++++++++++++--- 2 files changed, 50 insertions(+), 53 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index ac659c89405..8d2fd881d26 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -660,13 +660,6 @@ fn parse_custom_normalization(&self, mut line: &str, prefix: &str) -> Option<(St } } - 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 @@ macro_rules! decision { // 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::::from(reason))); + ignore_message = Some(&*Box::leak(Box::::from(reason))); } IgnoreDecision::Error { message } => { eprintln!("error: {}: {message}", path.display()); @@ -889,46 +882,8 @@ macro_rules! decision { }; } - { - 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::::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::::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)); diff --git a/src/tools/compiletest/src/header/cfg.rs b/src/tools/compiletest/src/header/cfg.rs index 3b9333dfe7a..a9694d4d52c 100644 --- a/src/tools/compiletest/src/header/cfg.rs +++ b/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 {