compiletest: Simplify handling of Clang-based tests.

michaelwoerister · michaelwoerister · commit b38125c3bba8 · 2019-01-17T16:49:32.000+01:00
diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
@@ -1106,13 +1106,11 @@ impl Step for Compiletest {
                     }).to_string()
             })
         };
-        let (lldb_exe, clang_exe) =
-            if builder.config.lldb_enabled && !target.contains("emscripten") {
+        let lldb_exe = if builder.config.lldb_enabled && !target.contains("emscripten") {
             // Test against the lldb that was just built.
-            (builder.llvm_out(target).join("bin").join("lldb"),
-             builder.llvm_out(target).join("bin").join("clang"))
+            builder.llvm_out(target).join("bin").join("lldb")
         } else {
-            (PathBuf::from("lldb"), PathBuf::from("clang"))
+            PathBuf::from("lldb")
         };
         let lldb_version = Command::new(&lldb_exe)
             .arg("--version")
@@ -1127,19 +1125,14 @@ impl Step for Compiletest {
             }
         }
 
-        let clang_version = Command::new(&clang_exe)
-            .arg("--version")
-            .output()
-            .map(|output| { String::from_utf8_lossy(&output.stdout).to_string() })
-            .ok();
-        if let Some(ref vers) = clang_version {
-            cmd.arg("--clang-version").arg(vers);
-        }
-
         if let Some(var) = env::var_os("RUSTBUILD_FORCE_CLANG_BASED_TESTS") {
-            match &var.to_string_lossy()[..] {
+            match &var.to_string_lossy().to_lowercase()[..] {
                 "1" | "yes" | "on" => {
-                    cmd.arg("--force-clang-based-tests");
+                    assert!(builder.config.lldb_enabled,
+                        "RUSTBUILD_FORCE_CLANG_BASED_TESTS needs Clang/LLDB to \
+                         be built.");
+                    let clang_exe = builder.llvm_out(target).join("bin").join("clang");
+                    cmd.arg("--run-clang-based-tests-with").arg(clang_exe);
                 }
                 "0" | "no" | "off" => {
                     // Nothing to do.
diff --git a/src/test/run-make-fulldeps/cross-lang-lto-clang/Makefile b/src/test/run-make-fulldeps/cross-lang-lto-clang/Makefile
@@ -9,7 +9,7 @@ all: cpp-executable rust-executable
 
 cpp-executable:
 	$(RUSTC) -Zcross-lang-lto=on -o $(TMPDIR)/librustlib-xlto.a -Copt-level=2 -Ccodegen-units=1 ./rustlib.rs
-	clang -flto=thin -fuse-ld=lld -L $(TMPDIR) -lrustlib-xlto -o $(TMPDIR)/cmain ./cmain.c -O3
+	$(CLANG) -flto=thin -fuse-ld=lld -L $(TMPDIR) -lrustlib-xlto -o $(TMPDIR)/cmain ./cmain.c -O3
 	# Make sure we don't find a call instruction to the function we expect to
 	# always be inlined.
 	llvm-objdump -d $(TMPDIR)/cmain | $(CGREP) -v -e "call.*rust_always_inlined"
@@ -18,8 +18,8 @@ cpp-executable:
 	llvm-objdump -d $(TMPDIR)/cmain | $(CGREP) -e "call.*rust_never_inlined"
 
 rust-executable:
-	clang ./clib.c -flto=thin -c -o $(TMPDIR)/clib.o -O2
+	$(CLANG) ./clib.c -flto=thin -c -o $(TMPDIR)/clib.o -O2
 	(cd $(TMPDIR); $(AR) crus ./libxyz.a ./clib.o)
-	$(RUSTC) -Zcross-lang-lto=on -L$(TMPDIR) -Copt-level=2 -Clinker=clang -Clink-arg=-fuse-ld=lld ./main.rs -o $(TMPDIR)/rsmain
+	$(RUSTC) -Zcross-lang-lto=on -L$(TMPDIR) -Copt-level=2 -Clinker=$(CLANG) -Clink-arg=-fuse-ld=lld ./main.rs -o $(TMPDIR)/rsmain
 	llvm-objdump -d $(TMPDIR)/rsmain | $(CGREP) -e "call.*c_never_inlined"
 	llvm-objdump -d $(TMPDIR)/rsmain | $(CGREP) -v -e "call.*c_always_inlined"
diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs
@@ -144,9 +144,9 @@ pub struct Config {
     /// (or, alternatively, to silently run them like regular run-pass tests).
     pub force_valgrind: bool,
 
-    /// Whether to fail if we don't have a clang version available that matches
-    /// rustc's LLVM version.
-    pub force_clang_based_tests: bool,
+    /// The path to the Clang executable to run Clang-based tests with. If
+    /// `None` then these tests will be ignored.
+    pub run_clang_based_tests_with: Option<String>,
 
     /// The directory containing the tests to run
     pub src_base: PathBuf,
@@ -209,9 +209,6 @@ pub struct Config {
     /// Is LLVM a system LLVM
     pub system_llvm: bool,
 
-    /// The version of Clang available to run-make tests (if any).
-    pub clang_version: Option<String>,
-
     /// Path to the android tools
     pub android_cross_path: PathBuf,
 
diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs
@@ -112,7 +112,7 @@ impl EarlyProps {
                     props.ignore = Ignore::Ignore;
                 }
 
-                if !config.force_clang_based_tests &&
+                if config.run_clang_based_tests_with.is_none() &&
                    config.parse_needs_matching_clang(ln) {
                     props.ignore = Ignore::Ignore;
                 }
diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs
@@ -50,32 +50,12 @@ pub mod util;
 fn main() {
     env_logger::init();
 
-    let mut config = parse_config(env::args().collect());
+    let config = parse_config(env::args().collect());
 
     if config.valgrind_path.is_none() && config.force_valgrind {
         panic!("Can't find Valgrind to run Valgrind tests");
     }
 
-    // Some run-make tests need a version of Clang available that matches
-    // rustc's LLVM version. Since this isn't always the case, these tests are
-    // opt-in.
-    let clang_based_tests_possible = check_clang_based_tests_possible(&config);
-    match (clang_based_tests_possible, config.force_clang_based_tests) {
-        (Ok(_), true) |
-        (Err(_), false) => {
-            // Nothing to do
-        }
-        (Ok(_), false) => {
-            // If a valid clang version is available, run the tests even if
-            // they are not forced.
-            config.force_clang_based_tests = true;
-        }
-        (Err(msg), true) => {
-            // Tests are forced but we don't have a valid version of Clang.
-            panic!("{}", msg)
-        }
-    }
-
     log_config(&config);
     run_tests(&config);
 }
@@ -128,10 +108,11 @@ pub fn parse_config(args: Vec<String>) -> Config {
             "force-valgrind",
             "fail if Valgrind tests cannot be run under Valgrind",
         )
-        .optflag(
+        .optopt(
             "",
-            "force-clang-based-tests",
-            "fail if Clang-based run-make tests can't be run for some reason",
+            "run-clang-based-tests-with",
+            "path to Clang executable",
+            "PATH",
         )
         .optopt(
             "",
@@ -214,12 +195,6 @@ pub fn parse_config(args: Vec<String>) -> Config {
             "VERSION STRING",
         )
         .optflag("", "system-llvm", "is LLVM the system LLVM")
-        .optopt(
-            "",
-            "clang-version",
-            "the version of Clang available to run-make tests",
-            "VERSION STRING",
-        )
         .optopt(
             "",
             "android-cross-path",
@@ -329,7 +304,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
         docck_python: matches.opt_str("docck-python").unwrap(),
         valgrind_path: matches.opt_str("valgrind-path"),
         force_valgrind: matches.opt_present("force-valgrind"),
-        force_clang_based_tests: matches.opt_present("force-clang-based-tests"),
+        run_clang_based_tests_with: matches.opt_str("run-clang-based-tests-with"),
         llvm_filecheck: matches.opt_str("llvm-filecheck").map(|s| PathBuf::from(&s)),
         src_base,
         build_base: opt_path(matches, "build-base"),
@@ -355,7 +330,6 @@ pub fn parse_config(args: Vec<String>) -> Config {
         lldb_native_rust,
         llvm_version: matches.opt_str("llvm-version"),
         system_llvm: matches.opt_present("system-llvm"),
-        clang_version: matches.opt_str("clang-version"),
         android_cross_path: android_cross_path,
         adb_path: opt_str2(matches.opt_str("adb-path")),
         adb_test_dir: opt_str2(matches.opt_str("adb-test-dir")),
@@ -1064,54 +1038,3 @@ fn test_extract_gdb_version() {
         7012050: "GNU gdb (GDB) 7.12.50.20161027-git",
     }
 }
-
-
-fn check_clang_based_tests_possible(config: &Config) -> Result<(), String> {
-
-    let llvm_version = if let Some(llvm_version) = config.llvm_version.as_ref() {
-        llvm_version
-    } else {
-        return Err(format!("Running `compiletest` with `--force-clang-based-tests` \
-                            requires `--llvm-version` to be specified."));
-    };
-
-    let clang_major_version = if let Some(ref version_string) = config.clang_version {
-        major_version_from_clang_version_string(version_string)?
-    } else {
-        return Err(format!("Clang is required for running tests \
-                            (because of --force-clang-based-tests) \
-                            but it does not seem to be available."));
-    };
-
-    let rustc_llvm_major_version = major_version_from_llvm_version_string(&llvm_version)?;
-
-    return if clang_major_version != rustc_llvm_major_version {
-        Err(format!("`--force-clang-based-tests` needs the major version of Clang \
-                     and rustc's LLVM to be the same. Clang version is: {}, \
-                     Rustc LLVM is: {}",
-                     config.clang_version.clone().unwrap(),
-                     llvm_version))
-    } else {
-        Ok(())
-    };
-
-    fn major_version_from_clang_version_string(clang_version: &str) -> Result<&str, String> {
-        let re = regex::Regex::new(r"clang version (\d)\.\d").unwrap();
-        if let Some(captures) = re.captures(clang_version) {
-            Ok(captures.get(1).unwrap().as_str())
-        } else {
-            Err(format!("Failed to parse major version from Clang version \
-                         string '{}'.", clang_version))
-        }
-    }
-
-    fn major_version_from_llvm_version_string(llvm_version: &str) -> Result<&str, String> {
-        let re = regex::Regex::new(r"(\d)\.\d").unwrap();
-        if let Some(captures) = re.captures(llvm_version) {
-            Ok(captures.get(1).unwrap().as_str())
-        } else {
-            Err(format!("Failed to parse major version from LLVM version \
-                         string '{}'.", llvm_version))
-        }
-    }
-}
diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs
@@ -2587,6 +2587,10 @@ impl<'test> TestCx<'test> {
             cmd.env("RUSTC_LINKER", linker);
         }
 
+        if let Some(ref clang) = self.config.run_clang_based_tests_with {
+            cmd.env("CLANG", clang);
+        }
+
         // We don't want RUSTFLAGS set from the outside to interfere with
         // compiler flags set in the test cases:
         cmd.env_remove("RUSTFLAGS");

Original file line number	Diff line number	Diff line change
`@@ -112,7 +112,7 @@ impl EarlyProps {`
`112`	`112`	`props.ignore = Ignore::Ignore;`
`113`	`113`	`}`
`114`	`114`
`115`		`- if !config.force_clang_based_tests &&`
	`115`	`+ if config.run_clang_based_tests_with.is_none() &&`
`116`	`116`	`config.parse_needs_matching_clang(ln) {`
`117`	`117`	`props.ignore = Ignore::Ignore;`
`118`	`118`	`}`