Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commit 6db5f33

Browse files
Rollup merge of #132033 - Zalathar:directive-line, r=jieyouxu
compiletest: Make `line_directive` return a `DirectiveLine` This reduces the need to juggle raw tuples, and opens up the possibility of moving more parts of directive parsing into `line_directive`. In order to make the main change possible, this PR also (partly) separates the debugger-command parsing from the main directive parser. That cleanup removes support for `[rev]` in debugger commands, which is not used by any tests.
2 parents f5aa456 + 997b7a6 commit 6db5f33

File tree

3 files changed

+56
-65
lines changed

3 files changed

+56
-65
lines changed

‎src/tools/compiletest/src/header.rs‎

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ impl EarlyProps {
5757
&mut poisoned,
5858
testfile,
5959
rdr,
60-
&mut |DirectiveLine { directive: ln, .. }| {
60+
&mut |DirectiveLine { raw_directive: ln, .. }| {
6161
parse_and_update_aux(config, ln, &mut props.aux);
6262
config.parse_and_update_revisions(testfile, ln, &mut props.revisions);
6363
},
@@ -344,8 +344,8 @@ impl TestProps {
344344
&mut poisoned,
345345
testfile,
346346
file,
347-
&mut |DirectiveLine { header_revision,directive: ln, .. }| {
348-
if header_revision.is_some() && header_revision != test_revision {
347+
&mut |directive @ DirectiveLine { raw_directive: ln, .. }| {
348+
if !directive.applies_to_test_revision(test_revision) {
349349
return;
350350
}
351351

@@ -678,28 +678,35 @@ impl TestProps {
678678
}
679679
}
680680

681-
/// Extract an `(Option<line_revision>, directive)` directive from a line if comment is present.
682-
///
683-
/// See [`DirectiveLine`] for a diagram.
684-
pubfnline_directive<'line>(
681+
/// If the given line begins with the appropriate comment prefix for a directive,
682+
/// returns a struct containing various parts of the directive.
683+
fnline_directive<'line>(
684+
line_number:usize,
685685
comment: &str,
686686
original_line: &'line str,
687-
) -> Option<(Option<&'linestr>,&'linestr)> {
687+
) -> Option<DirectiveLine<'line>> {
688688
// Ignore lines that don't start with the comment prefix.
689689
let after_comment = original_line.trim_start().strip_prefix(comment)?.trim_start();
690690

691+
let revision;
692+
let raw_directive;
693+
691694
if let Some(after_open_bracket) = after_comment.strip_prefix('[') {
692695
// A comment like `//@[foo]` only applies to revision `foo`.
693-
let Some((line_revision, directive)) = after_open_bracket.split_once(']') else {
696+
let Some((line_revision, after_close_bracket)) = after_open_bracket.split_once(']') else {
694697
panic!(
695698
"malformed condition directive: expected `{comment}[foo]`, found `{original_line}`"
696699
)
697700
};
698701

699-
Some((Some(line_revision), directive.trim_start()))
702+
revision = Some(line_revision);
703+
raw_directive = after_close_bracket.trim_start();
700704
} else {
701-
Some((None, after_comment))
702-
}
705+
revision = None;
706+
raw_directive = after_comment;
707+
};
708+
709+
Some(DirectiveLine { line_number, revision, raw_directive })
703710
}
704711

705712
// To prevent duplicating the list of commmands between `compiletest`,`htmldocck` and `jsondocck`,
@@ -730,28 +737,37 @@ const KNOWN_HTMLDOCCK_DIRECTIVE_NAMES: &[&str] = &[
730737
const KNOWN_JSONDOCCK_DIRECTIVE_NAMES: &[&str] =
731738
&["count", "!count", "has", "!has", "is", "!is", "ismany", "!ismany", "set", "!set"];
732739

733-
/// The broken-down contents of a line containing a test header directive,
740+
/// The (partly) broken-down contents of a line containing a test directive,
734741
/// which [`iter_header`] passes to its callback function.
735742
///
736743
/// For example:
737744
///
738745
/// ```text
739746
/// //@ compile-flags: -O
740-
/// ^^^^^^^^^^^^^^^^^ directive
747+
/// ^^^^^^^^^^^^^^^^^ raw_directive
741748
///
742749
/// //@ [foo] compile-flags: -O
743-
/// ^^^ header_revision
744-
/// ^^^^^^^^^^^^^^^^^ directive
750+
/// ^^^ revision
751+
/// ^^^^^^^^^^^^^^^^^ raw_directive
745752
/// ```
746753
struct DirectiveLine<'ln> {
747754
line_number: usize,
748-
/// Some header directives start with a revision name in square brackets
755+
/// Some test directives start with a revision name in square brackets
749756
/// (e.g. `[foo]`), and only apply to that revision of the test.
750757
/// If present, this field contains the revision name (e.g. `foo`).
751-
header_revision: Option<&'ln str>,
752-
/// The main part of the header directive, after removing the comment prefix
758+
revision: Option<&'ln str>,
759+
/// The main part of the directive, after removing the comment prefix
753760
/// and the optional revision specifier.
754-
directive: &'ln str,
761+
///
762+
/// This is "raw" because the directive's name and colon-separated value
763+
/// (if present) have not yet been extracted or checked.
764+
raw_directive: &'ln str,
765+
}
766+
767+
impl<'ln> DirectiveLine<'ln> {
768+
fn applies_to_test_revision(&self, test_revision: Option<&str>) -> bool {
769+
self.revision.is_none() || self.revision == test_revision
770+
}
755771
}
756772

757773
pub(crate) struct CheckDirectiveResult<'ln> {
@@ -819,8 +835,8 @@ fn iter_header(
819835
"ignore-cross-compile",
820836
];
821837
// Process the extra implied directives, with a dummy line number of 0.
822-
for directive in extra_directives {
823-
it(DirectiveLine { line_number: 0, header_revision: None, directive });
838+
for raw_directive in extra_directives {
839+
it(DirectiveLine { line_number: 0, revision: None, raw_directive });
824840
}
825841
}
826842

@@ -847,24 +863,21 @@ fn iter_header(
847863
return;
848864
}
849865

850-
let Some((header_revision, non_revisioned_directive_line)) = line_directive(comment, ln)
851-
else {
866+
let Some(directive_line) = line_directive(line_number, comment, ln) else {
852867
continue;
853868
};
854869

855870
// Perform unknown directive check on Rust files.
856871
if testfile.extension().map(|e| e == "rs").unwrap_or(false) {
857-
let directive_ln = non_revisioned_directive_line.trim();
858-
859872
let CheckDirectiveResult { is_known_directive, trailing_directive } =
860-
check_directive(directive_ln, mode, ln);
873+
check_directive(directive_line.raw_directive, mode, ln);
861874

862875
if !is_known_directive {
863876
*poisoned = true;
864877

865878
eprintln!(
866879
"error: detected unknown compiletest test directive `{}` in {}:{}",
867-
directive_ln,
880+
directive_line.raw_directive,
868881
testfile.display(),
869882
line_number,
870883
);
@@ -888,11 +901,7 @@ fn iter_header(
888901
}
889902
}
890903

891-
it(DirectiveLine {
892-
line_number,
893-
header_revision,
894-
directive: non_revisioned_directive_line,
895-
});
904+
it(directive_line);
896905
}
897906
}
898907

@@ -1292,8 +1301,8 @@ pub fn make_test_description<R: Read>(
12921301
&mut local_poisoned,
12931302
path,
12941303
src,
1295-
&mut |DirectiveLine { header_revision,directive: ln, line_number }| {
1296-
if header_revision.is_some() && header_revision != test_revision {
1304+
&mut |directive @ DirectiveLine { line_number,raw_directive: ln, .. }| {
1305+
if !directive.applies_to_test_revision(test_revision) {
12971306
return;
12981307
}
12991308

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ use std::io::{BufRead, BufReader};
44
use std::path::{Path, PathBuf};
55

66
use crate::common::Config;
7-
use crate::header::line_directive;
87
use crate::runtest::ProcRes;
98

109
/// Representation of information to invoke a debugger and check its output
@@ -24,7 +23,6 @@ impl DebuggerCommands {
2423
file: &Path,
2524
config: &Config,
2625
debugger_prefixes: &[&str],
27-
rev: Option<&str>,
2826
) -> Result<Self, String> {
2927
let directives = debugger_prefixes
3028
.iter()
@@ -39,18 +37,17 @@ impl DebuggerCommands {
3937
for (line_no, line) in reader.lines().enumerate() {
4038
counter += 1;
4139
let line = line.map_err(|e| format!("Error while parsing debugger commands: {}", e))?;
42-
let (lnrev, line) = line_directive("//", &line).unwrap_or((None, &line));
43-
44-
// Skip any revision specific directive that doesn't match the current
45-
// revision being tested
46-
if lnrev.is_some() && lnrev != rev {
47-
continue;
48-
}
4940

41+
// Breakpoints appear on lines with actual code, typically at the end of the line.
5042
if line.contains("#break") {
5143
breakpoint_lines.push(counter);
44+
continue;
5245
}
5346

47+
let Some(line) = line.trim_start().strip_prefix("//").map(str::trim_start) else {
48+
continue;
49+
};
50+
5451
for &(ref command_directive, ref check_directive) in &directives {
5552
config
5653
.parse_name_value_directive(&line, command_directive)

‎src/tools/compiletest/src/runtest/debuginfo.rs‎

Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -66,13 +66,8 @@ impl TestCx<'_> {
6666
};
6767

6868
// Parse debugger commands etc from test files
69-
let dbg_cmds = DebuggerCommands::parse_from(
70-
&self.testpaths.file,
71-
self.config,
72-
prefixes,
73-
self.revision,
74-
)
75-
.unwrap_or_else(|e| self.fatal(&e));
69+
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, prefixes)
70+
.unwrap_or_else(|e| self.fatal(&e));
7671

7772
// https://docs.microsoft.com/en-us/windows-hardware/drivers/debugger/debugger-commands
7873
let mut script_str = String::with_capacity(2048);
@@ -142,13 +137,8 @@ impl TestCx<'_> {
142137
}
143138

144139
fn run_debuginfo_gdb_test_no_opt(&self) {
145-
let dbg_cmds = DebuggerCommands::parse_from(
146-
&self.testpaths.file,
147-
self.config,
148-
&["gdb"],
149-
self.revision,
150-
)
151-
.unwrap_or_else(|e| self.fatal(&e));
140+
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, &["gdb"])
141+
.unwrap_or_else(|e| self.fatal(&e));
152142
let mut cmds = dbg_cmds.commands.join("\n");
153143

154144
// compile test file (it should have 'compile-flags:-g' in the header)
@@ -413,13 +403,8 @@ impl TestCx<'_> {
413403
}
414404

415405
// Parse debugger commands etc from test files
416-
let dbg_cmds = DebuggerCommands::parse_from(
417-
&self.testpaths.file,
418-
self.config,
419-
&["lldb"],
420-
self.revision,
421-
)
422-
.unwrap_or_else(|e| self.fatal(&e));
406+
let dbg_cmds = DebuggerCommands::parse_from(&self.testpaths.file, self.config, &["lldb"])
407+
.unwrap_or_else(|e| self.fatal(&e));
423408

424409
// Write debugger script:
425410
// We don't want to hang when calling `quit` while the process is still running

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /