Skip to content
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

Invalid code suggested when encountering unsatisfied trait bounds in derive macro code #104884

Closed
jonasbb opened this issue Nov 25, 2022 · 5 comments · Fixed by #104895
Closed
Assignees
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jonasbb
Copy link
Contributor

jonasbb commented Nov 25, 2022

Given the following code: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=0345475f1b234a0cf26f97bfb712cceb

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>>);

The current output is:

Compiling playground v0.0.1 (/playground)
error[[E0277]](https://doc.rust-lang.org/nightly/error-index.html#E0277): the trait bound `T: Ord` is not satisfied
   --> src/lib.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/lib.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`
   --> /playground/.cargo/registry/src/github.com-1ecc6299db9ec823/serde-1.0.147/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)]
    |                          ++++++++++++++++++

The last part of the suggestion #[derive(serde::Serialize, T: std::cmp::Ord)] is not valid Rust code and therefore should not be suggested like this.

This problem occurs on the Playground on both stable and nightly with basically identical error messages. serde v1.0.147 requires T: Ord such that BinaryHeap will be Serialize. Otherwise the problem does not seem serde specific.

@jonasbb jonasbb added A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 25, 2022
@chenyukang
Copy link
Member

@rustbot claim

@chenyukang
Copy link
Member

@jonasbb do you know how to create a minimal case without serde?
so we may add it to unit testing.

I give a fix and verified it's OK with my local testing, but I'm struggle to construct a minimal case without serde to reproduce it.

@jonasbb
Copy link
Contributor Author

jonasbb commented Nov 25, 2022

@chenyukang I assume this reproduces when using the same setup of derive macro implementing a trait while the implementation is missing bounds. I don't know if it is relevant that the derive macro produces a trait.

Something along these lines, with the proc macro crate omitted.

// main crate
trait FooBar {
    fn dostuff(self);
}

#[derive(...)]
struct Struct<T>(T);

// derive macro expanded code
impl<T> FooBar for Struct<T> {
    fn dostuff(self) {
		use std::collections::BinaryHeap;
		let mut bh = BinaryHeap::new();
		bh.push(self.0);
    }
}

@chenyukang
Copy link
Member

yes, it's caused by derive macro.

@chenyukang
Copy link
Member

emm,met this error 😒

error: can't use a procedural macro from the same crate that defines it

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 30, 2022
…, r=TaKO8Ki

Avoid Invalid code suggested when encountering unsatisfied trait bounds in derive macro code

Fixes rust-lang#104884
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Nov 30, 2022
…, r=TaKO8Ki

Avoid Invalid code suggested when encountering unsatisfied trait bounds in derive macro code

Fixes rust-lang#104884
@bors bors closed this as completed in 3980945 Dec 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
2 participants