Skip to content

Conversation

Muscraft
Copy link
Member

annotate-snippets does not have a "UI test" mode like rustc, where the code offset is not subtracted from the column width. This makes it so annotate-snippets will shift the output for some very long tests 5 - 7 columns to the left. As part of my work to have rustc use annotate-snippets, and to reduce the test differences between the two, I figured it would be best if rustc started subtracting the code offset from the width as well.

The first commit exists to keep the test output changes of adding a new line to a test separate from adding the --diagnostic-width flag in the second commit. This makes it easier to verify that adding the flag does not affect the test's output.

Zulip discussion

@rustbot
Copy link
Collaborator

rustbot commented Sep 29, 2025

The Miri subtree was changed

cc @rust-lang/miri

Some changes occurred in src/tools/clippy

cc @rust-lang/clippy

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-clippy Relevant to the Clippy team. labels Sep 29, 2025
@rustbot rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Sep 29, 2025

r? @nnethercote

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

@RalfJung
Copy link
Member

RalfJung commented Sep 29, 2025

From a test suite maintenance perspective this looks not great -- how does one know whether one has to add a random --diagnostic-width=145 to a Miri test? What happens if I don't add that?

@Muscraft
Copy link
Member Author

Personally, I would only add --diagnostic-width to tests that explicitly test how width affects the output. I only added --diagnostic-width to the tests in this PR to keep them the same as before. Without --diagnostic-width the miri tests would look:

error: Undefined Behavior: `simd_cast` intrinsic called on 3.40282347E+38f32 which cannot be represented in target type `i32`
  --> tests/fail/intrinsics/simd-float-to-int.rs:LL:CC
   |
LL | ...   let _x: i32x2 = f32x2::from_array([f32::MAX, f32::MIN]).to_int_unchecked();
   |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `main` at tests/fail/intrinsics/simd-float-to-int.rs:LL:CC
error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required
  --> tests/fail/intrinsics/copy_unaligned.rs:LL:CC
   |
LL | ...   std::intrinsics::copy_nonoverlapping(&data[5], ptr, 0);
   |       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
   |
   = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
   = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
   = note: BACKTRACE:
   = note: inside `main` at tests/fail/intrinsics/copy_unaligned.rs:LL:CC

Branch with miri output changes

@RalfJung
Copy link
Member

Without commenting on whether we want to have those output changes or not -- if the output does change like that, I'd much rather just have the stderr files updated in Miri than add random compiler flags.

@@ -1,4 +1,5 @@
//@aux-build:proc_macros.rs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for splitting this out into a separate commit!

@nnethercote
Copy link
Contributor

I'm going to pass this on to David, who already had intelligent things to say in the Zulip thread.

r? davidtwco

@rustbot rustbot assigned davidtwco and unassigned nnethercote Sep 30, 2025
@Muscraft Muscraft force-pushed the subtract-code-offset branch 2 times, most recently from dc0d2c6 to 47268e1 Compare October 1, 2025 00:40
@Muscraft
Copy link
Member Author

Muscraft commented Oct 1, 2025

I updated the PR so that the --diagnostic-width flag is only being added to tests in tests/ui/diagnostic-width.

@Muscraft Muscraft force-pushed the subtract-code-offset branch from 47268e1 to 9c6897b Compare October 2, 2025 11:45
@rustbot
Copy link
Collaborator

rustbot commented Oct 2, 2025

This PR was rebased onto a different master 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.

@davidtwco
Copy link
Member

This seems sensible to me, this has very little impact on the diagnostic output, and means that we'll be closer to matching annotate-snippets' output when we switch over to that.

@davidtwco
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Oct 6, 2025

📌 Commit 9c6897b has been approved by davidtwco

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 Oct 6, 2025
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Oct 7, 2025
…avidtwco

test: Subtract code_offset from width for ui_testing

`annotate-snippets` does not have a "UI test" mode like `rustc`, [where the code offset is not subtracted from the column width](https://github.com/rust-lang/rust/blob/f34ba774c78ea32b7c40598b8ad23e75cdac42a6/compiler/rustc_errors/src/emitter.rs#L1985-L1987). This makes it so `annotate-snippets` will shift the output for some very long tests 5 - 7 columns to the left. As part of my work to have `rustc` use `annotate-snippets`, and to reduce the test differences between the two, I figured it would be best if `rustc` started subtracting the code offset from the width as well.

The first commit exists to keep the test output changes of adding a new line to a test separate from adding the `--diagnostic-width` flag in the second commit. This makes it easier to verify that adding the flag does not affect the test's output.

[Zulip discussion](https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fdiagnostics/topic/annotate-snippets.20hurdles)
bors added a commit that referenced this pull request Oct 7, 2025
Rollup of 7 pull requests

Successful merges:

 - #145495 (Use declarative macro for `#[derive(TryFromU32)]`)
 - #147165 (test: Subtract code_offset from width for ui_testing)
 - #147354 (Fix wrong span for hightlight for duplicated diff lines)
 - #147395 (Improve diagnostics: update note and add help message)
 - #147396 (Fluent tidy improvements)
 - #147407 (Update books)
 - #147413 (don't panic on extern with just multiple quotes in the name)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4209a46 into rust-lang:master Oct 7, 2025
10 checks passed
@rustbot rustbot added this to the 1.92.0 milestone Oct 7, 2025
rust-timer added a commit that referenced this pull request Oct 7, 2025
Rollup merge of #147165 - Muscraft:subtract-code-offset, r=davidtwco

test: Subtract code_offset from width for ui_testing

`annotate-snippets` does not have a "UI test" mode like `rustc`, [where the code offset is not subtracted from the column width](https://github.com/rust-lang/rust/blob/f34ba774c78ea32b7c40598b8ad23e75cdac42a6/compiler/rustc_errors/src/emitter.rs#L1985-L1987). This makes it so `annotate-snippets` will shift the output for some very long tests 5 - 7 columns to the left. As part of my work to have `rustc` use `annotate-snippets`, and to reduce the test differences between the two, I figured it would be best if `rustc` started subtracting the code offset from the width as well.

The first commit exists to keep the test output changes of adding a new line to a test separate from adding the `--diagnostic-width` flag in the second commit. This makes it easier to verify that adding the flag does not affect the test's output.

[Zulip discussion](https://rust-lang.zulipchat.com/#narrow/channel/147480-t-compiler.2Fdiagnostics/topic/annotate-snippets.20hurdles)
github-actions bot pushed a commit to rust-lang/miri that referenced this pull request Oct 8, 2025
Rollup of 7 pull requests

Successful merges:

 - rust-lang/rust#145495 (Use declarative macro for `#[derive(TryFromU32)]`)
 - rust-lang/rust#147165 (test: Subtract code_offset from width for ui_testing)
 - rust-lang/rust#147354 (Fix wrong span for hightlight for duplicated diff lines)
 - rust-lang/rust#147395 (Improve diagnostics: update note and add help message)
 - rust-lang/rust#147396 (Fluent tidy improvements)
 - rust-lang/rust#147407 (Update books)
 - rust-lang/rust#147413 (don't panic on extern with just multiple quotes in the name)

r? `@ghost`
`@rustbot` modify labels: rollup
@Muscraft Muscraft deleted the subtract-code-offset branch October 8, 2025 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-clippy Relevant to the Clippy team. 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.

6 participants