-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add notes to macro-not-found diagnostics to point out how things with the same name were not a match. #88232
Add notes to macro-not-found diagnostics to point out how things with the same name were not a match. #88232
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love how these are looking 😃
I'm leaving some nitpicks, but we don't need to address them all to land this.
@@ -32,15 +32,15 @@ error: cannot find macro `Box` in this scope | |||
LL | Box!(); | |||
| ^^^ | |||
| | |||
= note: `Box` is in scope, but it is a struct | |||
= note: `Box` is in scope, but it is a struct, not a macro |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the cases where we don't have a span, wouldn't it make sense to turn this into the primary span? Something like:
error: cannot find macro `Copy` in this scope
--> $DIR/issue-88206.rs:56:5
|
LL | Copy!();
| ^^^^ this is not a macro, it is a derive macro: `#[derive(Copy)]`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd avoid pointing the note at the identifier the user wrote, because the error already says cannot find `Thing`
, and the note is about another Thing
, not the Thing
they wrote. (Even though they have the same name.)
So in the case we can point at an import, we do that to highlight Thing
here is not found, but Thing
there exists but is something else. Pointing both at the same identifier would make it less clear that we're talking about two different Thing
s.
bacb1c8
to
908ce2f
Compare
@bors r+ |
📌 Commit 908ce2f has been approved by |
Rollup of 6 pull requests Successful merges: - rust-lang#87976 (Account for tabs when highlighting multiline code suggestions) - rust-lang#88174 (Clarify some wording in Rust 2021 lint docs) - rust-lang#88188 (Greatly improve limitation handling on parallel rustdoc GUI test run) - rust-lang#88230 (Fix typos “a”→“an”) - rust-lang#88232 (Add notes to macro-not-found diagnostics to point out how things with the same name were not a match.) - rust-lang#88259 (Do not mark `-Z thir-unsafeck` as unsound anymore) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This adds notes like:
Fixes #88206
Includes #88229
r? @estebank