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

Jump to bottom

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

Show file tree
Hide file tree
Changes from all commits
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
50 changes: 50 additions & 0 deletions compiler/rustc_hir/src/hir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1042,6 +1042,49 @@ impl BinOpKind {
}
}

pub fn as_std_trait(self, assign: bool) -> Option<&'static str> {
Some(if assign {
match self {
BinOpKind::Add => "std::ops::AddAssign",
BinOpKind::Sub => "std::ops::SubAssign",
BinOpKind::Mul => "std::ops::MulAssign",
BinOpKind::Div => "std::ops::DivAssign",
BinOpKind::Rem => "std::ops::RemAssign",
BinOpKind::BitAnd => "std::ops::BitAndAssign",
BinOpKind::BitXor => "std::ops::BitXorAssign",
BinOpKind::BitOr => "std::ops::BitOrAssign",
BinOpKind::Shl => "std::ops::ShlAssign",
BinOpKind::Shr => "std::ops::ShrAssign",
BinOpKind::Eq
| BinOpKind::Ne
| BinOpKind::Lt
| BinOpKind::Le
| BinOpKind::Ge
| BinOpKind::Gt
| BinOpKind::And
| BinOpKind::Or => return None,
}
} else {
match self {
BinOpKind::Add => "std::ops::Add",
BinOpKind::Sub => "std::ops::Sub",
BinOpKind::Mul => "std::ops::Mul",
BinOpKind::Div => "std::ops::Div",
BinOpKind::Rem => "std::ops::Rem",
BinOpKind::BitAnd => "std::ops::BitAnd",
BinOpKind::BitXor => "std::ops::BitXor",
BinOpKind::BitOr => "std::ops::BitOr",
BinOpKind::Shl => "std::ops::Shl",
BinOpKind::Shr => "std::ops::Shr",
BinOpKind::Eq | BinOpKind::Ne => "std::cmp::PartialEq",
BinOpKind::Lt | BinOpKind::Le | BinOpKind::Ge | BinOpKind::Gt => {
"std::cmp::PartialOrd"
}
BinOpKind::And | BinOpKind::Or => return None,
}
})
}

pub fn is_lazy(self) -> bool {
matches!(self, BinOpKind::And | BinOpKind::Or)
}
Expand Down Expand Up @@ -1124,6 +1167,13 @@ impl UnOp {
Self::Neg => "-",
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
}
}

nit: newline

pub fn as_std_trait(self) -> &'static str {
match self {
Self::Neg => "std::ops::Neg",
Self::Not => "std::ops::Not",
Self::Deref => "std::ops::UnDerf",
}
}

/// Returns `true` if the unary operator takes its argument by value.
pub fn is_by_value(self) -> bool {
Expand Down
109 changes: 69 additions & 40 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 @@ -276,71 +276,58 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
lhs_expr.span,
format!("cannot use `{}=` on type `{}`", op.node.as_str(), lhs_ty),
);
let missing_trait = match op.node {
hir::BinOpKind::Add => Some("std::ops::AddAssign"),
hir::BinOpKind::Sub => Some("std::ops::SubAssign"),
hir::BinOpKind::Mul => Some("std::ops::MulAssign"),
hir::BinOpKind::Div => Some("std::ops::DivAssign"),
hir::BinOpKind::Rem => Some("std::ops::RemAssign"),
hir::BinOpKind::BitAnd => Some("std::ops::BitAndAssign"),
hir::BinOpKind::BitXor => Some("std::ops::BitXorAssign"),
hir::BinOpKind::BitOr => Some("std::ops::BitOrAssign"),
hir::BinOpKind::Shl => Some("std::ops::ShlAssign"),
hir::BinOpKind::Shr => Some("std::ops::ShrAssign"),
_ => None,
};
(err, missing_trait, false, false)
(err, Some(STDImplementationMissing::BinOpAssign(op.node)), false, false)
}
IsAssign::No => {
let (message, missing_trait, use_output) = match op.node {
hir::BinOpKind::Add => (
format!("cannot add `{}` to `{}`", rhs_ty, lhs_ty),
Some("std::ops::Add"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Sub => (
format!("cannot subtract `{}` from `{}`", rhs_ty, lhs_ty),
Some("std::ops::Sub"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Mul => (
format!("cannot multiply `{}` by `{}`", lhs_ty, rhs_ty),
Some("std::ops::Mul"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Div => (
format!("cannot divide `{}` by `{}`", lhs_ty, rhs_ty),
Some("std::ops::Div"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Rem => (
format!("cannot mod `{}` by `{}`", lhs_ty, rhs_ty),
Some("std::ops::Rem"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::BitAnd => (
format!("no implementation for `{} & {}`", lhs_ty, rhs_ty),
Some("std::ops::BitAnd"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::BitXor => (
format!("no implementation for `{} ^ {}`", lhs_ty, rhs_ty),
Some("std::ops::BitXor"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::BitOr => (
format!("no implementation for `{} | {}`", lhs_ty, rhs_ty),
Some("std::ops::BitOr"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Shl => (
format!("no implementation for `{} << {}`", lhs_ty, rhs_ty),
Some("std::ops::Shl"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Shr => (
format!("no implementation for `{} >> {}`", lhs_ty, rhs_ty),
Some("std::ops::Shr"),
Some(STDImplementationMissing::BinOp(op.node)),
true,
),
hir::BinOpKind::Eq | hir::BinOpKind::Ne => (
Expand All @@ -349,7 +336,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialEq"),
Some(STDImplementationMissing::BinOp(op.node)),
false,
),
hir::BinOpKind::Lt
Expand All @@ -361,7 +348,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
op.node.as_str(),
lhs_ty
),
Some("std::cmp::PartialOrd"),
Some(STDImplementationMissing::BinOp(op.node)),
false,
),
_ => (
Expand Down Expand Up @@ -428,6 +415,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
}
if let Some(missing_trait) = missing_trait {
let missing_trait_str = missing_trait.as_std_trait().unwrap_or_default();
let mut visitor = TypeParamVisitor(vec![]);
visitor.visit_ty(lhs_ty);

Expand Down Expand Up @@ -459,7 +447,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&mut err,
ty,
rhs_ty,
missing_trait,
missing_trait_str,
p,
use_output,
);
Expand All @@ -468,14 +456,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// this note as it is redundant.
err.note(&format!(
"the trait `{}` is not implemented for `{}`",
missing_trait, lhs_ty
missing_trait_str, lhs_ty
));
}
} else {
bug!("type param visitor stored a non type param: {:?}", ty.kind());
}
} else if !suggested_deref && !involves_fn {
suggest_impl_missing(&mut err, lhs_ty, &missing_trait);
suggest_impl_missing(&mut err, lhs_ty, missing_trait);
}
}
err.emit();
Expand Down Expand Up @@ -710,12 +698,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Str | Never | Char | Tuple(_) | Array(_, _) => {}
Ref(_, ref lty, _) if *lty.kind() == Str => {}
_ => {
let missing_trait = match op {
hir::UnOp::Neg => "std::ops::Neg",
hir::UnOp::Not => "std::ops::Not",
hir::UnOp::Deref => "std::ops::UnDerf",
};
suggest_impl_missing(&mut err, operand_ty, &missing_trait);
suggest_impl_missing(
&mut err,
operand_ty,
STDImplementationMissing::Unop(op),
);
}
}
err.emit();
Expand Down Expand Up @@ -951,14 +938,56 @@ fn is_builtin_binop<'tcx>(lhs: Ty<'tcx>, rhs: Ty<'tcx>, op: hir::BinOp) -> bool
}
}

enum STDImplementationMissing {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
enum STDImplementationMissing {
enum StdImplementationMissing {

nit: capitalisation

Unop(hir::UnOp),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Unop(hir::UnOp),
UnOp(hir::UnOp),

nit: consistency

BinOp(hir::BinOpKind),
BinOpAssign(hir::BinOpKind),
}

impl STDImplementationMissing {
fn as_std_trait(&self) -> Option<&'static str> {
match self {
Self::Unop(e) => Some(e.as_std_trait()),
Self::BinOpAssign(e) => e.as_std_trait(true),
Self::BinOp(e) => e.as_std_trait(false),
}
}
}

/// If applicable, note that an implementation of `trait` for `ty` may fix the error.
fn suggest_impl_missing(err: &mut DiagnosticBuilder<'_>, ty: Ty<'_>, missing_trait: &str) {
fn suggest_impl_missing(
err: &mut DiagnosticBuilder<'_>,
ty: Ty<'_>,
missing_trait: STDImplementationMissing,
) {
if let Adt(def, _) = ty.peel_refs().kind() {
if def.did.is_local() {
err.note(&format!(
"an implementation of `{}` might be missing for `{}`",
missing_trait, ty
));
if let Some(missing_trait_name) = missing_trait.as_std_trait() {
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.

"an implementation of `{}` might be missing for `{}`",
missing_trait_name, ty
));
}
// This checks if the missing trait is PartialEq to suggest adding a derive
if let STDImplementationMissing::BinOp(binary_op_trait) = missing_trait {
if matches!(binary_op_trait, hir::BinOpKind::Eq | hir::BinOpKind::Ne) {
err.note(&format!(
"add `#[derive(PartialEq)]` or manually implement `PartialEq` for `{}`",
ty
));
} else if matches!(
binary_op_trait,
hir::BinOpKind::Lt
| hir::BinOpKind::Le
| hir::BinOpKind::Ge
| hir::BinOpKind::Gt
) {
err.note(&format!(
"add `#[derive(PartialOrd)]` or manually implement `PartialOrd` for `{}`",
ty
));
}
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions src/test/ui/binop/issue-28837.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ LL | a == a;
| A
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `A`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `A`

error[E0369]: binary operation `!=` cannot be applied to type `A`
--> $DIR/issue-28837.rs:26:7
Expand All @@ -107,6 +108,7 @@ LL | a != a;
| A
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `A`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `A`

error[E0369]: binary operation `<` cannot be applied to type `A`
--> $DIR/issue-28837.rs:28:7
Expand All @@ -117,6 +119,7 @@ LL | a < a;
| A
|
= note: an implementation of `std::cmp::PartialOrd` might be missing for `A`
= note: add `#[derive(PartialOrd)]` or manually implement `PartialOrd` for `A`

error[E0369]: binary operation `<=` cannot be applied to type `A`
--> $DIR/issue-28837.rs:30:7
Expand All @@ -127,6 +130,7 @@ LL | a <= a;
| A
|
= note: an implementation of `std::cmp::PartialOrd` might be missing for `A`
= note: add `#[derive(PartialOrd)]` or manually implement `PartialOrd` for `A`

error[E0369]: binary operation `>` cannot be applied to type `A`
--> $DIR/issue-28837.rs:32:7
Expand All @@ -137,6 +141,7 @@ LL | a > a;
| A
|
= note: an implementation of `std::cmp::PartialOrd` might be missing for `A`
= note: add `#[derive(PartialOrd)]` or manually implement `PartialOrd` for `A`

error[E0369]: binary operation `>=` cannot be applied to type `A`
--> $DIR/issue-28837.rs:34:7
Expand All @@ -147,6 +152,7 @@ LL | a >= a;
| A
|
= note: an implementation of `std::cmp::PartialOrd` might be missing for `A`
= note: add `#[derive(PartialOrd)]` or manually implement `PartialOrd` for `A`

error: aborting due to 15 previous errors

Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/derives/derives-span-PartialEq-enum-struct-variant.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | x: Error
| ^^^^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0369]: binary operation `!=` cannot be applied to type `Error`
Expand All @@ -14,6 +15,7 @@ LL | x: Error
| ^^^^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/derives/derives-span-PartialEq-enum.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | Error
| ^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0369]: binary operation `!=` cannot be applied to type `Error`
Expand All @@ -14,6 +15,7 @@ LL | Error
| ^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/derives/derives-span-PartialEq-struct.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | x: Error
| ^^^^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0369]: binary operation `!=` cannot be applied to type `Error`
Expand All @@ -14,6 +15,7 @@ LL | x: Error
| ^^^^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
2 changes: 2 additions & 0 deletions src/test/ui/derives/derives-span-PartialEq-tuple-struct.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ LL | Error
| ^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0369]: binary operation `!=` cannot be applied to type `Error`
Expand All @@ -14,6 +15,7 @@ LL | Error
| ^^^^^
|
= note: an implementation of `std::cmp::PartialEq` might be missing for `Error`
= note: add `#[derive(PartialEq)]` or manually implement `PartialEq` for `Error`
= note: this error originates in the derive macro `PartialEq` (in Nightly builds, run with -Z macro-backtrace for more info)

error: aborting due to 2 previous errors
Expand Down
Loading