From cfa3a330149b78c4d827b10d9ac4ff8d381e7342 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 01/11] 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 703b87634cec3..4abb1db35a02f 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 d6e28e93c9667..0ee11608dc5b3 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 97272f1a9c1b6..2b0ff0da9f5c5 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(); - - 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")); - } - } + let full_version_line = full_version_line.trim(); - 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 31c151d29e916..7669ec53bf72f 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"))); From d778f326c385b2df7053b84fb5e3f89361b5fc3a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 02/11] compiletest: Rewrite extract_gdb_version function --- src/tools/compiletest/src/main.rs | 89 +++++++++--------------------- src/tools/compiletest/src/tests.rs | 2 +- 2 files changed, 28 insertions(+), 63 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 2b0ff0da9f5c5..80f611d2ad6a4 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -841,71 +841,36 @@ fn extract_gdb_version(full_version_line: &str) -> Option { // This particular form is documented in the GNU coding standards: // https://www.gnu.org/prep/standards/html_node/_002d_002dversion.html#g_t_002d_002dversion - // don't start parsing in the middle of a number - let mut prev_was_digit = false; - let mut in_parens = false; - for (pos, c) in full_version_line.char_indices() { - if in_parens { - if c == ')' { - in_parens = false; - } - continue; - } else if c == '(' { - in_parens = true; - continue; - } - - if prev_was_digit || !c.is_digit(10) { - prev_was_digit = c.is_digit(10); - continue; + let mut splits = full_version_line.rsplit(' '); + let version_string = splits.next().unwrap(); + + let mut splits = version_string.split('.'); + let major = splits.next().unwrap(); + let minor = splits.next().unwrap(); + let patch = splits.next(); + + let major: u32 = major.parse().unwrap(); + let (minor, patch): (u32, u32) = match minor.find(|c: char| !c.is_digit(10)) { + None => { + let minor = minor.parse().unwrap(); + let patch: u32 = match patch { + Some(patch) => match patch.find(|c: char| !c.is_digit(10)) { + None => patch.parse().unwrap(), + Some(idx) if idx > 3 => 0, + Some(idx) => patch[..idx].parse().unwrap(), + }, + None => 0, + }; + (minor, patch) } - - prev_was_digit = true; - - let line = &full_version_line[pos..]; - - let next_split = match line.find(|c: char| !c.is_digit(10)) { - Some(idx) => idx, - None => continue, // no minor version - }; - - if line.as_bytes()[next_split] != b'.' { - continue; // no minor version + // There is no patch version after minor-date (e.g. "4-2012"). + Some(idx) => { + let minor = minor[..idx].parse().unwrap(); + (minor, 0) } + }; - let major = &line[..next_split]; - let line = &line[next_split + 1..]; - - let (minor, patch) = match line.find(|c: char| !c.is_digit(10)) { - Some(idx) => { - if line.as_bytes()[idx] == b'.' { - let patch = &line[idx + 1..]; - - let patch_len = - patch.find(|c: char| !c.is_digit(10)).unwrap_or_else(|| patch.len()); - let patch = &patch[..patch_len]; - let patch = if patch_len > 3 || patch_len == 0 { None } else { Some(patch) }; - - (&line[..idx], patch) - } else { - (&line[..idx], None) - } - } - None => (line, None), - }; - - if minor.is_empty() { - continue; - } - - let major: u32 = major.parse().unwrap(); - let minor: u32 = minor.parse().unwrap(); - let patch: u32 = patch.unwrap_or("0").parse().unwrap(); - - return Some(((major * 1000) + minor) * 1000 + patch); - } - - None + Some(((major * 1000) + minor) * 1000 + patch) } /// Returns (LLDB version, LLDB is rust-enabled) diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 7669ec53bf72f..237bb756597a3 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -2,7 +2,7 @@ use super::*; #[test] fn test_extract_gdb_version() { - macro_rules! test { ($($expectation:tt: $input:tt,)*) => {{$( + macro_rules! test { ($($expectation:literal: $input:literal,)*) => {{$( assert_eq!(extract_gdb_version($input), Some($expectation)); )*}}} From 07d56cba8ffd854f1c8b91bb1372130e5abe6169 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 13:48:21 +0000 Subject: [PATCH 03/11] Fix panic as passing wrong format to `extract_gdb_version` --- src/tools/compiletest/src/header.rs | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 0ee11608dc5b3..dc561352642b3 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -132,32 +132,29 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { - if line.starts_with("min-gdb-version") { - let (start_ver, end_ver) = extract_gdb_version_range(line); + if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { + let (start_ver, end_ver) = extract_gdb_version_range(rest); if start_ver != end_ver { panic!("Expected single GDB version") } // Ignore if actual version is smaller the minimum required // version - actual_version < start_ver - } else if line.starts_with("ignore-gdb-version") { - let (min_version, max_version) = extract_gdb_version_range(line); + return actual_version < start_ver; + } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { + let (min_version, max_version) = extract_gdb_version_range(rest); if max_version < min_version { panic!("Malformed GDB version range: max < min") } - actual_version >= min_version && actual_version <= max_version - } else { - false + return actual_version >= min_version && actual_version <= max_version; } - } else { - false } + false } - // Takes a directive of the form "ignore-gdb-version [- ]", + // Takes a directive of the form " [- ]", // returns the numeric representation of and as // tuple: ( as u32, as u32) // If the part is omitted, the second component of the tuple From 79d5cbbf867a888000b8bbd47194bc57b343b72a Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 08:45:26 +0000 Subject: [PATCH 04/11] Use Option::as_deref --- src/tools/compiletest/src/main.rs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 80f611d2ad6a4..3929ce28ab0a6 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -171,7 +171,7 @@ pub fn parse_config(args: Vec) -> Config { .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) { + let color = match matches.opt_str("color").as_deref() { Some("auto") | None => ColorConfig::AutoColor, Some("always") => ColorConfig::AlwaysColor, Some("never") => ColorConfig::NeverColor, @@ -255,7 +255,7 @@ pub fn log_config(config: &Config) { logv(c, format!("stage_id: {}", config.stage_id)); logv(c, format!("mode: {}", config.mode)); logv(c, format!("run_ignored: {}", config.run_ignored)); - logv(c, format!("filter: {}", opt_str(&config.filter.as_ref().map(|re| re.to_owned())))); + logv(c, format!("filter: {}", opt_str(&config.filter))); logv(c, format!("filter_exact: {}", config.filter_exact)); logv( c, @@ -723,9 +723,7 @@ fn make_test_closure( let config = config.clone(); let testpaths = testpaths.clone(); let revision = revision.cloned(); - test::DynTestFn(Box::new(move || { - runtest::run(config, &testpaths, revision.as_ref().map(|s| s.as_str())) - })) + test::DynTestFn(Box::new(move || runtest::run(config, &testpaths, revision.as_deref()))) } /// Returns `true` if the given target is an Android target for the From 75caee076d41eca5163a4094e44ce6d48f6b4f1b Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 09:06:59 +0000 Subject: [PATCH 05/11] Extract closure to function --- src/tools/compiletest/src/main.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 3929ce28ab0a6..049bf44d6fd96 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -848,11 +848,11 @@ fn extract_gdb_version(full_version_line: &str) -> Option { let patch = splits.next(); let major: u32 = major.parse().unwrap(); - let (minor, patch): (u32, u32) = match minor.find(|c: char| !c.is_digit(10)) { + let (minor, patch): (u32, u32) = match minor.find(not_a_digit) { None => { let minor = minor.parse().unwrap(); let patch: u32 = match patch { - Some(patch) => match patch.find(|c: char| !c.is_digit(10)) { + Some(patch) => match patch.find(not_a_digit) { None => patch.parse().unwrap(), Some(idx) if idx > 3 => 0, Some(idx) => patch[..idx].parse().unwrap(), @@ -898,15 +898,19 @@ fn extract_lldb_version(full_version_line: &str) -> Option<(u32, bool)> { 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)) { + if let Some(idx) = apple_ver.find(not_a_digit) { 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)) { + if let Some(idx) = lldb_ver.find(not_a_digit) { let version: u32 = lldb_ver[..idx].parse().unwrap(); return Some((version * 100, full_version_line.contains("rust-enabled"))); } } None } + +fn not_a_digit(c: char) -> bool { + !c.is_digit(10) +} From 5aa33b11fc88d0f81c7b2dc586db1439fcf2b664 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 11 Jul 2020 13:50:46 +0000 Subject: [PATCH 06/11] Use subslice pattern --- src/tools/compiletest/src/header.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index dc561352642b3..e155a9249902f 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -170,14 +170,14 @@ impl EarlyProps { .take(3) // 3 or more = invalid, so take at most 3. .collect::>>(); - match range_components.len() { - 1 => { - let v = range_components[0].unwrap(); + match *range_components { + [v] => { + let v = v.unwrap(); (v, v) } - 2 => { - let v_min = range_components[0].unwrap(); - let v_max = range_components[1].expect(ERROR_MESSAGE); + [min, max] => { + let v_min = min.unwrap(); + let v_max = max.expect(ERROR_MESSAGE); (v_min, v_max) } _ => panic!(ERROR_MESSAGE), From 5e5d8163233df1421925c7351377ade2306742de Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sat, 18 Jul 2020 03:17:30 +0000 Subject: [PATCH 07/11] Add missing : after min-gdb-version --- src/test/debuginfo/function-call.rs | 2 +- src/test/debuginfo/pretty-huge-vec.rs | 2 +- src/test/debuginfo/pretty-std-collections.rs | 2 +- src/test/debuginfo/pretty-std.rs | 2 +- src/test/debuginfo/pretty-uninitialized-vec.rs | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/test/debuginfo/function-call.rs b/src/test/debuginfo/function-call.rs index 98ad8983a60a0..a5d5942b53953 100644 --- a/src/test/debuginfo/function-call.rs +++ b/src/test/debuginfo/function-call.rs @@ -1,5 +1,5 @@ // This test does not passed with gdb < 8.0. See #53497. -// min-gdb-version 8.0 +// min-gdb-version: 8.0 // compile-flags:-g diff --git a/src/test/debuginfo/pretty-huge-vec.rs b/src/test/debuginfo/pretty-huge-vec.rs index 2616c9465246e..cbd2278f7e27c 100644 --- a/src/test/debuginfo/pretty-huge-vec.rs +++ b/src/test/debuginfo/pretty-huge-vec.rs @@ -2,7 +2,7 @@ // ignore-freebsd: gdb package too new // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 // === GDB TESTS =================================================================================== diff --git a/src/test/debuginfo/pretty-std-collections.rs b/src/test/debuginfo/pretty-std-collections.rs index 4e95a028e0749..a4fbff5725c97 100644 --- a/src/test/debuginfo/pretty-std-collections.rs +++ b/src/test/debuginfo/pretty-std-collections.rs @@ -6,7 +6,7 @@ // The pretty printers being tested here require the patch from // https://sourceware.org/bugzilla/show_bug.cgi?id=21763 -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 diff --git a/src/test/debuginfo/pretty-std.rs b/src/test/debuginfo/pretty-std.rs index 57721ce103c39..7ae82d522b09d 100644 --- a/src/test/debuginfo/pretty-std.rs +++ b/src/test/debuginfo/pretty-std.rs @@ -2,7 +2,7 @@ // only-cdb // "Temporarily" ignored on GDB/LLDB due to debuginfo tests being disabled, see PR 47155 // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 7.7 +// min-gdb-version: 7.7 // min-lldb-version: 310 // === GDB TESTS =================================================================================== diff --git a/src/test/debuginfo/pretty-uninitialized-vec.rs b/src/test/debuginfo/pretty-uninitialized-vec.rs index 7ce004681e100..61791f48f4db7 100644 --- a/src/test/debuginfo/pretty-uninitialized-vec.rs +++ b/src/test/debuginfo/pretty-uninitialized-vec.rs @@ -2,7 +2,7 @@ // ignore-freebsd: gdb package too new // ignore-android: FIXME(#10381) // compile-flags:-g -// min-gdb-version 8.1 +// min-gdb-version: 8.1 // min-lldb-version: 310 // === GDB TESTS =================================================================================== From 2bcefa8d81fdf46f8ad9d893d271788477ccf95b Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 10:45:31 +0000 Subject: [PATCH 08/11] Add missing : after *llvm-version --- src/test/codegen/abi-efiapi.rs | 2 +- src/test/codegen/force-unwind-tables.rs | 2 +- src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs | 2 +- src/tools/compiletest/src/header/tests.rs | 8 ++++---- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/codegen/abi-efiapi.rs b/src/test/codegen/abi-efiapi.rs index 8aeee5859d0ad..7c61b7809901f 100644 --- a/src/test/codegen/abi-efiapi.rs +++ b/src/test/codegen/abi-efiapi.rs @@ -2,7 +2,7 @@ // revisions:x86_64 i686 arm -// min-llvm-version 9.0 +// min-llvm-version: 9.0 //[x86_64] compile-flags: --target x86_64-unknown-uefi //[i686] compile-flags: --target i686-unknown-linux-musl diff --git a/src/test/codegen/force-unwind-tables.rs b/src/test/codegen/force-unwind-tables.rs index fbaf38d69df7f..eba4a7469f930 100644 --- a/src/test/codegen/force-unwind-tables.rs +++ b/src/test/codegen/force-unwind-tables.rs @@ -1,4 +1,4 @@ -// min-llvm-version 8.0 +// min-llvm-version: 8.0 // compile-flags: -C no-prepopulate-passes -C force-unwind-tables=y #![crate_type="lib"] diff --git a/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs b/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs index 64d6ccf340916..9439df266d59b 100644 --- a/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs +++ b/src/test/ui/sanitize/new-llvm-pass-manager-thin-lto.rs @@ -2,7 +2,7 @@ // being run when compiling with new LLVM pass manager and ThinLTO. // Note: The issue occurred only on non-zero opt-level. // -// min-llvm-version 9.0 +// min-llvm-version: 9.0 // needs-sanitizer-support // needs-sanitizer-address // diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 72af34d78260b..58f73a4d7e060 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -120,16 +120,16 @@ fn llvm_version() { let mut config = config(); config.llvm_version = Some("8.1.2-rust".to_owned()); - assert!(parse_rs(&config, "// min-llvm-version 9.0").ignore); + assert!(parse_rs(&config, "// min-llvm-version: 9.0").ignore); config.llvm_version = Some("9.0.1-rust-1.43.0-dev".to_owned()); - assert!(parse_rs(&config, "// min-llvm-version 9.2").ignore); + assert!(parse_rs(&config, "// min-llvm-version: 9.2").ignore); config.llvm_version = Some("9.3.1-rust-1.43.0-dev".to_owned()); - assert!(!parse_rs(&config, "// min-llvm-version 9.2").ignore); + assert!(!parse_rs(&config, "// min-llvm-version: 9.2").ignore); config.llvm_version = Some("10.0.0-rust".to_owned()); - assert!(!parse_rs(&config, "// min-llvm-version 9.0").ignore); + assert!(!parse_rs(&config, "// min-llvm-version: 9.0").ignore); } #[test] From 99e3a3cdeaf9d6b42d329254b5d2752833c0c562 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 10:58:06 +0000 Subject: [PATCH 09/11] Extract extract_version_range --- src/tools/compiletest/src/header.rs | 66 +++++++++++++++-------------- 1 file changed, 34 insertions(+), 32 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index e155a9249902f..3d9c875499136 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -133,7 +133,7 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { - let (start_ver, end_ver) = extract_gdb_version_range(rest); + let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version); if start_ver != end_ver { panic!("Expected single GDB version") @@ -142,7 +142,8 @@ impl EarlyProps { // version return actual_version < start_ver; } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { - let (min_version, max_version) = extract_gdb_version_range(rest); + let (min_version, max_version) = + extract_version_range(rest, extract_gdb_version); if max_version < min_version { panic!("Malformed GDB version range: max < min") @@ -154,36 +155,6 @@ impl EarlyProps { false } - // Takes a directive of the form " [- ]", - // returns the numeric representation of and as - // tuple: ( as u32, as u32) - // If the part is omitted, the second component of the tuple - // is the same as . - fn extract_gdb_version_range(line: &str) -> (u32, u32) { - const ERROR_MESSAGE: &'static str = "Malformed GDB version directive"; - - let range_components = line - .split(&[' ', '-'][..]) - .filter(|word| !word.is_empty()) - .map(extract_gdb_version) - .skip_while(Option::is_none) - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>>(); - - match *range_components { - [v] => { - let v = v.unwrap(); - (v, v) - } - [min, max] => { - let v_min = min.unwrap(); - let v_max = max.expect(ERROR_MESSAGE); - (v_min, v_max) - } - _ => panic!(ERROR_MESSAGE), - } - } - fn ignore_lldb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.lldb_version { if let Some(min_version) = line.strip_prefix("min-lldb-version:").map(str::trim) { @@ -982,3 +953,34 @@ fn parse_normalization_string(line: &mut &str) -> Option { *line = &line[end + 1..]; Some(result) } + +// Takes a directive of the form " [- ]", +// returns the numeric representation of and as +// tuple: ( as u32, as u32) +// If the part is omitted, the second component of the tuple +// is the same as . +fn extract_version_range(line: &str, parse: F) -> (u32, u32) +where + F: Fn(&str) -> Option, +{ + let range_components = line + .split(&[' ', '-'][..]) + .filter(|word| !word.is_empty()) + .map(parse) + .skip_while(Option::is_none) + .take(3) // 3 or more = invalid, so take at most 3. + .collect::>>(); + + match *range_components { + [v] => { + let v = v.unwrap(); + (v, v) + } + [min, max] => { + let v_min = min.unwrap(); + let v_max = max.expect("Malformed version directive"); + (v_min, v_max) + } + _ => panic!("Malformed version directive"), + } +} From 60fac34c2048fe5e4fdb5c4516987b948d9f5192 Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 11:10:38 +0000 Subject: [PATCH 10/11] Rewrite extract_llvm_version --- src/tools/compiletest/src/common.rs | 2 +- src/tools/compiletest/src/header.rs | 83 +++++++++-------------- src/tools/compiletest/src/header/tests.rs | 8 +-- src/tools/compiletest/src/main.rs | 4 +- src/tools/compiletest/src/tests.rs | 9 +++ 5 files changed, 49 insertions(+), 57 deletions(-) diff --git a/src/tools/compiletest/src/common.rs b/src/tools/compiletest/src/common.rs index 4abb1db35a02f..c1c07e0bc00a0 100644 --- a/src/tools/compiletest/src/common.rs +++ b/src/tools/compiletest/src/common.rs @@ -274,7 +274,7 @@ pub struct Config { pub lldb_native_rust: bool, /// Version of LLVM - pub llvm_version: Option, + pub llvm_version: Option, /// Is LLVM a system LLVM pub system_llvm: bool, diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 3d9c875499136..4b5c44bca875b 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -181,48 +181,28 @@ impl EarlyProps { if config.system_llvm && line.starts_with("no-system-llvm") { return true; } - if let Some(ref actual_version) = config.llvm_version { - let actual_version = version_to_int(actual_version); - if line.starts_with("min-llvm-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed llvm version directive"); + if let Some(actual_version) = config.llvm_version { + if let Some(rest) = line.strip_prefix("min-llvm-version:").map(str::trim) { + let min_version = extract_llvm_version(rest).unwrap(); // Ignore if actual version is smaller the minimum required // version - actual_version < version_to_int(min_version) - } else if line.starts_with("min-system-llvm-version") { - let min_version = line - .trim_end() - .rsplit(' ') - .next() - .expect("Malformed llvm version directive"); + actual_version < min_version + } else if let Some(rest) = + line.strip_prefix("min-system-llvm-version:").map(str::trim) + { + let min_version = extract_llvm_version(rest).unwrap(); // Ignore if using system LLVM and actual version // is smaller the minimum required version - config.system_llvm && actual_version < version_to_int(min_version) - } else if line.starts_with("ignore-llvm-version") { - // Syntax is: "ignore-llvm-version [- ]" - let range_components = line - .split(' ') - .skip(1) // Skip the directive. - .map(|s| s.trim()) - .filter(|word| !word.is_empty() && word != &"-") - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>(); - match range_components.len() { - 1 => actual_version == version_to_int(range_components[0]), - 2 => { - let v_min = version_to_int(range_components[0]); - let v_max = version_to_int(range_components[1]); - if v_max < v_min { - panic!("Malformed LLVM version range: max < min") - } - // Ignore if version lies inside of range. - actual_version >= v_min && actual_version <= v_max - } - _ => panic!("Malformed LLVM version directive"), + config.system_llvm && actual_version < min_version + } else if let Some(rest) = line.strip_prefix("ignore-llvm-version:").map(str::trim) + { + // Syntax is: "ignore-llvm-version: [- ]" + let (v_min, v_max) = extract_version_range(rest, extract_llvm_version); + if v_max < v_min { + panic!("Malformed LLVM version range: max < min") } + // Ignore if version lies inside of range. + actual_version >= v_min && actual_version <= v_max } else { false } @@ -230,20 +210,6 @@ impl EarlyProps { false } } - - fn version_to_int(version: &str) -> u32 { - let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); - let components: Vec = version_without_suffix - .split('.') - .map(|s| s.parse().expect("Malformed version component")) - .collect(); - match components.len() { - 1 => components[0] * 10000, - 2 => components[0] * 10000 + components[1] * 100, - 3 => components[0] * 10000 + components[1] * 100 + components[2], - _ => panic!("Malformed version"), - } - } } } @@ -954,6 +920,21 @@ fn parse_normalization_string(line: &mut &str) -> Option { Some(result) } +pub fn extract_llvm_version(version: &str) -> Option { + let version_without_suffix = version.trim_end_matches("git").split('-').next().unwrap(); + let components: Vec = version_without_suffix + .split('.') + .map(|s| s.parse().expect("Malformed version component")) + .collect(); + let version = match *components { + [a] => a * 10_000, + [a, b] => a * 10_000 + b * 100, + [a, b, c] => a * 10_000 + b * 100 + c, + _ => panic!("Malformed version"), + }; + Some(version) +} + // Takes a directive of the form " [- ]", // returns the numeric representation of and as // tuple: ( as u32, as u32) diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index 58f73a4d7e060..adcd8691d0f58 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -119,16 +119,16 @@ fn no_system_llvm() { fn llvm_version() { let mut config = config(); - config.llvm_version = Some("8.1.2-rust".to_owned()); + config.llvm_version = Some(80102); assert!(parse_rs(&config, "// min-llvm-version: 9.0").ignore); - config.llvm_version = Some("9.0.1-rust-1.43.0-dev".to_owned()); + config.llvm_version = Some(90001); assert!(parse_rs(&config, "// min-llvm-version: 9.2").ignore); - config.llvm_version = Some("9.3.1-rust-1.43.0-dev".to_owned()); + config.llvm_version = Some(90301); assert!(!parse_rs(&config, "// min-llvm-version: 9.2").ignore); - config.llvm_version = Some("10.0.0-rust".to_owned()); + config.llvm_version = Some(100000); assert!(!parse_rs(&config, "// min-llvm-version: 9.0").ignore); } diff --git a/src/tools/compiletest/src/main.rs b/src/tools/compiletest/src/main.rs index 049bf44d6fd96..0b46aace49944 100644 --- a/src/tools/compiletest/src/main.rs +++ b/src/tools/compiletest/src/main.rs @@ -177,6 +177,8 @@ pub fn parse_config(args: Vec) -> Config { Some("never") => ColorConfig::NeverColor, Some(x) => panic!("argument for --color must be auto, always, or never, but found `{}`", x), }; + let llvm_version = + matches.opt_str("llvm-version").as_deref().and_then(header::extract_llvm_version); let src_base = opt_path(matches, "src-base"); let run_ignored = matches.opt_present("ignored"); @@ -217,7 +219,7 @@ pub fn parse_config(args: Vec) -> Config { gdb_native_rust, lldb_version, lldb_native_rust, - llvm_version: matches.opt_str("llvm-version"), + llvm_version, system_llvm: matches.opt_present("system-llvm"), android_cross_path, adb_path: opt_str2(matches.opt_str("adb-path")), diff --git a/src/tools/compiletest/src/tests.rs b/src/tools/compiletest/src/tests.rs index 237bb756597a3..ea9bc1c1a5b7f 100644 --- a/src/tools/compiletest/src/tests.rs +++ b/src/tools/compiletest/src/tests.rs @@ -1,3 +1,4 @@ +use super::header::extract_llvm_version; use super::*; #[test] @@ -60,3 +61,11 @@ fn is_test_test() { assert_eq!(false, is_test(&OsString::from("#a_dog_gif"))); assert_eq!(false, is_test(&OsString::from("~a_temp_file"))); } + +#[test] +fn test_extract_llvm_version() { + assert_eq!(extract_llvm_version("8.1.2-rust"), Some(80102)); + assert_eq!(extract_llvm_version("9.0.1-rust-1.43.0-dev"), Some(90001)); + assert_eq!(extract_llvm_version("9.3.1-rust-1.43.0-dev"), Some(90301)); + assert_eq!(extract_llvm_version("10.0.0-rust"), Some(100000)); +} From 1314d31fb5974e8c32aa8d695a02d37c4ab4bd7e Mon Sep 17 00:00:00 2001 From: Lzu Tao Date: Sun, 19 Jul 2020 15:08:35 +0000 Subject: [PATCH 11/11] Rewrite extract_version_range --- src/tools/compiletest/src/header.rs | 54 +++++++++++++---------- src/tools/compiletest/src/header/tests.rs | 15 +++++++ 2 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/tools/compiletest/src/header.rs b/src/tools/compiletest/src/header.rs index 4b5c44bca875b..2ab764eb9207c 100644 --- a/src/tools/compiletest/src/header.rs +++ b/src/tools/compiletest/src/header.rs @@ -133,7 +133,10 @@ impl EarlyProps { fn ignore_gdb(config: &Config, line: &str) -> bool { if let Some(actual_version) = config.gdb_version { if let Some(rest) = line.strip_prefix("min-gdb-version:").map(str::trim) { - let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version); + let (start_ver, end_ver) = extract_version_range(rest, extract_gdb_version) + .unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if start_ver != end_ver { panic!("Expected single GDB version") @@ -143,7 +146,9 @@ impl EarlyProps { return actual_version < start_ver; } else if let Some(rest) = line.strip_prefix("ignore-gdb-version:").map(str::trim) { let (min_version, max_version) = - extract_version_range(rest, extract_gdb_version); + extract_version_range(rest, extract_gdb_version).unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if max_version < min_version { panic!("Malformed GDB version range: max < min") @@ -197,7 +202,10 @@ impl EarlyProps { } else if let Some(rest) = line.strip_prefix("ignore-llvm-version:").map(str::trim) { // Syntax is: "ignore-llvm-version: [- ]" - let (v_min, v_max) = extract_version_range(rest, extract_llvm_version); + let (v_min, v_max) = extract_version_range(rest, extract_llvm_version) + .unwrap_or_else(|| { + panic!("couldn't parse version range: {:?}", rest); + }); if v_max < v_min { panic!("Malformed LLVM version range: max < min") } @@ -940,28 +948,28 @@ pub fn extract_llvm_version(version: &str) -> Option { // tuple: ( as u32, as u32) // If the part is omitted, the second component of the tuple // is the same as . -fn extract_version_range(line: &str, parse: F) -> (u32, u32) +fn extract_version_range(line: &str, parse: F) -> Option<(u32, u32)> where F: Fn(&str) -> Option, { - let range_components = line - .split(&[' ', '-'][..]) - .filter(|word| !word.is_empty()) - .map(parse) - .skip_while(Option::is_none) - .take(3) // 3 or more = invalid, so take at most 3. - .collect::>>(); - - match *range_components { - [v] => { - let v = v.unwrap(); - (v, v) - } - [min, max] => { - let v_min = min.unwrap(); - let v_max = max.expect("Malformed version directive"); - (v_min, v_max) - } - _ => panic!("Malformed version directive"), + let mut splits = line.splitn(2, "- ").map(str::trim); + let min = splits.next().unwrap(); + if min.ends_with('-') { + return None; + } + + let max = splits.next(); + + if min.is_empty() { + return None; } + + let min = parse(min)?; + let max = match max { + Some(max) if max.is_empty() => return None, + Some(max) => parse(max)?, + _ => min, + }; + + Some((min, max)) } diff --git a/src/tools/compiletest/src/header/tests.rs b/src/tools/compiletest/src/header/tests.rs index adcd8691d0f58..1f82b137ee6cf 100644 --- a/src/tools/compiletest/src/header/tests.rs +++ b/src/tools/compiletest/src/header/tests.rs @@ -220,3 +220,18 @@ fn sanitizers() { assert!(parse_rs(&config, "// needs-sanitizer-memory").ignore); assert!(parse_rs(&config, "// needs-sanitizer-thread").ignore); } + +#[test] +fn test_extract_version_range() { + use super::{extract_llvm_version, extract_version_range}; + + assert_eq!(extract_version_range("1.2.3 - 4.5.6", extract_llvm_version), Some((10203, 40506))); + assert_eq!(extract_version_range("0 - 4.5.6", extract_llvm_version), Some((0, 40506))); + assert_eq!(extract_version_range("1.2.3 -", extract_llvm_version), None); + assert_eq!(extract_version_range("1.2.3 - ", extract_llvm_version), None); + assert_eq!(extract_version_range("- 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range("-", extract_llvm_version), None); + assert_eq!(extract_version_range(" - 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range(" - 4.5.6", extract_llvm_version), None); + assert_eq!(extract_version_range("0 -", extract_llvm_version), None); +}