-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Avoid Invalid code suggested when encountering unsatisfied trait bounds in derive macro code #104895
Conversation
(rustbot has picked a reviewer for you, use r? to override) |
I verified the change fixed this issue with Appreciate if anyone could help me on this. |
r? rust-lang/diagnostics |
See |
Thank you for your contribution. I found a mcve. Given the following code as a MCVE: src/test/ui/proc-macro/add-trait-impl.rs // aux-build:add-trait-impl.rs
use std::collections::BinaryHeap;
#[macro_use]
extern crate add_trait_impl;
#[derive(PartialEq, Eq, PartialOrd, Ord)]
struct PriorityQueueEntry<T> {
value: T,
}
#[derive(PartialOrd, AddImpl)]
struct PriorityQueue<T>(BinaryHeap<PriorityQueueEntry<T>>);
fn main() {} src/test/ui/proc-macro/auxiliary/add-trait-impl.rs // force-host
// no-prefer-dynamic
#![crate_type = "proc-macro"]
extern crate proc_macro;
use proc_macro::TokenStream;
#[proc_macro_derive(AddImpl)]
pub fn derive(input: TokenStream) -> TokenStream {
"use std::cmp::Ordering;
impl<T> Ord for PriorityQueue<T> {
fn cmp(&self, other: &Self) -> Ordering {
self.0.cmp(&self.height)
}
}
"
.parse()
.unwrap()
} Current output is:
The suggestion is a little bit different from what the issue says, but it might help you. |
Thanks very much! @TaKO8Ki |
dc1e083
to
7d2346e
Compare
…unsatisfied trait bounds in derive macro code
After reviewing your code, I thought that rustc should suggest like the following in this case. What do you think about? That is because skipping suggestions for macro code is just a quick fix. Given the following code: use std::collections::BinaryHeap;
#[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)]
struct PriorityQueueEntry<T> {
value: T,
}
#[derive(serde::Serialize)]
struct PriorityQueue<T>(BinaryHeap<PriorityQueueEntry<T>>); Rustc should suggest like: error[E0277]: the trait bound `T: Ord` is not satisfied
--> src/main.rs:8:10
|
8 | #[derive(serde::Serialize)]
| ^^^^^^^^^^^^^^^^ the trait `Ord` is not implemented for `T`
9 | struct PriorityQueue<T>(BinaryHeap<PriorityQueueEntry<T>>);
| --------------------------------- required by a bound introduced by this call
|
note: required for `PriorityQueueEntry<T>` to implement `Ord`
--> src/main.rs:3:37
|
3 | #[derive(PartialEq, Eq, PartialOrd, Ord, serde::Serialize)]
| ^^^
= note: required for `BinaryHeap<PriorityQueueEntry<T>>` to implement `Serialize`
note: required by a bound in `serialize_newtype_struct`
--> /Users/maeda.takayuki/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.148/src/ser/mod.rs:904:12
|
904 | T: Serialize;
| ^^^^^^^^^ required by this bound in `serialize_newtype_struct`
= note: this error originates in the derive macro `Ord` (in Nightly builds, run with -Z macro-backtrace for more info)
- help: consider further restricting type parameter `T`
- |
- 8 | #[derive(serde::Serialize, T: std::cmp::Ord)]
- | ++++++++++++++++++
+ help: consider restricting type parameter `T`
+ |
+ 9 | struct PriorityQueue<T: std::cmp::Ord>(BinaryHeap<PriorityQueueEntry<T>>);
+ | +++++++++++++++ |
Yes, you're right! |
@TaKO8Ki I'm still learning this part of code, |
I guess |
7d2346e
to
3980945
Compare
@bors r+ rollup |
…, r=TaKO8Ki Avoid Invalid code suggested when encountering unsatisfied trait bounds in derive macro code Fixes rust-lang#104884
…iaskrgr Rollup of 9 pull requests Successful merges: - rust-lang#103065 (rustdoc-json: Document and Test that args can be patterns.) - rust-lang#104865 (Don't overwrite local changes when updating submodules) - rust-lang#104895 (Avoid Invalid code suggested when encountering unsatisfied trait bounds in derive macro code) - rust-lang#105063 (Rustdoc Json Tests: Don't assume that core::fmt::Debug will always have one item.) - rust-lang#105064 (rustdoc: add comment to confusing CSS `main { min-width: 0 }`) - rust-lang#105074 (Add Nicholas Bishop to `.mailmap`) - rust-lang#105081 (Add a regression test for rust-lang#104322) - rust-lang#105086 (rustdoc: clean up sidebar link CSS) - rust-lang#105091 (add Tshepang Mbambo to .mailmap) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
Fixes #104884