Skip to content

Commit 5b68883

Browse files
committed
compiletest: Add LineNumber newtype to avoid +1 magic here and there
Start small. If it works well we can increase usage bit by bit as time passes.
1 parent b53da99 commit 5b68883

File tree

6 files changed

+52
-18
lines changed

6 files changed

+52
-18
lines changed

src/tools/compiletest/src/directives.rs

Lines changed: 33 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,35 @@ mod needs;
3333
#[cfg(test)]
3434
mod tests;
3535

36+
/// A line number in a file. Internally the first line has index 1.
37+
/// If it is 0 it means "no specific line" (used e.g. for implied directives).
38+
/// When displayed, the first line is 1.
39+
pub(crate) struct LineNumber(usize);
40+
41+
impl LineNumber {
42+
/// Create a LineNumber from a zero-based line index. I.e. if `zero_based`
43+
/// is `0` it means "the first line".
44+
pub(crate) fn from_zero_based(zero_based: usize) -> Self {
45+
// Ensure to panic on overflow.
46+
LineNumber(zero_based.strict_add(1))
47+
}
48+
49+
pub(crate) fn from_one_based(one_based: usize) -> LineNumber {
50+
assert!(one_based > 0);
51+
LineNumber(one_based)
52+
}
53+
54+
pub(crate) fn none() -> LineNumber {
55+
LineNumber(0)
56+
}
57+
}
58+
59+
impl std::fmt::Display for LineNumber {
60+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
61+
write!(f, "{}", self.0)
62+
}
63+
}
64+
3665
pub struct DirectivesCache {
3766
/// "Conditions" used by `ignore-*` and `only-*` directives, prepared in
3867
/// advance so that they don't have to be evaluated repeatedly.
@@ -591,7 +620,7 @@ fn iter_directives(
591620
];
592621
// Process the extra implied directives, with a dummy line number of 0.
593622
for directive_str in extra_directives {
594-
let directive_line = line_directive(testfile, 0, directive_str)
623+
let directive_line = line_directive(testfile, LineNumber::none(), directive_str)
595624
.unwrap_or_else(|| panic!("bad extra-directive line: {directive_str:?}"));
596625
it(&directive_line);
597626
}
@@ -703,7 +732,7 @@ impl Config {
703732
line: &DirectiveLine<'_>,
704733
directive: &str,
705734
) -> Option<String> {
706-
let &DirectiveLine { file_path, line_number, .. } = line;
735+
let &DirectiveLine { file_path, ref line_number, .. } = line;
707736

708737
if line.name != directive {
709738
return None;
@@ -938,7 +967,7 @@ pub(crate) fn make_test_description(
938967
iter_directives(
939968
config.mode,
940969
file_directives,
941-
&mut |ln @ &DirectiveLine { line_number, .. }| {
970+
&mut |ln @ &DirectiveLine { ref line_number, .. }| {
942971
if !ln.applies_to_test_revision(test_revision) {
943972
return;
944973
}
@@ -1262,7 +1291,7 @@ enum IgnoreDecision {
12621291

12631292
fn parse_edition_range(config: &Config, line: &DirectiveLine<'_>) -> Option<EditionRange> {
12641293
let raw = config.parse_name_value_directive(line, "edition")?;
1265-
let &DirectiveLine { file_path: testfile, line_number, .. } = line;
1294+
let &DirectiveLine { file_path: testfile, ref line_number, .. } = line;
12661295

12671296
// Edition range is half-open: `[lower_bound, upper_bound)`
12681297
if let Some((lower_bound, upper_bound)) = raw.split_once("..") {

src/tools/compiletest/src/directives/file.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
use camino::Utf8Path;
22

3+
use crate::directives::LineNumber;
34
use crate::directives::line::{DirectiveLine, line_directive};
45

56
pub(crate) struct FileDirectives<'a> {
@@ -12,6 +13,8 @@ impl<'a> FileDirectives<'a> {
1213
let mut lines = vec![];
1314

1415
for (line_number, ln) in (1..).zip(file_contents.lines()) {
16+
let line_number = LineNumber::from_one_based(line_number);
17+
1518
let ln = ln.trim();
1619

1720
if let Some(directive_line) = line_directive(path, line_number, ln) {

src/tools/compiletest/src/directives/line.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,15 @@ use std::fmt;
22

33
use camino::Utf8Path;
44

5+
use crate::directives::LineNumber;
6+
57
const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@";
68

79
/// If the given line begins with the appropriate comment prefix for a directive,
810
/// returns a struct containing various parts of the directive.
911
pub(crate) fn line_directive<'a>(
1012
file_path: &'a Utf8Path,
11-
line_number: usize,
13+
line_number: LineNumber,
1214
original_line: &'a str,
1315
) -> Option<DirectiveLine<'a>> {
1416
// Ignore lines that don't start with the comment prefix.
@@ -60,7 +62,7 @@ pub(crate) struct DirectiveLine<'a> {
6062
/// Mostly used for diagnostics, but some directives (e.g. `//@ pp-exact`)
6163
/// also use it to compute a value based on the filename.
6264
pub(crate) file_path: &'a Utf8Path,
63-
pub(crate) line_number: usize,
65+
pub(crate) line_number: LineNumber,
6466

6567
/// Some test directives start with a revision name in square brackets
6668
/// (e.g. `[foo]`), and only apply to that revision of the test.

src/tools/compiletest/src/directives/tests.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use semver::Version;
66
use crate::common::{Config, Debugger, TestMode};
77
use crate::directives::{
88
self, AuxProps, DIRECTIVE_HANDLERS_MAP, DirectivesCache, EarlyProps, Edition, EditionRange,
9-
FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, extract_llvm_version, extract_version_range,
10-
line_directive, parse_edition, parse_normalize_rule,
9+
FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, extract_llvm_version,
10+
extract_version_range, line_directive, parse_edition, parse_normalize_rule,
1111
};
1212
use crate::executor::{CollectedTestDesc, ShouldFail};
1313

@@ -1000,7 +1000,8 @@ fn parse_edition_range(line: &str) -> Option<EditionRange> {
10001000
let config = cfg().build();
10011001

10021002
let line_with_comment = format!("//@ {line}");
1003-
let line = line_directive(Utf8Path::new("tmp.rs"), 0, &line_with_comment).unwrap();
1003+
let line =
1004+
line_directive(Utf8Path::new("tmp.rs"), LineNumber::none(), &line_with_comment).unwrap();
10041005

10051006
super::parse_edition_range(&config, &line)
10061007
}

src/tools/compiletest/src/lib.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ mod edition;
1212
mod errors;
1313
mod executor;
1414
mod json;
15+
mod line_no;
1516
mod output_capture;
1617
mod panic_hook;
1718
mod raise_fd_limit;

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,17 @@ use std::io::{BufRead, BufReader};
44

55
use camino::{Utf8Path, Utf8PathBuf};
66

7+
use crate::directives::LineNumber;
78
use crate::runtest::ProcRes;
89

910
/// Representation of information to invoke a debugger and check its output
1011
pub(super) struct DebuggerCommands {
1112
/// Commands for the debuuger
1213
pub commands: Vec<String>,
1314
/// Lines to insert breakpoints at
14-
pub breakpoint_lines: Vec<usize>,
15+
pub breakpoint_lines: Vec<LineNumber>,
1516
/// Contains the source line number to check and the line itself
16-
check_lines: Vec<(usize, String)>,
17+
check_lines: Vec<(LineNumber, String)>,
1718
/// Source file name
1819
file: Utf8PathBuf,
1920
}
@@ -26,15 +27,13 @@ impl DebuggerCommands {
2627
let mut breakpoint_lines = vec![];
2728
let mut commands = vec![];
2829
let mut check_lines = vec![];
29-
let mut counter = 0;
3030
let reader = BufReader::new(File::open(file.as_std_path()).unwrap());
3131
for (line_no, line) in reader.lines().enumerate() {
32-
counter += 1;
3332
let line = line.map_err(|e| format!("Error while parsing debugger commands: {}", e))?;
3433

3534
// Breakpoints appear on lines with actual code, typically at the end of the line.
3635
if line.contains("#break") {
37-
breakpoint_lines.push(counter);
36+
breakpoint_lines.push(LineNumber::from_zero_based(line_no));
3837
continue;
3938
}
4039

@@ -46,7 +45,7 @@ impl DebuggerCommands {
4645
commands.push(command);
4746
}
4847
if let Some(pattern) = parse_name_value(&line, &check_directive) {
49-
check_lines.push((line_no, pattern));
48+
check_lines.push((LineNumber::from_zero_based(line_no), pattern));
5049
}
5150
}
5251

@@ -88,15 +87,14 @@ impl DebuggerCommands {
8887
);
8988

9089
for (src_lineno, err_line) in missing {
91-
write!(msg, "\n ({fname}:{num}) `{err_line}`", num = src_lineno + 1).unwrap();
90+
write!(msg, "\n ({fname}:{src_lineno}) `{err_line}`").unwrap();
9291
}
9392

9493
if !found.is_empty() {
9594
let init = "\nthe following subset of check directive(s) was found successfully:";
9695
msg.push_str(init);
9796
for (src_lineno, found_line) in found {
98-
write!(msg, "\n ({fname}:{num}) `{found_line}`", num = src_lineno + 1)
99-
.unwrap();
97+
write!(msg, "\n ({fname}:{src_lineno}) `{found_line}`").unwrap();
10098
}
10199
}
102100

0 commit comments

Comments
 (0)