From ce185739fc07dd743043b6e3265ad7442846e837 Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Fri, 8 Oct 2021 18:27:44 -0700 Subject: [PATCH 1/4] Move DebuggerCommands to its own file --- src/tools/compiletest/src/runtest.rs | 63 +++++-------------- src/tools/compiletest/src/runtest/debugger.rs | 56 +++++++++++++++++ 2 files changed, 71 insertions(+), 48 deletions(-) create mode 100644 src/tools/compiletest/src/runtest/debugger.rs diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 934839bbd605b..9cc373fbad127 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -38,6 +38,9 @@ use tracing::*; use crate::extract_gdb_version; use crate::is_android_gdb_target; +mod debugger; +use debugger::DebuggerCommands; + #[cfg(test)] mod tests; @@ -200,12 +203,6 @@ struct TestCx<'test> { revision: Option<&'test str>, } -struct DebuggerCommands { - commands: Vec, - check_lines: Vec, - breakpoint_lines: Vec, -} - enum ReadFrom { Path, Stdin(String), @@ -674,7 +671,10 @@ impl<'test> TestCx<'test> { // Parse debugger commands etc from test files let DebuggerCommands { commands, check_lines, breakpoint_lines, .. } = - self.parse_debugger_commands(prefixes); + match DebuggerCommands::parse_from(&self.testpaths.file, self.config, prefixes) { + Ok(cmds) => cmds, + Err(e) => self.fatal(&e), + }; // https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands let mut script_str = String::with_capacity(2048); @@ -757,7 +757,10 @@ impl<'test> TestCx<'test> { }; let DebuggerCommands { commands, check_lines, breakpoint_lines } = - self.parse_debugger_commands(prefixes); + match DebuggerCommands::parse_from(&self.testpaths.file, self.config, prefixes) { + Ok(cmds) => cmds, + Err(e) => self.fatal(&e), + }; let mut cmds = commands.join("\n"); // compile test file (it should have 'compile-flags:-g' in the header) @@ -1018,7 +1021,10 @@ impl<'test> TestCx<'test> { // Parse debugger commands etc from test files let DebuggerCommands { commands, check_lines, breakpoint_lines, .. } = - self.parse_debugger_commands(prefixes); + match DebuggerCommands::parse_from(&self.testpaths.file, self.config, prefixes) { + Ok(cmds) => cmds, + Err(e) => self.fatal(&e), + }; // Write debugger script: // We don't want to hang when calling `quit` while the process is still running @@ -1131,45 +1137,6 @@ impl<'test> TestCx<'test> { ProcRes { status, stdout: out, stderr: err, cmdline: format!("{:?}", cmd) } } - fn parse_debugger_commands(&self, debugger_prefixes: &[&str]) -> DebuggerCommands { - let directives = debugger_prefixes - .iter() - .map(|prefix| (format!("{}-command", prefix), format!("{}-check", prefix))) - .collect::>(); - - let mut breakpoint_lines = vec![]; - let mut commands = vec![]; - let mut check_lines = vec![]; - let mut counter = 1; - let reader = BufReader::new(File::open(&self.testpaths.file).unwrap()); - for line in reader.lines() { - match line { - Ok(line) => { - let line = - if line.starts_with("//") { line[2..].trim_start() } else { line.as_str() }; - - if line.contains("#break") { - breakpoint_lines.push(counter); - } - - for &(ref command_directive, ref check_directive) in &directives { - self.config - .parse_name_value_directive(&line, command_directive) - .map(|cmd| commands.push(cmd)); - - self.config - .parse_name_value_directive(&line, check_directive) - .map(|cmd| check_lines.push(cmd)); - } - } - Err(e) => self.fatal(&format!("Error while parsing debugger commands: {}", e)), - } - counter += 1; - } - - DebuggerCommands { commands, check_lines, breakpoint_lines } - } - fn cleanup_debug_info_options(&self, options: &Option) -> Option { if options.is_none() { return None; diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs new file mode 100644 index 0000000000000..ad8b805e53c44 --- /dev/null +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -0,0 +1,56 @@ +use crate::common::Config; + +use std::fs::File; +use std::io::{BufRead, BufReader}; +use std::path::Path; + +pub(super) struct DebuggerCommands { + pub commands: Vec, + pub check_lines: Vec, + pub breakpoint_lines: Vec, +} + +impl DebuggerCommands { + pub(super) fn parse_from( + file: &Path, + config: &Config, + debugger_prefixes: &[&str], + ) -> Result { + let directives = debugger_prefixes + .iter() + .map(|prefix| (format!("{}-command", prefix), format!("{}-check", prefix))) + .collect::>(); + + let mut breakpoint_lines = vec![]; + let mut commands = vec![]; + let mut check_lines = vec![]; + let mut counter = 1; + let reader = BufReader::new(File::open(file).unwrap()); + for line in reader.lines() { + match line { + Ok(line) => { + let line = + if line.starts_with("//") { line[2..].trim_start() } else { line.as_str() }; + + if line.contains("#break") { + breakpoint_lines.push(counter); + } + + for &(ref command_directive, ref check_directive) in &directives { + config + .parse_name_value_directive(&line, command_directive) + .map(|cmd| commands.push(cmd)); + + config + .parse_name_value_directive(&line, check_directive) + .map(|cmd| check_lines.push(cmd)); + } + } + Err(e) => return Err(format!("Error while parsing debugger commands: {}", e)), + } + counter += 1; + } + + Ok(Self { commands, check_lines, breakpoint_lines }) + } +} From 0e2a782463a3ec667e1a137d3743e46549538072 Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Fri, 8 Oct 2021 18:41:40 -0700 Subject: [PATCH 2/4] Move check_debugger_output to the debugger module --- src/tools/compiletest/src/runtest.rs | 74 +++---------------- src/tools/compiletest/src/runtest/debugger.rs | 59 +++++++++++++++ 2 files changed, 69 insertions(+), 64 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 9cc373fbad127..46799bc4c1506 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -39,7 +39,7 @@ use crate::extract_gdb_version; use crate::is_android_gdb_target; mod debugger; -use debugger::DebuggerCommands; +use debugger::{check_debugger_output, DebuggerCommands}; #[cfg(test)] mod tests; @@ -726,7 +726,9 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec("Error while running CDB", &debugger_run_result); } - self.check_debugger_output(&debugger_run_result, &check_lines); + if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + self.fatal_proc_rec(&e, &debugger_run_result); + } } fn run_debuginfo_gdb_test(&self) { @@ -963,7 +965,9 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec("gdb failed to execute", &debugger_run_result); } - self.check_debugger_output(&debugger_run_result, &check_lines); + if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + self.fatal_proc_rec(&e, &debugger_run_result); + } } fn run_debuginfo_lldb_test(&self) { @@ -1100,7 +1104,9 @@ impl<'test> TestCx<'test> { self.fatal_proc_rec("Error while running LLDB", &debugger_run_result); } - self.check_debugger_output(&debugger_run_result, &check_lines); + if let Err(e) = check_debugger_output(&debugger_run_result, &check_lines) { + self.fatal_proc_rec(&e, &debugger_run_result); + } } fn run_lldb( @@ -1183,66 +1189,6 @@ impl<'test> TestCx<'test> { } } - fn check_debugger_output(&self, debugger_run_result: &ProcRes, check_lines: &[String]) { - let num_check_lines = check_lines.len(); - - let mut check_line_index = 0; - for line in debugger_run_result.stdout.lines() { - if check_line_index >= num_check_lines { - break; - } - - if check_single_line(line, &(check_lines[check_line_index])[..]) { - check_line_index += 1; - } - } - if check_line_index != num_check_lines && num_check_lines > 0 { - self.fatal_proc_rec( - &format!("line not found in debugger output: {}", check_lines[check_line_index]), - debugger_run_result, - ); - } - - fn check_single_line(line: &str, check_line: &str) -> bool { - // Allow check lines to leave parts unspecified (e.g., uninitialized - // bits in the wrong case of an enum) with the notation "[...]". - let line = line.trim(); - let check_line = check_line.trim(); - let can_start_anywhere = check_line.starts_with("[...]"); - let can_end_anywhere = check_line.ends_with("[...]"); - - let check_fragments: Vec<&str> = - check_line.split("[...]").filter(|frag| !frag.is_empty()).collect(); - if check_fragments.is_empty() { - return true; - } - - let (mut rest, first_fragment) = if can_start_anywhere { - match line.find(check_fragments[0]) { - Some(pos) => (&line[pos + check_fragments[0].len()..], 1), - None => return false, - } - } else { - (line, 0) - }; - - for current_fragment in &check_fragments[first_fragment..] { - match rest.find(current_fragment) { - Some(pos) => { - rest = &rest[pos + current_fragment.len()..]; - } - None => return false, - } - } - - if !can_end_anywhere && !rest.is_empty() { - return false; - } - - true - } - } - fn check_error_patterns( &self, output_to_check: &str, diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs index ad8b805e53c44..cbd5e4c431f56 100644 --- a/src/tools/compiletest/src/runtest/debugger.rs +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -1,4 +1,5 @@ use crate::common::Config; +use crate::runtest::ProcRes; use std::fs::File; use std::io::{BufRead, BufReader}; @@ -54,3 +55,61 @@ impl DebuggerCommands { Ok(Self { commands, check_lines, breakpoint_lines }) } } + +pub(super) fn check_debugger_output( + debugger_run_result: &ProcRes, + check_lines: &[String], +) -> Result<(), String> { + let num_check_lines = check_lines.len(); + + let mut check_line_index = 0; + for line in debugger_run_result.stdout.lines() { + if check_line_index >= num_check_lines { + break; + } + + if check_single_line(line, &(check_lines[check_line_index])[..]) { + check_line_index += 1; + } + } + if check_line_index != num_check_lines && num_check_lines > 0 { + Err(format!("line not found in debugger output: {}", check_lines[check_line_index])) + } else { + Ok(()) + } +} + +fn check_single_line(line: &str, check_line: &str) -> bool { + // Allow check lines to leave parts unspecified (e.g., uninitialized + // bits in the wrong case of an enum) with the notation "[...]". + let line = line.trim(); + let check_line = check_line.trim(); + let can_start_anywhere = check_line.starts_with("[...]"); + let can_end_anywhere = check_line.ends_with("[...]"); + + let check_fragments: Vec<&str> = + check_line.split("[...]").filter(|frag| !frag.is_empty()).collect(); + if check_fragments.is_empty() { + return true; + } + + let (mut rest, first_fragment) = if can_start_anywhere { + match line.find(check_fragments[0]) { + Some(pos) => (&line[pos + check_fragments[0].len()..], 1), + None => return false, + } + } else { + (line, 0) + }; + + for current_fragment in &check_fragments[first_fragment..] { + match rest.find(current_fragment) { + Some(pos) => { + rest = &rest[pos + current_fragment.len()..]; + } + None => return false, + } + } + + if !can_end_anywhere && !rest.is_empty() { false } else { true } +} From d2a522f4232c155bca29befc8c2800e906563c2a Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Sat, 9 Oct 2021 12:12:13 -0700 Subject: [PATCH 3/4] Simplified two printlns picked out by clippy --- src/tools/compiletest/src/runtest.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 46799bc4c1506..7e29b980ae1e7 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -2067,9 +2067,9 @@ impl<'test> TestCx<'test> { fn maybe_dump_to_stdout(&self, out: &str, err: &str) { if self.config.verbose { - println!("------{}------------------------------", "stdout"); + println!("------stdout------------------------------"); println!("{}", out); - println!("------{}------------------------------", "stderr"); + println!("------stderr------------------------------"); println!("{}", err); println!("------------------------------------------"); } From 2d827ca3b8490342f450cb205fc14c7db23b551e Mon Sep 17 00:00:00 2001 From: Nicholas-Baron Date: Sat, 9 Oct 2021 12:15:14 -0700 Subject: [PATCH 4/4] Simplified 3 ifs found by clippy. --- src/tools/compiletest/src/runtest.rs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/src/tools/compiletest/src/runtest.rs b/src/tools/compiletest/src/runtest.rs index 7e29b980ae1e7..4470272a9f866 100644 --- a/src/tools/compiletest/src/runtest.rs +++ b/src/tools/compiletest/src/runtest.rs @@ -232,10 +232,8 @@ impl<'test> TestCx<'test> { /// Code executed for each revision in turn (or, if there are no /// revisions, exactly once, with revision == None). fn run_revision(&self) { - if self.props.should_ice { - if self.config.mode != Incremental { - self.fatal("cannot use should-ice in a test that is not cfail"); - } + if self.props.should_ice && self.config.mode != Incremental { + self.fatal("cannot use should-ice in a test that is not cfail"); } match self.config.mode { RunPassValgrind => self.run_valgrind_test(), @@ -3162,11 +3160,10 @@ impl<'test> TestCx<'test> { if !proc_res.status.success() { self.fatal_proc_rec("test run failed!", &proc_res); } - } else { - if proc_res.status.success() { - self.fatal_proc_rec("test run succeeded!", &proc_res); - } + } else if proc_res.status.success() { + self.fatal_proc_rec("test run succeeded!", &proc_res); } + if !self.props.error_patterns.is_empty() { // "// error-pattern" comments self.check_error_patterns(&proc_res.stderr, &proc_res, pm); @@ -3213,10 +3210,11 @@ impl<'test> TestCx<'test> { if !res.status.success() { self.fatal_proc_rec("failed to compile fixed code", &res); } - if !res.stderr.is_empty() && !self.props.rustfix_only_machine_applicable { - if !json::rustfix_diagnostics_only(&res.stderr).is_empty() { - self.fatal_proc_rec("fixed code is still producing diagnostics", &res); - } + if !res.stderr.is_empty() + && !self.props.rustfix_only_machine_applicable + && !json::rustfix_diagnostics_only(&res.stderr).is_empty() + { + self.fatal_proc_rec("fixed code is still producing diagnostics", &res); } } }