Change implementation per remaining review

Author: inquisitivecrystal

src/tools/compiletest/src/header.rs

@@ -4,10 +4,9 @@ use std::fs::File;
 use std::io::prelude::*;
 use std::io::BufReader;
 use std::path::{Path, PathBuf};
+use std::process::Command;
 
-use lazy_static::lazy_static;
-use regex::Regex;
-use tracing::*;
+use tracing::{self, *};
 
 use crate::common::{CompareMode, Config, Debugger, FailMode, Mode, PanicStrategy, PassMode};
 use crate::util;
@@ -707,35 +706,9 @@ impl Config {
     }
 
     fn evaluate_prop_for_target(&self, prop: &str, target: &str) -> EvaluatedProp {
-        // This matches optional whitespace, followed by a group containing a series of word
-        // characters (including '_' and '-'), followed optionally by a sequence consisting
-        // of a colon, optional whitespace, and another group containing word characters.
-        //
-        // Matches in full:
-        //   cfg-target-has-atomic: 128
-        //   cfg-target-has-atomic:128
-        //   cfg-target-has-atomic
-        //
-        // Matches up to the first space (exclusive):
-        //   ignore-test - This test really shouldn't ever be run
-        //
-        // Matches up to the second space (exclusive):
-        //   cfg-target-has-atomic: 128 - this requires fancy newfangled atomics
-        //
-        // Matches up to the second colon (exclusive)
-        //   cfg-target-has-atomic:128: I put an extra colon here to confuse other programmers!
-        //
-        // Does not match:
-        //   (a line consisting solely of whitespace)
-        //   &*#$ cfg-target-has-atomic
-        //
-        lazy_static! {
-            static ref CFG_REGEX: Regex = Regex::new(r"^\s*([\w-]+)(?::\s*([\w-]+))?").unwrap();
-        }
-
-        let captures = CFG_REGEX.captures(&prop).unwrap();
-        let name = captures.get(1).unwrap().as_str();
-        let maybe_value = captures.get(2).map(|v| v.as_str().trim());
+        let mut iter = prop.split(&[':', ' '][..]);
+        let name = iter.next().unwrap();
+        let maybe_value = iter.find(|s| !s.is_empty());
 
         let is_match = name == "test" ||
             target == name ||                                   // triple
@@ -763,18 +736,30 @@ impl Config {
                 Some(Debugger::Gdb) => name == "gdb",
                 Some(Debugger::Lldb) => name == "lldb",
                 None => false,
-            } ||
-            match name.strip_prefix("cfg-") {
-                Some(rustc_cfg_name) => {
-                    let cfg_data = util::fetch_cfg_from_rustc_for_target(&self.rustc_path, target);
-                    util::cfg_has(&cfg_data, rustc_cfg_name, maybe_value)
-                },
-                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, "");
+
+        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('"', "");
+
+        let matches_value = |line: &str| line == value || line.split('=').next().unwrap() == value;
+
+        cfg_data.lines().any(matches_value)
+    }
+
     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",

src/tools/compiletest/src/main.rs

@@ -201,7 +201,6 @@ pub fn parse_config(args: Vec<String>) -> Config {
     let llvm_version =
         matches.opt_str("llvm-version").as_deref().and_then(header::extract_llvm_version);
 
-    let rustc_path = opt_path(matches, "rustc-path");
     let src_base = opt_path(matches, "src-base");
     let run_ignored = matches.opt_present("ignored");
     let mode = matches.opt_str("mode").unwrap().parse().expect("invalid mode");
@@ -215,12 +214,11 @@ pub fn parse_config(args: Vec<String>) -> Config {
         // Avoid spawning an external command when we know tidy won't be used.
         false
     };
-
     Config {
         bless: matches.opt_present("bless"),
         compile_lib_path: make_absolute(opt_path(matches, "compile-lib-path")),
         run_lib_path: make_absolute(opt_path(matches, "run-lib-path")),
-        rustc_path: rustc_path,
+        rustc_path: opt_path(matches, "rustc-path"),
         rustdoc_path: matches.opt_str("rustdoc-path").map(PathBuf::from),
         rust_demangler_path: matches.opt_str("rust-demangler-path").map(PathBuf::from),
         lldb_python: matches.opt_str("lldb-python").unwrap(),

src/tools/compiletest/src/util.rs

@@ -1,10 +1,7 @@
 use crate::common::Config;
-
-use std::collections::HashMap;
 use std::env;
 use std::ffi::OsStr;
 use std::path::PathBuf;
-use std::process::Command;
 
 use tracing::*;
 
@@ -218,64 +215,6 @@ pub fn logv(config: &Config, s: String) {
     }
 }
 
-pub fn fetch_cfg_from_rustc_for_target<P: AsRef<OsStr>>(
-    rustc_path: &P,
-    target: &str,
-) -> HashMap<String, Vec<String>> {
-    let mut target_cfg = HashMap::new();
-
-    if !cfg!(test) {
-        let rustc_output = Command::new(&rustc_path)
-            .args(&["--target", &target])
-            .args(&["--print", "cfg"])
-            .output()
-            .unwrap()
-            .stdout;
-        let rustc_output = String::from_utf8(rustc_output).unwrap();
-
-        for line in rustc_output.lines() {
-            if let Some((name, value)) = line.split_once('=') {
-                let normalized_value = value.trim_matches('"');
-                cfg_add(&mut target_cfg, name, normalized_value);
-            } else {
-                cfg_add(&mut target_cfg, line, "");
-            }
-        }
-    }
-    target_cfg
-}
-
-/// Adds the given name and value to the provided cfg [`HashMap`]. If the `name` already
-/// points to a vector, this adds `value` to the vector. If `name` does not point
-/// to a vector, this adds a new vector containing only `value` to the [`HashMap`].
-fn cfg_add(map: &mut HashMap<String, Vec<String>>, name: &str, value: &str) {
-    let name = name.to_string();
-    let value = value.to_string();
-
-    if let Some(values) = map.get_mut(&name) {
-        values.push(value.to_string());
-    } else {
-        map.insert(name, vec![value.to_string()]);
-    }
-}
-
-/// Checks if the cfg HashMap has the given `name`. If the `required_value` is
-/// `Some(value)`, this will only return `true` if `name` is associated with `value`.
-pub fn cfg_has(
-    map: &HashMap<String, Vec<String>>,
-    name: &str,
-    required_value: Option<&str>,
-) -> bool {
-    let name = name.replace("-", "_");
-    let required_value = required_value.map(str::trim).map(str::to_string);
-
-    match (map.get(&name), required_value) {
-        (None, _) => false,
-        (Some(_), None) => true,
-        (Some(values), Some(required_value)) => values.contains(&required_value),
-    }
-}
-
 pub trait PathBufExt {
     /// Append an extension to the path, even if it already has one.
     fn with_extra_extension<S: AsRef<OsStr>>(&self, extension: S) -> PathBuf;