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.

Sign up for GitHub

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

Teach compiletest to parse arbitrary cfg options #87396

Closed
wants to merge 5 commits into from
Closed

Teach compiletest to parse arbitrary cfg options #87396

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cbb3b03
Teach compiletest to parse arbitrary cfg options
inquisitivecrystal Jul 23, 2021
bc315aa
Evaulate cfg props for the real target in compiletest
inquisitivecrystal Dec 17, 2021
e600e46
Change implementation per remaining review
inquisitivecrystal Dec 19, 2021
50656c8
Fix test
inquisitivecrystal Jan 3, 2022
ae92083
Add tests
inquisitivecrystal Jan 4, 2022
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
159 changes: 121 additions & 38 deletions src/tools/compiletest/src/header.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ use std::fs::File;
use std::io::prelude::*;
use std::io::BufReader;
use std::path::{Path, PathBuf};
use std::process::Command;

use tracing::*;
use tracing::{self, *};

use crate::common::{CompareMode, Config, Debugger, FailMode, Mode, PanicStrategy, PassMode};
use crate::util;
Expand All @@ -14,9 +15,12 @@ use crate::{extract_cdb_version, extract_gdb_version};
#[cfg(test)]
mod tests;

/// The result of parse_cfg_name_directive.
/// Whether a given property holds true of this test instance. Usually,
/// a property is something about the target (e.g. whether the target is Windows),
/// but it could also be something more general (e.g. whether this test is being
/// cross-compiled.)
#[derive(Clone, Copy, PartialEq, Debug)]
enum ParsedNameDirective {
enum EvaluatedProp {
/// No match.
NoMatch,
/// Match.
Expand Down Expand Up @@ -226,6 +230,10 @@ impl TestProps {
if !testfile.is_dir() {
let file = File::open(testfile).unwrap();

let mut target = config.target.clone();
let mut per_prop_normalize_stdout: Vec<(String, (String, String))> = Vec::new();
let mut per_prop_normalize_stderr: Vec<(String, (String, String))> = Vec::new();

iter_header(testfile, file, &mut |revision, ln| {
if revision.is_some() && revision != cfg {
return;
Expand Down Expand Up @@ -338,11 +346,16 @@ impl TestProps {
self.ignore_pass = config.parse_ignore_pass(ln);
}

if let Some(target_override) = config.parse_target_override(ln) {
target = target_override;
}

if let Some(rule) = config.parse_custom_normalization(ln, "normalize-stdout") {
self.normalize_stdout.push(rule);
per_prop_normalize_stdout.push(rule);
}

if let Some(rule) = config.parse_custom_normalization(ln, "normalize-stderr") {
self.normalize_stderr.push(rule);
per_prop_normalize_stderr.push(rule);
}

if let Some(code) = config.parse_failure_status(ln) {
Expand Down Expand Up @@ -370,6 +383,17 @@ impl TestProps {
self.incremental = config.parse_incremental(ln);
}
});
for (prop, normalization) in per_prop_normalize_stdout {
if config.evaluate_prop_for_target(&prop, &target) == EvaluatedProp::Match {
self.normalize_stdout.push(normalization);
}
}

for (prop, normalization) in per_prop_normalize_stderr {
if config.evaluate_prop_for_target(&prop, &target) == EvaluatedProp::Match {
self.normalize_stderr.push(normalization);
}
}
}

if self.failure_status == -1 {
Expand Down Expand Up @@ -648,14 +672,15 @@ impl Config {
}
}

fn parse_custom_normalization(&self, mut line: &str, prefix: &str) -> Option<(String, String)> {
if self.parse_cfg_name_directive(line, prefix) == ParsedNameDirective::Match {
let from = parse_normalization_string(&mut line)?;
let to = parse_normalization_string(&mut line)?;
Some((from, to))
} else {
None
}
fn parse_custom_normalization(
&self,
mut line: &str,
prefix: &str,
) -> Option<(String, (String, String))> {
let prop = self.parse_prop_directive(line, prefix)?;
let from = parse_normalization_string(&mut line)?;
let to = parse_normalization_string(&mut line)?;
Some((prop, (from, to)))
}

fn parse_needs_matching_clang(&self, line: &str) -> bool {
Expand All @@ -668,27 +693,34 @@ impl Config {

/// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
/// or `normalize-stderr-32bit`.
fn parse_cfg_name_directive(&self, line: &str, prefix: &str) -> ParsedNameDirective {
fn parse_prop_directive(&self, line: &str, prefix: &str) -> Option<String> {
if !line.as_bytes().starts_with(prefix.as_bytes()) {
return ParsedNameDirective::NoMatch;
return None;
}
if line.as_bytes().get(prefix.len()) != Some(&b'-') {
return ParsedNameDirective::NoMatch;
return None;
}

let name = line[prefix.len() + 1..].split(&[':', ' '][..]).next().unwrap();
let config = &line[&prefix.len() + 1..].to_string();
Some(config.to_string())
}

fn evaluate_prop_for_target(&self, prop: &str, target: &str) -> EvaluatedProp {
let mut iter = prop.split(&[':', ' '][..]);
let name = iter.next().unwrap();
let maybe_value = iter.find(|s| !s.is_empty());

let is_match = name == "test" ||
self.target == name || // triple
util::matches_os(&self.target, name) || // target
util::matches_env(&self.target, name) || // env
self.target.ends_with(name) || // target and env
name == util::get_arch(&self.target) || // architecture
name == util::get_pointer_width(&self.target) || // pointer width
target == name || // triple
util::matches_os(target, name) || // target
util::matches_env(target, name) || // env
target.ends_with(name) || // target and env
name == util::get_arch(target) || // architecture
name == util::get_pointer_width(target) || // pointer width
name == self.stage_id.split('-').next().unwrap() || // stage
name == self.channel || // channel
(self.target != self.host && name == "cross-compile") ||
(name == "endian-big" && util::is_big_endian(&self.target)) ||
(target != self.host && name == "cross-compile") ||
(name == "endian-big" && util::is_big_endian(target)) ||
(self.remote_test_client.is_some() && name == "remote") ||
match self.compare_mode {
Some(CompareMode::Nll) => name == "compare-mode-nll",
Expand All @@ -704,12 +736,31 @@ impl Config {
Some(Debugger::Gdb) => name == "gdb",
Some(Debugger::Lldb) => name == "lldb",
None => false,
};
} || name == "cfg" && self.cfg_matches_for_target(maybe_value.unwrap(), target);

if is_match { EvaluatedProp::Match } else { EvaluatedProp::NoMatch }
}

fn cfg_matches_for_target(&self, value: &str, target: &str) -> bool {
let whitespace_or_double_quote = |c: char| c.is_whitespace() || c == '"';

let value = value.replace(whitespace_or_double_quote, "");

if is_match { ParsedNameDirective::Match } else { ParsedNameDirective::NoMatch }
let cfg_data = Command::new(&self.rustc_path)
.args(&["--target", &target])
.args(&["--print", "cfg"])
.output()
.unwrap()
.stdout;
let cfg_data = String::from_utf8(cfg_data).unwrap();
let cfg_data = cfg_data.replace('"', "");
Copy link
Contributor Author

@inquisitivecrystal inquisitivecrystal Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cfg_data could be cached somewhere around this point. Doing so could presumably result in some small performance saving for the case where target == self.target, which is likely to come up with the most frequency. I'm not sure whether this is worthwhile, or if so, how to go about it.

As an amusing anecdote, I actually fiddled around with a version where all of the targets had their information calculated in advance and it was stored in Config. I figured that computing all of that would only take a few extra seconds. It took me a few days to figure out why doing so was causing a one third to one half regression in testing times. It turns out that Config gets cloned a fair number of times, and by the end was taking enough memory that it was vastly increasing the duration of forking the threads to execute the tests. At least, that's what it was as far as I can tell. In any case, not doing that again!


let matches_value = |line: &str| line == value || line.split('=').next().unwrap() == value;

cfg_data.lines().any(matches_value)
}

fn has_cfg_prefix(&self, line: &str, prefix: &str) -> bool {
fn has_prop_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.
Expand All @@ -734,6 +785,21 @@ impl Config {
}
}

pub fn parse_target_override(&self, line: &str) -> Option<String> {
let flags = self.parse_name_value_directive(line, "compile-flags")?;
let (_, tail) = flags.split_once("--target")?;
if tail.starts_with(|c: char| c.is_whitespace() || c == '=') {
let tail = &tail[1..];
let target = match tail.split_once(|c: char| c.is_whitespace()) {
Some((target, _)) => target,
None => &tail,
};
Some(target.to_string())
} else {
None
}
}

pub fn find_rust_src_root(&self) -> Option<PathBuf> {
let mut path = self.src_base.clone();
let path_postfix = Path::new("src/etc/lldb_batchmode.py");
Expand Down Expand Up @@ -866,6 +932,10 @@ pub fn make_test_description<R: Read>(
let mut ignore = false;
let mut should_fail = false;

let mut target = config.target.clone();
let mut ignored_props: Vec<String> = Vec::new();
let mut only_test_on_props: Vec<String> = Vec::new();

let rustc_has_profiler_support = env::var_os("RUSTC_PROFILER_SUPPORT").is_some();
let rustc_has_sanitizer_support = env::var_os("RUSTC_SANITIZER_SUPPORT").is_some();
let has_asm_support = util::has_asm_support(&config.target);
Expand All @@ -874,7 +944,6 @@ pub fn make_test_description<R: Read>(
let has_msan = util::MSAN_SUPPORTED_TARGETS.contains(&&*config.target);
let has_tsan = util::TSAN_SUPPORTED_TARGETS.contains(&&*config.target);
let has_hwasan = util::HWASAN_SUPPORTED_TARGETS.contains(&&*config.target);
// for `-Z gcc-ld=lld`
let has_rust_lld = config
.compile_lib_path
.join("rustlib")
Expand All @@ -887,15 +956,16 @@ pub fn make_test_description<R: Read>(
if revision.is_some() && revision != cfg {
return;
}
ignore = match config.parse_cfg_name_directive(ln, "ignore") {
ParsedNameDirective::Match => true,
ParsedNameDirective::NoMatch => ignore,
};
if config.has_cfg_prefix(ln, "only") {
ignore = match config.parse_cfg_name_directive(ln, "only") {
ParsedNameDirective::Match => ignore,
ParsedNameDirective::NoMatch => true,
};
if let Some(target_override) = config.parse_target_override(ln) {
target = target_override;
}
if let Some(prop) = config.parse_prop_directive(ln, "ignore") {
ignored_props.push(prop);
}
if config.has_prop_prefix(ln, "only") {
if let Some(prop) = config.parse_prop_directive(ln, "only") {
only_test_on_props.push(prop);
}
}
ignore |= ignore_llvm(config, ln);
ignore |=
Expand All @@ -920,6 +990,19 @@ pub fn make_test_description<R: Read>(
should_fail |= config.parse_name_directive(ln, "should-fail");
});

for prop in ignored_props {
ignore = match config.evaluate_prop_for_target(&prop, &target) {
EvaluatedProp::Match => true,
EvaluatedProp::NoMatch => ignore,
};
}
for prop in only_test_on_props {
ignore = match config.evaluate_prop_for_target(&prop, &target) {
EvaluatedProp::Match => ignore,
EvaluatedProp::NoMatch => true,
};
}

// The `should-fail` annotation doesn't apply to pretty tests,
// since we run the pretty printer across all tests by default.
// If desired, we could add a `should-fail-pretty` annotation.
Expand Down