From 4dcb9d06131b28d48dc9345ee236dbc5c93dfe6d Mon Sep 17 00:00:00 2001 From: Martin Nordholts Date: Sat, 20 Dec 2025 18:42:41 +0100 Subject: [PATCH] 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` everywhere is much bigger and noisier change. That can be done later if wanted. --- src/tools/compiletest/src/directives.rs | 4 ++- src/tools/compiletest/src/directives/file.rs | 3 ++ src/tools/compiletest/src/directives/line.rs | 6 ++-- .../compiletest/src/directives/line_number.rs | 30 +++++++++++++++++++ src/tools/compiletest/src/directives/tests.rs | 7 +++-- src/tools/compiletest/src/runtest/debugger.rs | 17 +++++------ 6 files changed, 52 insertions(+), 15 deletions(-) create mode 100644 src/tools/compiletest/src/directives/line_number.rs diff --git a/src/tools/compiletest/src/directives.rs b/src/tools/compiletest/src/directives.rs index c154886ebcdec..77ad36852c6da 100644 --- a/src/tools/compiletest/src/directives.rs +++ b/src/tools/compiletest/src/directives.rs @@ -29,6 +29,8 @@ mod directive_names; mod file; mod handlers; mod line; +mod line_number; +pub(crate) use line_number::LineNumber; mod needs; #[cfg(test)] mod tests; @@ -591,7 +593,7 @@ fn iter_directives( ]; // Process the extra implied directives, with a dummy line number of 0. for directive_str in extra_directives { - let directive_line = line_directive(testfile, 0, directive_str) + let directive_line = line_directive(testfile, LineNumber::none(), directive_str) .unwrap_or_else(|| panic!("bad extra-directive line: {directive_str:?}")); it(&directive_line); } diff --git a/src/tools/compiletest/src/directives/file.rs b/src/tools/compiletest/src/directives/file.rs index 82819ac0c8f06..44646a8ffd5b3 100644 --- a/src/tools/compiletest/src/directives/file.rs +++ b/src/tools/compiletest/src/directives/file.rs @@ -1,5 +1,6 @@ use camino::Utf8Path; +use crate::directives::LineNumber; use crate::directives::line::{DirectiveLine, line_directive}; pub(crate) struct FileDirectives<'a> { @@ -12,6 +13,8 @@ impl<'a> FileDirectives<'a> { let mut lines = vec![]; for (line_number, ln) in (1..).zip(file_contents.lines()) { + let line_number = LineNumber::from_one_based(line_number); + let ln = ln.trim(); if let Some(directive_line) = line_directive(path, line_number, ln) { diff --git a/src/tools/compiletest/src/directives/line.rs b/src/tools/compiletest/src/directives/line.rs index 16dd9a8de1c03..9cc24c98a859d 100644 --- a/src/tools/compiletest/src/directives/line.rs +++ b/src/tools/compiletest/src/directives/line.rs @@ -2,13 +2,15 @@ use std::fmt; use camino::Utf8Path; +use crate::directives::LineNumber; + const COMPILETEST_DIRECTIVE_PREFIX: &str = "//@"; /// If the given line begins with the appropriate comment prefix for a directive, /// returns a struct containing various parts of the directive. pub(crate) fn line_directive<'a>( file_path: &'a Utf8Path, - line_number: usize, + line_number: LineNumber, original_line: &'a str, ) -> Option> { // Ignore lines that don't start with the comment prefix. @@ -60,7 +62,7 @@ pub(crate) struct DirectiveLine<'a> { /// Mostly used for diagnostics, but some directives (e.g. `//@ pp-exact`) /// also use it to compute a value based on the filename. pub(crate) file_path: &'a Utf8Path, - pub(crate) line_number: usize, + pub(crate) line_number: LineNumber, /// Some test directives start with a revision name in square brackets /// (e.g. `[foo]`), and only apply to that revision of the test. diff --git a/src/tools/compiletest/src/directives/line_number.rs b/src/tools/compiletest/src/directives/line_number.rs new file mode 100644 index 0000000000000..fa4fbb837520e --- /dev/null +++ b/src/tools/compiletest/src/directives/line_number.rs @@ -0,0 +1,30 @@ +/// A line number in a file. Internally the first line has index 1. +/// If it is 0 it means "no specific line" (used e.g. for implied directives). +/// When displayed, the first line is 1. +#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub(crate) struct LineNumber(usize); + +impl LineNumber { + /// Create a LineNumber from a zero-based line index. I.e. if `zero_based` + /// is `0` it means "the first line". + pub(crate) fn from_zero_based(zero_based: usize) -> Self { + // Ensure to panic on overflow. + LineNumber(zero_based.strict_add(1)) + } + + pub(crate) fn from_one_based(one_based: usize) -> LineNumber { + // You can't pass zero here. Use .none() for "no specific line". + assert!(one_based > 0); + LineNumber(one_based) + } + + pub(crate) fn none() -> LineNumber { + LineNumber(0) + } +} + +impl std::fmt::Display for LineNumber { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}", self.0) + } +} diff --git a/src/tools/compiletest/src/directives/tests.rs b/src/tools/compiletest/src/directives/tests.rs index 90e2cb77e304c..3d16c642afc47 100644 --- a/src/tools/compiletest/src/directives/tests.rs +++ b/src/tools/compiletest/src/directives/tests.rs @@ -6,8 +6,8 @@ use semver::Version; use crate::common::{Config, Debugger, TestMode}; use crate::directives::{ self, AuxProps, DIRECTIVE_HANDLERS_MAP, DirectivesCache, EarlyProps, Edition, EditionRange, - FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, extract_llvm_version, extract_version_range, - line_directive, parse_edition, parse_normalize_rule, + FileDirectives, KNOWN_DIRECTIVE_NAMES_SET, LineNumber, extract_llvm_version, + extract_version_range, line_directive, parse_edition, parse_normalize_rule, }; use crate::executor::{CollectedTestDesc, ShouldFail}; @@ -1000,7 +1000,8 @@ fn parse_edition_range(line: &str) -> Option { let config = cfg().build(); let line_with_comment = format!("//@ {line}"); - let line = line_directive(Utf8Path::new("tmp.rs"), 0, &line_with_comment).unwrap(); + let line = + line_directive(Utf8Path::new("tmp.rs"), LineNumber::none(), &line_with_comment).unwrap(); super::parse_edition_range(&config, &line) } diff --git a/src/tools/compiletest/src/runtest/debugger.rs b/src/tools/compiletest/src/runtest/debugger.rs index 00935ab57d1c3..6f65705a0df24 100644 --- a/src/tools/compiletest/src/runtest/debugger.rs +++ b/src/tools/compiletest/src/runtest/debugger.rs @@ -4,6 +4,7 @@ use std::io::{BufRead, BufReader}; use camino::{Utf8Path, Utf8PathBuf}; +use crate::directives::LineNumber; use crate::runtest::ProcRes; /// Representation of information to invoke a debugger and check its output @@ -11,9 +12,9 @@ pub(super) struct DebuggerCommands { /// Commands for the debuuger pub commands: Vec, /// Lines to insert breakpoints at - pub breakpoint_lines: Vec, + pub breakpoint_lines: Vec, /// Contains the source line number to check and the line itself - check_lines: Vec<(usize, String)>, + check_lines: Vec<(LineNumber, String)>, /// Source file name file: Utf8PathBuf, } @@ -26,15 +27,14 @@ impl DebuggerCommands { let mut breakpoint_lines = vec![]; let mut commands = vec![]; let mut check_lines = vec![]; - let mut counter = 0; let reader = BufReader::new(File::open(file.as_std_path()).unwrap()); for (line_no, line) in reader.lines().enumerate() { - counter += 1; let line = line.map_err(|e| format!("Error while parsing debugger commands: {}", e))?; + let line_number = LineNumber::from_zero_based(line_no); // Breakpoints appear on lines with actual code, typically at the end of the line. if line.contains("#break") { - breakpoint_lines.push(counter); + breakpoint_lines.push(line_number); continue; } @@ -46,7 +46,7 @@ impl DebuggerCommands { commands.push(command); } if let Some(pattern) = parse_name_value(&line, &check_directive) { - check_lines.push((line_no, pattern)); + check_lines.push((line_number, pattern)); } } @@ -88,15 +88,14 @@ impl DebuggerCommands { ); for (src_lineno, err_line) in missing { - write!(msg, "\n ({fname}:{num}) `{err_line}`", num = src_lineno + 1).unwrap(); + write!(msg, "\n ({fname}:{src_lineno}) `{err_line}`").unwrap(); } if !found.is_empty() { let init = "\nthe following subset of check directive(s) was found successfully:"; msg.push_str(init); for (src_lineno, found_line) in found { - write!(msg, "\n ({fname}:{num}) `{found_line}`", num = src_lineno + 1) - .unwrap(); + write!(msg, "\n ({fname}:{src_lineno}) `{found_line}`").unwrap(); } }