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.

Sign up for GitHub

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

Fixed #85845: Added a note in E0369 if the missing trait is PartialEq #85929

Closed
wants to merge 9 commits into from
Closed

Fixed #85845: Added a note in E0369 if the missing trait is PartialEq #85929

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
518f643
Added a note in E0369 if the missing trait is PartialEq to use the de…
ccgauche Jun 2, 2021
01996be
Updated the fix for #85845
ccgauche Aug 1, 2021
a1d18be
Removed unused contains
ccgauche Aug 1, 2021
96c4539
Fixed error unit test + Added correction for PartialOrd
ccgauche Aug 1, 2021
bcb49dc
Fixed CI 2
ccgauche Aug 1, 2021
afcef43
Fixed derives-span-PartialEq-enum tests
ccgauche Aug 2, 2021
38830d2
Replaced String checks with enums so we can ensure that everything wi…
ccgauche Aug 17, 2021
b58393b
Fixed CI errors
ccgauche Aug 17, 2021
207f9b1
Fixed borrow issue
ccgauche Aug 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions compiler/rustc_typeck/src/check/op.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -959,6 +959,13 @@ fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_tra
"an implementation of `{}` might be missing for `{}`",
missing_trait, ty
));
// This checks if the missing trait is PartialEq to suggest adding a derive
if missing_trait.ends_with("PartialEq") {
ccgauche marked this conversation as resolved.
Show resolved Hide resolved
err.note(&format!(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use span_suggestion, as this is a suggestion. You should move this to somewhere else, so you can look up the span of the original definition for the type.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure that a span suggestion is better. A note at the end is clearer. Why do you want a span_suggestion?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because you want to suggest adding a derive to the type being compared here. So you use the span of the original definition and add a #[derive(PartialEq)] on top of it, or add PartialEq to the existing derive.

"add `#[derive(PartialEq)]` or manually implement `PartialEq` for `{}`",
ty
));
}
}
}
}
Expand Down