Rollup merge of #89691 - Nicholas-Baron:debugger_commands, r=Mark-Simulacrum

Move `DebuggerCommands` and `check_debugger_output` to a separate module

Work towards #89475.

As part of this move, the public functions were changed to return `Result`. This is so that the error handling that initially took `&self: TestCx` can still use that `TestCx`.

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::{check_debugger_output, DebuggerCommands};
+
 #[cfg(test)]
 mod tests;
 
@@ -200,12 +203,6 @@ struct TestCx<'test> {
     revision: Option<&'test str>,
 }
 
-struct DebuggerCommands {
-    commands: Vec<String>,
-    check_lines: Vec<String>,
-    breakpoint_lines: Vec<usize>,
-}
-
 enum ReadFrom {
     Path,
     Stdin(String),
@@ -235,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.props.should_ice && self.config.mode != Incremental {
-            if self.config.mode != Incremental {
+            self.fatal("cannot use should-ice in a test that is not cfail");
-                self.fatal("cannot use should-ice in a test that is not cfail");
-            }
         }
         match self.config.mode {
             RunPassValgrind => self.run_valgrind_test(),
@@ -674,7 +669,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);
@@ -726,7 +724,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) {
@@ -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)
@@ -960,7 +963,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) {
@@ -1018,7 +1023,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
@@ -1094,7 +1102,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(
@@ -1131,45 +1141,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::<Vec<_>>();
-
-        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<String>) -> Option<String> {
         if options.is_none() {
             return None;
@@ -1216,66 +1187,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,
@@ -2154,9 +2065,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!("------------------------------------------");
         }
@@ -3249,11 +3160,10 @@ impl<'test> TestCx<'test> {
                 if !proc_res.status.success() {
                     self.fatal_proc_rec("test run failed!", &proc_res);
                 }
-            } else {
+            } else if proc_res.status.success() {
-                if proc_res.status.success() {
+                self.fatal_proc_rec("test run succeeded!", &proc_res);
-                    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);
@@ -3300,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 !res.stderr.is_empty()
-                if !json::rustfix_diagnostics_only(&res.stderr).is_empty() {
+                && !self.props.rustfix_only_machine_applicable
-                    self.fatal_proc_rec("fixed code is still producing diagnostics", &res);
+                && !json::rustfix_diagnostics_only(&res.stderr).is_empty()
-                }
+            {
+                self.fatal_proc_rec("fixed code is still producing diagnostics", &res);
             }
         }
     }

src/tools/compiletest/src/runtest/debugger.rs

@@ -0,0 +1,115 @@
+use crate::common::Config;
+use crate::runtest::ProcRes;
+
+use std::fs::File;
+use std::io::{BufRead, BufReader};
+use std::path::Path;
+
+pub(super) struct DebuggerCommands {
+    pub commands: Vec<String>,
+    pub check_lines: Vec<String>,
+    pub breakpoint_lines: Vec<usize>,
+}
+
+impl DebuggerCommands {
+    pub(super) fn parse_from(
+        file: &Path,
+        config: &Config,
+        debugger_prefixes: &[&str],
+    ) -> Result<Self, String> {
+        let directives = debugger_prefixes
+            .iter()
+            .map(|prefix| (format!("{}-command", prefix), format!("{}-check", prefix)))
+            .collect::<Vec<_>>();
+
+        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 })
+    }
+}
+
+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 }
+}