Skip to content

Commit 5a015a9

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 5a015a9

File tree

5 files changed

+50
-15
lines changed

5 files changed

+50
-15
lines changed

src/tools/compiletest/src/directives.rs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,36 @@ 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+
#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
40+
pub(crate) struct LineNumber(usize);
41+
42+
impl LineNumber {
43+
/// Create a LineNumber from a zero-based line index. I.e. if `zero_based`
44+
/// is `0` it means "the first line".
45+
pub(crate) fn from_zero_based(zero_based: usize) -> Self {
46+
// Ensure to panic on overflow.
47+
LineNumber(zero_based.strict_add(1))
48+
}
49+
50+
pub(crate) fn from_one_based(one_based: usize) -> LineNumber {
51+
assert!(one_based > 0);
52+
LineNumber(one_based)
53+
}
54+
55+
pub(crate) fn none() -> LineNumber {
56+
LineNumber(0)
57+
}
58+
}
59+
60+
impl std::fmt::Display for LineNumber {
61+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
62+
write!(f, "{}", self.0)
63+
}
64+
}
65+
3666
pub struct DirectivesCache {
3767
/// "Conditions" used by `ignore-*` and `only-*` directives, prepared in
3868
/// advance so that they don't have to be evaluated repeatedly.
@@ -591,7 +621,7 @@ fn iter_directives(
591621
];
592622
// Process the extra implied directives, with a dummy line number of 0.
593623
for directive_str in extra_directives {
594-
let directive_line = line_directive(testfile, 0, directive_str)
624+
let directive_line = line_directive(testfile, LineNumber::none(), directive_str)
595625
.unwrap_or_else(|| panic!("bad extra-directive line: {directive_str:?}"));
596626
it(&directive_line);
597627
}

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/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)