Skip to content

Conversation

@Enselic
Copy link
Member

@Enselic Enselic commented Dec 20, 2025

And start using revisions in tests/debuginfo/macro-stepping.rs to prevent regressing both with and without SingleUseConsts MIR pass.

I recommend commit-by-commit review.

TODO

CC

CC @Zalathar since you might have opinions about that I expose a helper function to reduce duplication

CC @saethlin since this is what we will use for tests/debuginfo/basic-stepping.rs in #147426 (in the same way I use it in tests/debuginfo/macro-stepping.rs here)

@rustbot rustbot added A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Dec 20, 2025
@rust-log-analyzer

This comment has been minimized.

@Enselic Enselic force-pushed the debugger-tests-revisions-2 branch from e25c6c9 to a63fee8 Compare December 20, 2025 17:13
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 21, 2025
tests/debuginfo/function-arg-initialization.rs: Stop disabling SingleUseConsts MIR pass

This test passes locally for me. Let's see if it passes CI too.

Discovered while I worked on rust-lang#150201.

CC rust-lang#128945
bors added a commit that referenced this pull request Dec 21, 2025
tests/debuginfo/function-arg-initialization.rs: Stop disabling SingleUseConsts MIR pass

This test passes locally for me. Let's see if it passes CI too.

Discovered while I worked on #150201.

CC #128945
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 21, 2025
tests/debuginfo/function-arg-initialization.rs: Stop disabling SingleUseConsts MIR pass

This test passes locally for me. Let's see if it passes CI too.

Discovered while I worked on rust-lang#150201.

CC rust-lang#128945
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 21, 2025
tests/debuginfo/function-arg-initialization.rs: Stop disabling SingleUseConsts MIR pass

This test passes locally for me. Let's see if it passes CI too.

Discovered while I worked on rust-lang#150201.

CC rust-lang#128945
rust-timer added a commit that referenced this pull request Dec 22, 2025
Rollup merge of #150210 - Enselic:run-mir-pass, r=saethlin

tests/debuginfo/function-arg-initialization.rs: Stop disabling SingleUseConsts MIR pass

This test passes locally for me. Let's see if it passes CI too.

Discovered while I worked on #150201.

CC #128945
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 23, 2025
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.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Dec 23, 2025
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.
rust-timer added a commit that referenced this pull request Dec 23, 2025
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.
…tests

This not only reduces code duplication already, it also gives us
revision parsing for free which we need in an upcoming commit.
…, `no-SingleUseConsts-mir-pass`

To prevent the test from regressing both with and without
`SingleUseConsts` MIR pass.
@Enselic Enselic force-pushed the debugger-tests-revisions-2 branch from a63fee8 to 423a8dc Compare December 23, 2025 14:35
@Enselic Enselic marked this pull request as ready for review December 23, 2025 14:36
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Dec 23, 2025
@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2025

r? @Zalathar

rustbot has assigned @Zalathar.
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

@rustbot
Copy link
Collaborator

rustbot commented Dec 23, 2025

Some changes occurred in src/tools/compiletest

cc @jieyouxu

@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Dec 23, 2025
@Zalathar
Copy link
Member

I'm a bit mixed on making debuginfo directives use DirectiveLine.

On one hand, it's clearly a a much-needed improvement to the debuginfo directive handling.

On the other hand, the reason I disconnected debuginfo from the main directive parser in the first place is that I didn't want to have to worry about the particular quirks of debuginfo directives when making overall improvements to directive parsing in general, and I fear that this might come up again in the future.

I think I'm tentatively OK with making debuginfo directives use DirectiveLine, under the condition that I might split them again in the future if necessary. But if that does happen, then at least debuginfo's separate directive parser will be a fork of some more up-to-date parsing code.

@Enselic
Copy link
Member Author

Enselic commented Jan 3, 2026

@Zalathar In case you didn't notice my 👍 I would just like to say that that sounds good to me. Is there anything else you'd like me to change?

@Zalathar
Copy link
Member

Zalathar commented Jan 4, 2026

Sorry for the confusion; I'm not used to rustbot actually assigning reviews to me.

@bors r+

@bors
Copy link
Collaborator

bors commented Jan 4, 2026

📌 Commit 423a8dc has been approved by Zalathar

It is now in the queue for this repository.

@bors 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 Jan 4, 2026
jhpratt added a commit to jhpratt/rust that referenced this pull request Jan 4, 2026
…, r=Zalathar

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

And start using revisions in `tests/debuginfo/macro-stepping.rs` to prevent regressing both with and without `SingleUseConsts` MIR pass.

I recommend commit-by-commit review.

## ~TODO~

- [x] Verify this more carefully.
- [x] Possibly do some preparatory PRs before taking this PR out of draft.
    - [x] Rebase on rust-lang#150205 once merged so we don't have to add another "`+ 1`".

## CC

CC `@Zalathar` since you might have opinions about that I expose a helper function to reduce duplication

CC `@saethlin` since this is what we will use for `tests/debuginfo/basic-stepping.rs` in rust-lang#147426 (in the same way I use it in `tests/debuginfo/macro-stepping.rs` here)
bors added a commit that referenced this pull request Jan 4, 2026
Rollup of 4 pull requests

Successful merges:

 - #150201 (compiletest: Support revisions in debuginfo (read: debugger) tests)
 - #150225 (Weekly `cargo update`)
 - #150642 (mutex.rs: remove needless-maybe-unsized bounds)
 - #150658 (Clarify panic conditions in `Iterator::last`)

r? `@ghost`
`@rustbot` modify labels: rollup
bors added a commit that referenced this pull request Jan 4, 2026
Rollup of 7 pull requests

Successful merges:

 - #150201 (compiletest: Support revisions in debuginfo (read: debugger) tests)
 - #150642 (mutex.rs: remove needless-maybe-unsized bounds)
 - #150643 (vec in-place-drop: avoid creating an intermediate slice)
 - #150650 (Forbid generic parameters in types of #[type_const] items)
 - #150658 (Clarify panic conditions in `Iterator::last`)
 - #150659 (Add missing translator resources for interface parse_cfg and parse_check_cfg)
 - #150666 (Fix ambig-unambig-ty-and-consts link)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 336c665 into rust-lang:main Jan 4, 2026
11 checks passed
@rustbot rustbot added this to the 1.94.0 milestone Jan 4, 2026
rust-timer added a commit that referenced this pull request Jan 4, 2026
Rollup merge of #150201 - Enselic:debugger-tests-revisions-2, r=Zalathar

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

And start using revisions in `tests/debuginfo/macro-stepping.rs` to prevent regressing both with and without `SingleUseConsts` MIR pass.

I recommend commit-by-commit review.

## ~TODO~

- [x] Verify this more carefully.
- [x] Possibly do some preparatory PRs before taking this PR out of draft.
    - [x] Rebase on #150205 once merged so we don't have to add another "`+ 1`".

## CC

CC ``@Zalathar`` since you might have opinions about that I expose a helper function to reduce duplication

CC ``@saethlin`` since this is what we will use for `tests/debuginfo/basic-stepping.rs` in #147426 (in the same way I use it in `tests/debuginfo/macro-stepping.rs` here)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-compiletest Area: The compiletest test runner A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants