Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compiletest: Slightly simplify the handling of debugger directive prefixes #134849

Merged
merged 2 commits into from
Dec 29, 2024
+11 −26
Merged

compiletest: Slightly simplify the handling of debugger directive prefixes #134849

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a625ddd
Remove the unused `cdbg-*` debugger directive prefix
Zalathar Dec 28, 2024
2f8824a
Simplify `DebuggerCommands::parse_from` to only take one prefix
Zalathar Dec 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 8 additions & 17 deletions src/tools/compiletest/src/runtest/debugger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,9 @@ pub(super) struct DebuggerCommands {
}

impl DebuggerCommands {
pub fn parse_from(
file: &Path,
config: &Config,
debugger_prefixes: &[&str],
) -> Result<Self, String> {
let directives = debugger_prefixes
.iter()
.map(|prefix| (format!("{prefix}-command"), format!("{prefix}-check")))
.collect::<Vec<_>>();
pub fn parse_from(file: &Path, config: &Config, debugger_prefix: &str) -> Result<Self, String> {
let command_directive = format!("{debugger_prefix}-command");
let check_directive = format!("{debugger_prefix}-check");

let mut breakpoint_lines = vec![];
let mut commands = vec![];
Expand All @@ -48,14 +42,11 @@ impl DebuggerCommands {
continue;
};

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((line_no, cmd)));
if let Some(command) = config.parse_name_value_directive(&line, &command_directive) {
commands.push(command);
}
if let Some(pattern) = config.parse_name_value_directive(&line, &check_directive) {
check_lines.push((line_no, pattern));
}
}

Expand Down
12 changes: 3 additions & 9 deletions src/tools/compiletest/src/runtest/debuginfo.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,8 @@ impl TestCx<'_> {
return;
}

let prefixes = {
static PREFIXES: &[&str] = &["cdb", "cdbg"];
// No "native rust support" variation for CDB yet.
PREFIXES
};

// Parse debugger commands etc from test files
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, prefixes)
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, "cdb")
Comment on lines -62 to +63
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some digging, this was introduced in the very original PR that added cdb support, i.e. #60970. There was no discussion about cdbg in the PR either.

Possibilities:

In any case, none of the debuginfo tests use it.

Copy link
Contributor Author

@Zalathar Zalathar Dec 28, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At that time, gdb had the prefixes [gdb, gdbr, gdbg], and similarly for lldb.

I think the idea was that r would only apply when using a Rust-aware debugger, and g would only apply when using a non-Rust-aware debugger.

I imagine that the PR author copied what the other debuggers were doing, but without the r variant as there was no Rust-aware cdb (hence the mysterious comment about no "native Rust support").

Later, the whole r/g distinction was removed from gdb/lldb by #129218. But cdb was not updated, so the useless g remained.

.unwrap_or_else(|e| self.fatal(&e));

// https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands
Expand Down Expand Up @@ -137,7 +131,7 @@ impl TestCx<'_> {
}

fn run_debuginfo_gdb_test_no_opt(&self) {
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, &["gdb"])
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, "gdb")
.unwrap_or_else(|e| self.fatal(&e));
let mut cmds = dbg_cmds.commands.join("\n");

Expand Down Expand Up @@ -403,7 +397,7 @@ impl TestCx<'_> {
}

// Parse debugger commands etc from test files
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, &["lldb"])
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, "lldb")
.unwrap_or_else(|e| self.fatal(&e));

// Write debugger script:
Expand Down
Loading