Skip to content

Conversation

@ada4a
Copy link
Contributor

@ada4a ada4a commented Nov 20, 2025

changelog: [missing_asserts_for_indexing]: overhaul diagnostics

This is a general advice, and so shouldn't be repeated
This allows printing the labels in the same diagnostic.

Note: if we were to _only_ replaces notes by labels, but still pointed
the diagnostic at `full_span`, the resulting diagnostic would end up
rather ugly:
```
error: indexing into a slice multiple times with an `assert` that does not cover the highest index
  --> tests/ui/missing_asserts_for_indexing.rs:30:5
   |
LL |     v[0] + v[1] + v[2] + v[3] + v[4]
   |     ----^^^----^^^----^^^----^^^----
   |     |      |      |      |      |
   |     |      |      |      |      slice indexed here
   |     |      |      |      slice indexed here
   |     |      |      slice indexed here
   |     |      slice indexed here
   |     slice indexed here
   |
```
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Nov 20, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 20, 2025

r? @Jarcho

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

@github-actions
Copy link

Lintcheck changes for bc76aef

Lint Added Removed Changed
clippy::missing_asserts_for_indexing 0 0 63

This comment will be updated if you push new changes

Copy link
Contributor

@Jarcho Jarcho left a comment

Choose a reason for hiding this comment

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

Just the labels need to be removed. Looks good otherwise.

View changes since this review

diag.span_note(*span, "slice indexed here");
}
diag.note("asserting the length before indexing will elide bounds checks");
diag.span_labels(index_spans, "slice indexed here");
Copy link
Contributor

Choose a reason for hiding this comment

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

This label adds more noise than anything.

Comment on lines +5 to +10
| ^^^^ ^^^^ ^^^^ ^^^^ ^^^^ slice indexed here
| | | | |
| | | | slice indexed here
| | | slice indexed here
| | slice indexed here
| slice indexed here
Copy link
Contributor

Choose a reason for hiding this comment

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

Per the previous. This is several lines cluttering the diagnostic while not adding new information.

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status) and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Nov 26, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 26, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-author Status: This is awaiting some action from the author. (Use `@rustbot ready` to update this status)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants