From cfa3a330149b78c4d827b10d9ac4ff8d381e7342 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH] compiletest: Rewrite extract_lldb_version function This makes extract_lldb_version has the same version type like extract_gdb_version. This is technically a breaking change for rustc-dev users. But note that rustc-dev is a nightly component. --- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/header.rs | 23 +++----- src/tools/compiletest/src/main.rs | 90 ++++++++++------------------- src/tools/compiletest/src/tests.rs | 11 ++++ 4 files changed, 50 insertions(+), 76 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 703b87634cec..4abb1db35a02 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -268,7 +268,7 @@ pub struct Config { pub gdb_native_rust: bool, /// Version of LLDB - pub lldb_version: Option, + pub lldb_version: Option, /// Whether LLDB has native rust support pub lldb_native_rust: bool, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index d6e28e93c966..0ee11608dc5b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -188,16 +188,17 @@ impl EarlyProps { } fn ignore_lldb(config: &Config, line: &str) -> bool { - if let Some(ref actual_version) = config.lldb_version { - if line.starts_with("min-lldb-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed lldb version directive"); + if let Some(actual_version) = config.lldb_version { + if let Some(min_version) = line.strip_prefix("min-lldb-version:").map(str::trim) { + let min_version = min_version.parse().unwrap_or_else(|e| { + panic!( + "Unexpected format of LLDB version string: {}\n{:?}", + min_version, e + ); + }); // Ignore if actual version is smaller the minimum required // version - lldb_version_to_int(actual_version) < lldb_version_to_int(min_version) + actual_version < min_version } else if line.starts_with("rust-lldb") && !config.lldb_native_rust { true } else { @@ -944,12 +945,6 @@ impl Config { } } -pub fn lldb_version_to_int(version_string: &str) -> isize { - let error_string = - format!("Encountered LLDB version string with unexpected format: {}", version_string); - version_string.parse().expect(&error_string) -} - fn expand_variables(mut value: String, config: &Config) -> String { const CWD: &'static str = "{{cwd}}"; const SRC_BASE: &'static str = "{{src-base}}"; diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 97272f1a9c1b..2b0ff0da9f5c 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -165,8 +165,12 @@ pub fn parse_config(args: Vec) -> Config { let cdb = analyze_cdb(matches.opt_str("cdb"), &target); let (gdb, gdb_version, gdb_native_rust) = analyze_gdb(matches.opt_str("gdb"), &target, &android_cross_path); - let (lldb_version, lldb_native_rust) = extract_lldb_version(matches.opt_str("lldb-version")); - + let (lldb_version, lldb_native_rust) = matches + .opt_str("lldb-version") + .as_deref() + .and_then(extract_lldb_version) + .map(|(v, b)| (Some(v), b)) + .unwrap_or((None, false)); let color = match matches.opt_str("color").as_ref().map(|x| &**x) { Some("auto") | None => ColorConfig::AutoColor, Some("always") => ColorConfig::AlwaysColor, @@ -400,17 +404,14 @@ fn configure_lldb(config: &Config) -> Option { return None; } - if let Some(lldb_version) = config.lldb_version.as_ref() { - if lldb_version == "350" { - println!( - "WARNING: The used version of LLDB ({}) has a \ - known issue that breaks debuginfo tests. See \ - issue #32520 for more information. Skipping all \ - LLDB-based tests!", - lldb_version - ); - return None; - } + if let Some(350) = config.lldb_version { + println!( + "WARNING: The used version of LLDB (350) has a \ + known issue that breaks debuginfo tests. See \ + issue #32520 for more information. Skipping all \ + LLDB-based tests!", + ); + return None; } // Some older versions of LLDB seem to have problems with multiple @@ -908,7 +909,7 @@ fn extract_gdb_version(full_version_line: &str) -> Option { } /// Returns (LLDB version, LLDB is rust-enabled) -fn extract_lldb_version(full_version_line: Option) -> (Option, bool) { +fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> { // Extract the major LLDB version from the given version string. // LLDB version strings are different for Apple and non-Apple platforms. // The Apple variant looks like this: @@ -917,7 +918,7 @@ fn extract_lldb_version(full_version_line: Option) -> (Option, b // lldb-300.2.51 (new versions) // // We are only interested in the major version number, so this function - // will return `Some("179")` and `Some("300")` respectively. + // will return `Some(179)` and `Some(300)` respectively. // // Upstream versions look like: // lldb version 6.0.1 @@ -929,53 +930,20 @@ fn extract_lldb_version(full_version_line: Option) -> (Option, b // normally fine because the only non-Apple version we test is // rust-enabled. - if let Some(ref full_version_line) = full_version_line { - if !full_version_line.trim().is_empty() { - let full_version_line = full_version_line.trim(); + let full_version_line = full_version_line.trim(); - for (pos, l) in full_version_line.char_indices() { - if l != 'l' && l != 'L' { - continue; - } - if pos + 5 >= full_version_line.len() { - continue; - } - let l = full_version_line[pos + 1..].chars().next().unwrap(); - if l != 'l' && l != 'L' { - continue; - } - let d = full_version_line[pos + 2..].chars().next().unwrap(); - if d != 'd' && d != 'D' { - continue; - } - let b = full_version_line[pos + 3..].chars().next().unwrap(); - if b != 'b' && b != 'B' { - continue; - } - let dash = full_version_line[pos + 4..].chars().next().unwrap(); - if dash != '-' { - continue; - } - - let vers = full_version_line[pos + 5..] - .chars() - .take_while(|c| c.is_digit(10)) - .collect::(); - if !vers.is_empty() { - return (Some(vers), full_version_line.contains("rust-enabled")); - } - } - - if full_version_line.starts_with("lldb version ") { - let vers = full_version_line[13..] - .chars() - .take_while(|c| c.is_digit(10)) - .collect::(); - if !vers.is_empty() { - return (Some(vers + "00"), full_version_line.contains("rust-enabled")); - } - } + if let Some(apple_ver) = + full_version_line.strip_prefix("LLDB-").or_else(|| full_version_line.strip_prefix("lldb-")) + { + if let Some(idx) = apple_ver.find(|c: char| !c.is_digit(10)) { + let version: u32 = apple_ver[..idx].parse().unwrap(); + return Some((version, full_version_line.contains("rust-enabled"))); + } + } else if let Some(lldb_ver) = full_version_line.strip_prefix("lldb version ") { + if let Some(idx) = lldb_ver.find(|c: char| !c.is_digit(10)) { + let version: u32 = lldb_ver[..idx].parse().unwrap(); + return Some((version * 100, full_version_line.contains("rust-enabled"))); } } - (None, false) + None } diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 31c151d29e91..7669ec53bf72 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -41,6 +41,17 @@ fn test_extract_gdb_version() { } } +#[test] +fn test_extract_lldb_version() { + // Apple variants + assert_eq!(extract_lldb_version("LLDB-179.5"), Some((179, false))); + assert_eq!(extract_lldb_version("lldb-300.2.51"), Some((300, false))); + + // Upstream versions + assert_eq!(extract_lldb_version("lldb version 6.0.1"), Some((600, false))); + assert_eq!(extract_lldb_version("lldb version 9.0.0"), Some((900, false))); +} + #[test] fn is_test_test() { assert_eq!(true, is_test(&OsString::from("a_test.rs")));