-
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 more info for common trait resolution and async/await errors #83689
Conversation
r? @davidtwco (rust-highfive has picked a reviewer for you, use r? to override) |
err.note(&format!("required because it appears within the type `{}`", ty)); | ||
let msg = format!("required because it appears within the type `{}`", ty); | ||
match ty.kind() { | ||
ty::Adt(def, _) => match self.tcx.item_name_from_hir(def.did) { |
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.
Why use item_name_from_hir
instead of opt_item_name
? Do you expect def
to always be local?
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.
Using item_name_from_hir
isn't going to cause ICEs with non-local definitions, we'll only be missing the span in those cases. I assume the idea is that this span is only helpful and is actionable for locally defined types?
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.
It feels to me that seeing where the requirement is included in another crate isn't as useful as when the requirement comes from the current crate, because people can't change them, but it might useful for extra context? 🤷♀️
I'll make the change and see what it actually looks like in the test suite.
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.
Just included this change. No effects in the tests (but might increase verbosity in the wild). Regardless, I can change it after real world usage if people complain.
r=me after resolving @jyn514's comment |
@bors r=davidtwco |
📌 Commit 06b82ab has been approved by |
…dtwco Add more info for common trait resolution and async/await errors * Suggest `Pin::new`/`Box::new`/`Arc::new`/`Box::pin` in more cases * Point at `impl` and type defs introducing requirements on E0277
…dtwco Add more info for common trait resolution and async/await errors * Suggest `Pin::new`/`Box::new`/`Arc::new`/`Box::pin` in more cases * Point at `impl` and type defs introducing requirements on E0277
⌛ Testing commit 06b82ab with merge de6b40206b19b4a752e3637dd13fddda0a96741c... |
@bors retry |
This comment has been minimized.
This comment has been minimized.
06b82ab
to
64e6288
Compare
@bors r=davidtwco |
📌 Commit 64e6288 has been approved by |
…dtwco Add more info for common trait resolution and async/await errors * Suggest `Pin::new`/`Box::new`/`Arc::new`/`Box::pin` in more cases * Point at `impl` and type defs introducing requirements on E0277
Rollup of 5 pull requests Successful merges: - rust-lang#82497 (Fix handling of `--output-format json` flag) - rust-lang#83689 (Add more info for common trait resolution and async/await errors) - rust-lang#83952 (Account for `ExprKind::Block` when suggesting .into() and deref) - rust-lang#83965 (Add Debug implementation for hir::intravisit::FnKind) - rust-lang#83974 (Fix outdated crate names in `rustc_interface::callbacks`) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Pin::new
/Box::new
/Arc::new
/Box::pin
in more casesimpl
and type defs introducing requirements on E0277