compiletest: Add `LineNumber` newtype to avoid `+1` magic here and there #150205

Enselic · 2025-12-20T17:48:06Z

Start small. If it works well we can increase usage bit by bit as time passes.

My main motivation for doing this is to get rid of the + 1 I otherwise have to add in #150201 on this line:

                crate::directives::line::line_directive(file, zero_based_line_no + 1, &line)

But I think this is a nice general improvement by itself.

Note that we keep using "0" to represent "no specific line" because changing to Option<LineNumber> everywhere is a very noisy and significant change. That can be changed later if wanted, but let's not do it now.

rustbot · 2025-12-20T17:48:08Z

Some changes occurred in src/tools/compiletest

cc @jieyouxu

rustbot · 2025-12-20T17:48:10Z

r? @jieyouxu

rustbot has assigned @jieyouxu.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Enselic · 2025-12-22T15:34:26Z

I've done some additional verification and code cleanup and am happy with how this works and looks. Ready for review!

jieyouxu

Seems good, thanks

View changes since this review

jieyouxu · 2025-12-23T01:44:37Z

@bors r+ rollup

bors · 2025-12-23T01:44:40Z

📌 Commit 4dcb9d0 has been approved by jieyouxu

It is now in the queue for this repository.

Zalathar · 2025-12-23T02:31:12Z

src/tools/compiletest/src/directives/line_number.rs

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


These constructors seem overly complicated and unnecessary to me.

From what I can tell, every call site would be made much simpler by just calling a single LineNumber::enumerate(..) method that always starts from 1.

Yep that's a lot nicer, thanks. Done.

jieyouxu · 2025-12-23T02:48:52Z

Good point re. the constructors.
@bors r- (since not in a rollup yet).

src/tools/compiletest/src/directives/line_number.rs

jieyouxu · 2025-12-23T05:04:16Z

r? @Zalathar (off review for a few days)

src/tools/compiletest/src/directives/line_number.rs

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.

Zalathar · 2025-12-23T07:29:51Z

Thanks for the adjustments.

@bors r=Zalathar,jieyouxu

bors · 2025-12-23T07:29:53Z

📌 Commit ffbb9a5 has been approved by Zalathar,jieyouxu

It is now in the queue for this repository.

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. My main motivation for doing this is to get rid of the `+ 1` I otherwise have to add in rust-lang#150201 on this line: ```rs crate::directives::line::line_directive(file, zero_based_line_no + 1, &line) ``` But I think this is a nice general improvement by itself. Note that we keep using "0" to represent "no specific line" because changing to `Option<LineNumber>` everywhere is a very noisy and significant change. That _can_ be changed later if wanted, but let's not do it now.

…uwer Rollup of 3 pull requests Successful merges: - #150205 (compiletest: Add `LineNumber` newtype to avoid `+1` magic here and there) - #150295 (Fix compilation error in hermit-abi time.rs) - #150301 (std: remove unsupported pipe module from VEXos pal) r? `@ghost` `@rustbot` modify labels: rollup

…uwer Rollup of 6 pull requests Successful merges: - #150130 (Support syntax for one-line trait reuse) - #150205 (compiletest: Add `LineNumber` newtype to avoid `+1` magic here and there) - #150282 (move a ui test to coretests unit test) - #150295 (Fix compilation error in hermit-abi time.rs) - #150301 (std: remove unsupported pipe module from VEXos pal) - #150303 (`target_features::Stability`: tweak docs of `requires_nightly()`) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of #150205 - Enselic:line-no, r=Zalathar,jieyouxu 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. My main motivation for doing this is to get rid of the `+ 1` I otherwise have to add in #150201 on this line: ```rs crate::directives::line::line_directive(file, zero_based_line_no + 1, &line) ``` But I think this is a nice general improvement by itself. Note that we keep using "0" to represent "no specific line" because changing to `Option<LineNumber>` everywhere is a very noisy and significant change. That _can_ be changed later if wanted, but let's not do it now.

euclio · 2025-12-23T14:23:41Z

src/tools/compiletest/src/directives/line_number.rs

+
+impl LineNumber {
+    /// This represents "no specific line" (used e.g. for implied directives).
+    pub(crate) const ZERO: Self = Self(0);


Nit: could call this ANY or similar to better communicate intent. ZERO seems more like an implementation detail

rustbot assigned jieyouxu Dec 20, 2025

Enselic mentioned this pull request Dec 20, 2025

compiletest: Support revisions in debuginfo (read: debugger) tests #150201

Open

3 tasks

Enselic marked this pull request as draft December 20, 2025 18:21

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 20, 2025

Enselic force-pushed the line-no branch from 2c00db8 to 5b68883 Compare December 20, 2025 18:49

This comment has been minimized.

Sign in to view

Enselic force-pushed the line-no branch 2 times, most recently from 5a015a9 to 334900c Compare December 22, 2025 07:58

Enselic changed the title ~~compiletest: Add LineNo newtype to avoid +1 magic here and there~~ compiletest: Add LineNumber newtype to avoid +1 magic here and there Dec 22, 2025

This comment has been minimized.

Sign in to view

Enselic force-pushed the line-no branch from 334900c to 4dcb9d0 Compare December 22, 2025 13:57

jieyouxu mentioned this pull request Dec 22, 2025

tests/rustdoc-gui/setting-auto-hide-item-methods-docs.goml timed out #146587

Open

Enselic marked this pull request as ready for review December 22, 2025 15:34

This comment was marked as off-topic.

Sign in to view

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 22, 2025

jieyouxu approved these changes Dec 23, 2025

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 23, 2025

Zalathar reviewed Dec 23, 2025

View reviewed changes

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Dec 23, 2025

Zalathar reviewed Dec 23, 2025

View reviewed changes

src/tools/compiletest/src/directives/line_number.rs Outdated Show resolved Hide resolved

rustbot assigned Zalathar and unassigned jieyouxu Dec 23, 2025

This comment has been minimized.

Sign in to view

Enselic force-pushed the line-no branch from 88254d5 to bd86489 Compare December 23, 2025 06:48

Zalathar reviewed Dec 23, 2025

View reviewed changes

src/tools/compiletest/src/directives/line_number.rs Show resolved Hide resolved

Enselic force-pushed the line-no branch from bd86489 to ffbb9a5 Compare December 23, 2025 07:22

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Dec 23, 2025

JonathanBrouwer mentioned this pull request Dec 23, 2025

Rollup of 3 pull requests #150304

Closed

JonathanBrouwer mentioned this pull request Dec 23, 2025

Rollup of 6 pull requests #150306

Merged

bors merged commit c0337ca into rust-lang:main Dec 23, 2025
11 checks passed

rustbot added this to the 1.94.0 milestone Dec 23, 2025

euclio reviewed Dec 23, 2025

View reviewed changes

Uh oh!

compiletest: Add LineNumber newtype to avoid +1 magic here and there #150205

compiletest: Add LineNumber newtype to avoid +1 magic here and there #150205

Conversation

Enselic commented Dec 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Dec 20, 2025

Uh oh!

rustbot commented Dec 20, 2025

Uh oh!

This comment has been minimized.

This comment has been minimized.

Enselic commented Dec 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as off-topic.

jieyouxu left a comment • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Dec 23, 2025

Uh oh!

bors commented Dec 23, 2025

Uh oh!

Zalathar Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

Enselic Dec 23, 2025

Choose a reason for hiding this comment

Uh oh!

jieyouxu commented Dec 23, 2025

Uh oh!

Uh oh!

jieyouxu commented Dec 23, 2025

Uh oh!

This comment has been minimized.

Uh oh!

Zalathar commented Dec 23, 2025

Uh oh!

bors commented Dec 23, 2025

Uh oh!

Uh oh!

euclio Dec 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

compiletest: Add `LineNumber` newtype to avoid `+1` magic here and there #150205

compiletest: Add `LineNumber` newtype to avoid `+1` magic here and there #150205

Enselic commented Dec 20, 2025 •

edited

Loading

Enselic commented Dec 22, 2025 •

edited

Loading

jieyouxu left a comment •

edited by rustbot

Loading

euclio Dec 23, 2025 •

edited

Loading