Skip to content

Commit 4dcb9d0

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. Note that we keep using "0" to represent "no specific line" because changing to `Option<LineNumber>` everywhere is much bigger and noisier change. That can be done later if wanted.
1 parent b53da99 commit 4dcb9d0

File tree

6 files changed

+52
-15
lines changed

6 files changed

+52
-15
lines changed

src/tools/compiletest/src/directives.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ mod directive_names;
2929
mod file;
3030
mod handlers;
3131
mod line;
32+
mod line_number;
33+
pub(crate) use line_number::LineNumber;
3234
mod needs;
3335
#[cfg(test)]
3436
mod tests;
@@ -591,7 +593,7 @@ fn iter_directives(
591593
];
592594
// Process the extra implied directives, with a dummy line number of 0.
593595
for directive_str in extra_directives {
594-
let directive_line = line_directive(testfile, 0, directive_str)
596+
let directive_line = line_directive(testfile, LineNumber::none(), directive_str)
595597
.unwrap_or_else(|| panic!("bad extra-directive line: {directive_str:?}"));
596598
it(&directive_line);
597599
}

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.
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/// A line number in a file. Internally the first line has index 1.
2+
/// If it is 0 it means "no specific line" (used e.g. for implied directives).
3+
/// When displayed, the first line is 1.
4+
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
5+
pub(crate) struct LineNumber(usize);
6+
7+
impl LineNumber {
8+
/// Create a LineNumber from a zero-based line index. I.e. if `zero_based`
9+
/// is `0` it means "the first line".
10+
pub(crate) fn from_zero_based(zero_based: usize) -> Self {
11+
// Ensure to panic on overflow.
12+
LineNumber(zero_based.strict_add(1))
13+
}
14+
15+
pub(crate) fn from_one_based(one_based: usize) -> LineNumber {
16+
// You can't pass zero here. Use .none() for "no specific line".
17+
assert!(one_based > 0);
18+
LineNumber(one_based)
19+
}
20+
21+
pub(crate) fn none() -> LineNumber {
22+
LineNumber(0)
23+
}
24+
}
25+
26+
impl std::fmt::Display for LineNumber {
27+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
28+
write!(f, "{}", self.0)
29+
}
30+
}

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/runtest/debugger.rs

Lines changed: 8 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,14 @@ 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))?;
33+
let line_number = LineNumber::from_zero_based(line_no);
3434

3535
// Breakpoints appear on lines with actual code, typically at the end of the line.
3636
if line.contains("#break") {
37-
breakpoint_lines.push(counter);
37+
breakpoint_lines.push(line_number);
3838
continue;
3939
}
4040

@@ -46,7 +46,7 @@ impl DebuggerCommands {
4646
commands.push(command);
4747
}
4848
if let Some(pattern) = parse_name_value(&line, &check_directive) {
49-
check_lines.push((line_no, pattern));
49+
check_lines.push((line_number, pattern));
5050
}
5151
}
5252

@@ -88,15 +88,14 @@ impl DebuggerCommands {
8888
);
8989

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

9494
if !found.is_empty() {
9595
let init = "\nthe following subset of check directive(s) was found successfully:";
9696
msg.push_str(init);
9797
for (src_lineno, found_line) in found {
98-
write!(msg, "\n ({fname}:{num}) `{found_line}`", num = src_lineno + 1)
99-
.unwrap();
98+
write!(msg, "\n ({fname}:{src_lineno}) `{found_line}`").unwrap();
10099
}
101100
}
102101

0 commit comments

Comments
 (0)