tests/debuginfo/basic-stepping.rs: Add revisions default-mir-passes, no-SingleUseConsts-mir-pass
#147426
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rustbot
added
S-waiting-on-review
Collaborator
|
rustbot has assigned @Mark-Simulacrum. Use |
This comment has been minimized.
This comment has been minimized.
Enselic
force-pushed
the
better-stepping
branch
from
b0564e3 to
5b47948
Compare
This was referenced Oct 7, 2025
Closed
Member
|
r? @saethlin I'm personally dubious on disabling MIR passes in debuginfo tests... but I don't have the context here to really evaluate the tradeoffs. |
Member
Author
|
If we don't disable the pass, we can get other accidental regressions. If we disable the pass we at least know the situation will not get worse. And that risk of additional regressions is high since it already happened at least twice in the past. Long term we should do a proper fix of course. |
Member
|
I'm very confused by the way you are describing this. If there are actual regressions happening in the behavior of the compiler, then what this PR does doesn't fix them. All it does is make sure the test suite keeps passing, which would make it easier for us to ship a regression. |
saethlin
added
S-waiting-on-author
Collaborator
|
☔ The latest upstream changes (presumably #146414) made this pull request unmergeable. Please resolve the merge conflicts. |
1 task
bors
added a commit
that referenced
this pull request
Nov 14, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in #144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing #33013. CC #147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
bors
added a commit
that referenced
this pull request
Nov 14, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in #144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing #33013. CC #147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
github-actions bot
pushed a commit
to rust-lang/miri
that referenced
this pull request
Nov 17, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in rust-lang/rust#144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae92dc76d9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing rust-lang/rust#33013. CC rust-lang/rust#147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
bors
added a commit
that referenced
this pull request
Nov 26, 2025
…thlin compiletest: Use `//@` prefixes also for debuginfo test directives So that when we later add support for revisions we can use the same syntax for revisions as elsewhere (for #147426). This also prevents people from making typos for commands since `src/tools/compiletest/src/directives/directive_names.rs` will catch such typos now. Note that we add one FIXME for a non-trivial change for later: ``` // FIXME(#148097): Change `// cdb-checksimple_closure` to `//@ cdb-check:simple_closure` ``` ### TODO - [x] Triple-check that all tests still run and all directives are still applied. Done: #147799 (comment) ### Zulip discussion https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/.2F.2F.40.20syntax.20for.20debuginfo.20tests/with/545015582
bors
added a commit
that referenced
this pull request
Nov 26, 2025
…thlin compiletest: Use `//@` prefixes also for debuginfo test directives So that when we later add support for revisions we can use the same syntax for revisions as elsewhere (for #147426). This also prevents people from making typos for commands since `src/tools/compiletest/src/directives/directive_names.rs` will catch such typos now. Note that we add one FIXME for a non-trivial change for later: ``` // FIXME(#148097): Change `// cdb-checksimple_closure` to `//@ cdb-check:simple_closure` ``` ### TODO - [x] Triple-check that all tests still run and all directives are still applied. Done: #147799 (comment) ### Zulip discussion https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/.2F.2F.40.20syntax.20for.20debuginfo.20tests/with/545015582
github-actions bot
pushed a commit
to rust-lang/compiler-builtins
that referenced
this pull request
Nov 27, 2025
…thlin compiletest: Use `//@` prefixes also for debuginfo test directives So that when we later add support for revisions we can use the same syntax for revisions as elsewhere (for rust-lang/rust#147426). This also prevents people from making typos for commands since `src/tools/compiletest/src/directives/directive_names.rs` will catch such typos now. Note that we add one FIXME for a non-trivial change for later: ``` // FIXME(#148097): Change `// cdb-checksimple_closure` to `//@ cdb-check:simple_closure` ``` ### TODO - [x] Triple-check that all tests still run and all directives are still applied. Done: rust-lang/rust#147799 (comment) ### Zulip discussion https://rust-lang.zulipchat.com/#narrow/channel/326414-t-infra.2Fbootstrap/topic/.2F.2F.40.20syntax.20for.20debuginfo.20tests/with/545015582
3 tasks
Kobzol
pushed a commit
to Kobzol/rustc_codegen_gcc
that referenced
this pull request
Dec 21, 2025
rustc_codegen_llvm: Require `opt-level >= 1` for index-based write_operand_repeatedly() loop To make debugger stepping intuitive with `-Copt-level=0`. See the adjusted `basic-stepping.rs` test. This is kind of a revert of **bd0aae92dc76d9 (cg_llvm: use index-based loop in write_operand_repeatedly)**, except we don't revert it, we just make it conditional on `opt-level`. That commit regressed `basic-stepping.rs`, but it was not noticed since that test did not exist back then (it was added later in rust-lang/rust#144876). I have retroactively bisected to find that out. It seems messy to sprinkle if-cases inside of `write_operand_repeatedly()` so make the whole function conditional. The test that bd0aae92dc76d9 added in `tests/codegen/issues/issue-111603.rs` already use `-Copt-level=3`, so we don't need to adjust the compiler flags for it to keep passing. This PR takes us one step closer to fixing rust-lang/rust#33013. CC rust-lang/rust#147426 which is related (there will be trivial conflicts for me to resolve in basic-stepping.rs once one of them lands)
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)
Zalathar
added a commit
to Zalathar/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)
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)
…nore-*` We want directives nice and tidy.
…, `no-SingleUseConsts-mir-pass` To prevent regressions our test must cover the code both inside and outside of the `SingleUseConsts` MIR pass. Use revisions for that.
Enselic
force-pushed
the
better-stepping
branch
from
5b47948 to
622572f
Compare
Collaborator
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
Enselic
changed the title
default-mir-passes, no-SingleUseConsts-mir-pass
Member
Author
|
You are completely right. We need to test both with and without Now that we have revisions support from #150201 I have added revisions to the test and updated the PR description. I will investigate what happens if we only apply @rustbot ready |
rustbot
added
S-waiting-on-review
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To prevent regressions our test must cover the code both inside and outside of the
SingleUseConstsMIR pass. Use revisions for that.We know this use case is sensitive to regressions because it already happened at least once. See #33013 (comment).
CC #130896