Repository navigation
Improve diagnostics for suggestions of unstable items - #161943
KevinA-cpu wants to merge 7 commits into
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
@bors try jobs=x86_64-gnu-stable |
This comment has been minimized.
This comment has been minimized.
…rinsics-suggestions, r=<try> Improve rustc diagnostics try-job: x86_64-gnu-stable
| LL | let _: BorrowedBuf; | ||
| | ^^^^^^^^^^^ not found in this scope | ||
| | | ||
| = note: 'std::io::BorrowedBuf' is unstable in nightly Rust and is only available with the `#![feature(core_io_borrowed_buf)]` attribute |
There was a problem hiding this comment.
Could you place the note below the help: consider importing this struct? That improves the reading order
There was a problem hiding this comment.
I want to clarify @JonathanBrouwer, do you want to reorder so that the note is below the help universally, or do you only want this particular new note we're adding to be below the help?
Below is an example where a note above the help might be more useful:
= note: 'std::convert::TryFrom' is included in the prelude starting in Edition 2021
help: consider importing this traitThere was a problem hiding this comment.
I mean I want the note to be under the help universally.
The idea being that we first tell the user "you might have meant x" then "by the way, x is unstable". If you start with "x is unstable" its not obvious why the compiler is telling us that until you read the next sentence
There was a problem hiding this comment.
The note is still above the help.
That is, I want
error[E0425]: cannot find type `BorrowedBuf` in this scope
--> $DIR/import-suggestion-unstable-feature-note.rs:9:12
|
LL | let _: BorrowedBuf;
| ^^^^^^^^^^^ not found in this scope
|
= note: 'std::io::BorrowedBuf' is unstable, add `#![feature(core_io_borrowed_buf)]` to the crate attributes to enable
help: consider importing this struct
|
LL + use std::io::BorrowedBuf;
|
To change to (approximately)
error[E0425]: cannot find type `BorrowedBuf` in this scope
--> $DIR/import-suggestion-unstable-feature-note.rs:9:12
|
LL | let _: BorrowedBuf;
| ^^^^^^^^^^^ not found in this scope
|
help: consider importing this struct
|
LL + use std::io::BorrowedBuf;
|
note: 'std::io::BorrowedBuf' is unstable, add `#![feature(core_io_borrowed_buf)]` to the crate attributes to enable
There was a problem hiding this comment.
I noticed you implemented this universally, for all suggestions. I intended to limit this to only this specific diagnostic, this seems like quite a big change for this
There was a problem hiding this comment.
Yep, I got confused by
I mean I want the
noteto be under thehelpuniversally.
I will revert most of the changes, LOL
There was a problem hiding this comment.
Oh I'm sorry, the way I answered that is indeed confusing, I misunderstood your question. I meant for all places where this diagnostic is emitted
There was a problem hiding this comment.
So from my efforts and investigate, it looks like it won't be a small change to try and make the note of this diagnostics to be below the help.
The emitter in compiler/rustc_errors/src/annotate_snippet_emitter_writer.rs always prints every child (note/help added with err.note etc.) before every suggestion. So to achieve the order you're looking for it would require new mechanism and thus change alot of ui tests. (or making this help not a suggestion)
Unless, do you have any suggestions (pun intended)?
There was a problem hiding this comment.
I'll take a look sometime in the next few days, thanks for trying!
|
Reminder, once the PR becomes ready for a review, use |
|
@rustbot ready |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
a7202d8 to
df6364a
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Fyi I'm on holidays (at Eurorust) this week I'll get to any review next week |
|
@rustbot ready |
1029dfb to
12923c0
Compare
|
cc @Muscraft |
This comment has been minimized.
This comment has been minimized.
12923c0 to
23c5065
Compare
This comment has been minimized.
This comment has been minimized.
|
cc @rust-lang/clippy |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
bc001e2 to
e0719a1
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
- Add a note for unstable suggestions - Only suggest intrinsics when `core_intrinsics` feature is enabled
870fc09 to
e43ff05
Compare
|
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. |
This comment has been minimized.
This comment has been minimized.
086bf56 to
63763a9
Compare
|
@rustbot ready |
View all comments
core_intrinsicsfeature is enabledr? @JonathanBrouwer
Related:
Started from issue #78130
Created PR #161540 for regression tests
Then move to this PR for improvement points