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

Provide better type hints when a type doesn't support a binary operator #110877

Merged
merged 3 commits into from
Apr 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 3 additions & 9 deletions compiler/rustc_hir_typeck/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,15 +461,9 @@ fn fatally_break_rust(sess: &Session) {
));
}

fn has_expected_num_generic_args(
tcx: TyCtxt<'_>,
trait_did: Option<DefId>,
expected: usize,
) -> bool {
trait_did.map_or(true, |trait_did| {
let generics = tcx.generics_of(trait_did);
generics.count() == expected + if generics.has_self { 1 } else { 0 }
})
fn has_expected_num_generic_args(tcx: TyCtxt<'_>, trait_did: DefId, expected: usize) -> bool {
let generics = tcx.generics_of(trait_did);
generics.count() == expected + if generics.has_self { 1 } else { 0 }
}

pub fn provide(providers: &mut Providers) {
Expand Down
39 changes: 26 additions & 13 deletions compiler/rustc_hir_typeck/src/method/suggest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use rustc_middle::traits::util::supertraits;
use rustc_middle::ty::fast_reject::DeepRejectCtxt;
use rustc_middle::ty::fast_reject::{simplify_type, TreatParams};
use rustc_middle::ty::print::{with_crate_prefix, with_forced_trimmed_paths};
use rustc_middle::ty::IsSuggestable;
use rustc_middle::ty::{self, GenericArgKind, Ty, TyCtxt, TypeVisitableExt};
use rustc_middle::ty::{IsSuggestable, ToPolyTraitRef};
use rustc_span::symbol::{kw, sym, Ident};
use rustc_span::Symbol;
use rustc_span::{edit_distance, source_map, ExpnKind, FileName, MacroKind, Span};
Expand Down Expand Up @@ -2068,7 +2068,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut derives = Vec::<(String, Span, Symbol)>::new();
let mut traits = Vec::new();
for (pred, _, _) in unsatisfied_predicates {
let ty::PredicateKind::Clause(ty::Clause::Trait(trait_pred)) = pred.kind().skip_binder() else { continue };
let Some(ty::PredicateKind::Clause(ty::Clause::Trait(trait_pred))) =
pred.kind().no_bound_vars()
else {
continue
};
let adt = match trait_pred.self_ty().ty_adt_def() {
Some(adt) if adt.did().is_local() => adt,
_ => continue,
Expand All @@ -2085,22 +2089,31 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
| sym::Hash
| sym::Debug => true,
_ => false,
} && match trait_pred.trait_ref.substs.as_slice() {
// Only suggest deriving if lhs == rhs...
[lhs, rhs] => {
if let Some(lhs) = lhs.as_type()
&& let Some(rhs) = rhs.as_type()
{
self.can_eq(self.param_env, lhs, rhs)
} else {
false
}
},
// Unary ops can always be derived
[_] => true,
_ => false,
};
if can_derive {
let self_name = trait_pred.self_ty().to_string();
let self_span = self.tcx.def_span(adt.did());
if let Some(poly_trait_ref) = pred.to_opt_poly_trait_pred() {
for super_trait in supertraits(self.tcx, poly_trait_ref.to_poly_trait_ref())
for super_trait in
supertraits(self.tcx, ty::Binder::dummy(trait_pred.trait_ref))
{
if let Some(parent_diagnostic_name) =
self.tcx.get_diagnostic_name(super_trait.def_id())
{
if let Some(parent_diagnostic_name) =
self.tcx.get_diagnostic_name(super_trait.def_id())
{
derives.push((
self_name.clone(),
self_span,
parent_diagnostic_name,
));
}
derives.push((self_name.clone(), self_span, parent_diagnostic_name));
}
}
derives.push((self_name, self_span, diagnostic_name));
Expand Down
48 changes: 35 additions & 13 deletions compiler/rustc_hir_typeck/src/op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,14 +408,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
}
};

let is_compatible = |lhs_ty, rhs_ty| {
let is_compatible_after_call = |lhs_ty, rhs_ty| {
self.lookup_op_method(
lhs_ty,
Some((rhs_expr, rhs_ty)),
Op::Binary(op, is_assign),
expected,
)
.is_ok()
// Suggest calling even if, after calling, the types don't
// implement the operator, since it'll lead to better
// diagnostics later.
|| self.can_eq(self.param_env, lhs_ty, rhs_ty)
};

// We should suggest `a + b` => `*a + b` if `a` is copy, and suggest
Expand All @@ -436,16 +440,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
suggest_deref_binop(*lhs_deref_ty);
}
} else if self.suggest_fn_call(&mut err, lhs_expr, lhs_ty, |lhs_ty| {
is_compatible(lhs_ty, rhs_ty)
is_compatible_after_call(lhs_ty, rhs_ty)
}) || self.suggest_fn_call(&mut err, rhs_expr, rhs_ty, |rhs_ty| {
is_compatible(lhs_ty, rhs_ty)
is_compatible_after_call(lhs_ty, rhs_ty)
}) || self.suggest_two_fn_call(
&mut err,
rhs_expr,
rhs_ty,
lhs_expr,
lhs_ty,
|lhs_ty, rhs_ty| is_compatible(lhs_ty, rhs_ty),
|lhs_ty, rhs_ty| is_compatible_after_call(lhs_ty, rhs_ty),
) {
// Cool
}
Expand Down Expand Up @@ -719,7 +723,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
Op::Binary(op, _) => op.span,
Op::Unary(_, span) => span,
};
let (opname, trait_did) = lang_item_for_op(self.tcx, op, span);
let (opname, Some(trait_did)) = lang_item_for_op(self.tcx, op, span) else {
// Bail if the operator trait is not defined.
return Err(vec![]);
};

debug!(
"lookup_op_method(lhs_ty={:?}, op={:?}, opname={:?}, trait_did={:?})",
Expand Down Expand Up @@ -759,18 +766,33 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
},
);

let method = trait_did.and_then(|trait_did| {
self.lookup_method_in_trait(cause.clone(), opname, trait_did, lhs_ty, Some(input_types))
});

match (method, trait_did) {
(Some(ok), _) => {
let method = self.lookup_method_in_trait(
cause.clone(),
opname,
trait_did,
lhs_ty,
Some(input_types),
);
match method {
Some(ok) => {
let method = self.register_infer_ok_obligations(ok);
self.select_obligations_where_possible(|_| {});
Ok(method)
}
(None, None) => Err(vec![]),
(None, Some(trait_did)) => {
None => {
// This path may do some inference, so make sure we've really
// doomed compilation so as to not accidentally stabilize new
// inference or something here...
self.tcx.sess.delay_span_bug(span, "this path really should be doomed...");
// Guide inference for the RHS expression if it's provided --
// this will allow us to better error reporting, at the expense
// of making some error messages a bit more specific.
if let Some((rhs_expr, rhs_ty)) = opt_rhs
&& rhs_ty.is_ty_var()
{
self.check_expr_coercible_to_type(rhs_expr, rhs_ty, None);
}

let (obligation, _) =
self.obligation_for_method(cause, trait_did, lhs_ty, Some(input_types));
// FIXME: This should potentially just add the obligation to the `FnCtxt`
Expand Down
42 changes: 22 additions & 20 deletions compiler/rustc_hir_typeck/src/place_op.rs
Original file line number Diff line number Diff line change
Expand Up @@ -200,9 +200,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!("try_overloaded_place_op({:?},{:?},{:?})", span, base_ty, op);

let (imm_tr, imm_op) = match op {
let (Some(imm_tr), imm_op) = (match op {
PlaceOp::Deref => (self.tcx.lang_items().deref_trait(), sym::deref),
PlaceOp::Index => (self.tcx.lang_items().index_trait(), sym::index),
}) else {
// Bail if `Deref` or `Index` isn't defined.
return None;
};

// If the lang item was declared incorrectly, stop here so that we don't
Expand All @@ -219,15 +222,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return None;
}

imm_tr.and_then(|trait_did| {
self.lookup_method_in_trait(
self.misc(span),
Ident::with_dummy_span(imm_op),
trait_did,
base_ty,
Some(arg_tys),
)
})
self.lookup_method_in_trait(
self.misc(span),
Ident::with_dummy_span(imm_op),
imm_tr,
base_ty,
Some(arg_tys),
)
}

fn try_mutable_overloaded_place_op(
Expand All @@ -239,9 +240,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
) -> Option<InferOk<'tcx, MethodCallee<'tcx>>> {
debug!("try_mutable_overloaded_place_op({:?},{:?},{:?})", span, base_ty, op);

let (mut_tr, mut_op) = match op {
let (Some(mut_tr), mut_op) = (match op {
PlaceOp::Deref => (self.tcx.lang_items().deref_mut_trait(), sym::deref_mut),
PlaceOp::Index => (self.tcx.lang_items().index_mut_trait(), sym::index_mut),
}) else {
// Bail if `DerefMut` or `IndexMut` isn't defined.
return None;
};

// If the lang item was declared incorrectly, stop here so that we don't
Expand All @@ -258,15 +262,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return None;
}

mut_tr.and_then(|trait_did| {
self.lookup_method_in_trait(
self.misc(span),
Ident::with_dummy_span(mut_op),
trait_did,
base_ty,
Some(arg_tys),
)
})
self.lookup_method_in_trait(
self.misc(span),
Ident::with_dummy_span(mut_op),
mut_tr,
base_ty,
Some(arg_tys),
)
}

/// Convert auto-derefs, indices, etc of an expression from `Deref` and `Index`
Expand Down
7 changes: 7 additions & 0 deletions tests/ui/binop/eq-arr.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
fn main() {
struct X;
//~^ HELP consider annotating `X` with `#[derive(PartialEq)]`
let xs = [X, X, X];
let eq = xs == [X, X, X];
//~^ ERROR binary operation `==` cannot be applied to type `[X; 3]`
}
22 changes: 22 additions & 0 deletions tests/ui/binop/eq-arr.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
error[E0369]: binary operation `==` cannot be applied to type `[X; 3]`
--> $DIR/eq-arr.rs:5:17
|
LL | let eq = xs == [X, X, X];
| -- ^^ --------- [X; 3]
| |
| [X; 3]
|
note: an implementation of `PartialEq` might be missing for `X`
--> $DIR/eq-arr.rs:2:5
|
LL | struct X;
| ^^^^^^^^ must implement `PartialEq`
help: consider annotating `X` with `#[derive(PartialEq)]`
|
LL + #[derive(PartialEq)]
LL | struct X;
|

error: aborting due to previous error

For more information about this error, try `rustc --explain E0369`.
13 changes: 13 additions & 0 deletions tests/ui/binop/eq-vec.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
fn main() {
#[derive(Debug)]
enum Foo {
//~^ HELP consider annotating `Foo` with `#[derive(PartialEq)]`
Bar,
Qux,
}

let vec1 = vec![Foo::Bar, Foo::Qux];
let vec2 = vec![Foo::Bar, Foo::Qux];
assert_eq!(vec1, vec2);
//~^ ERROR binary operation `==` cannot be applied to type `Vec<Foo>`
}
24 changes: 24 additions & 0 deletions tests/ui/binop/eq-vec.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
error[E0369]: binary operation `==` cannot be applied to type `Vec<Foo>`
--> $DIR/eq-vec.rs:11:5
|
LL | assert_eq!(vec1, vec2);
| ^^^^^^^^^^^^^^^^^^^^^^
| |
| Vec<Foo>
| Vec<Foo>
|
note: an implementation of `PartialEq` might be missing for `Foo`
--> $DIR/eq-vec.rs:3:5
|
LL | enum Foo {
| ^^^^^^^^ must implement `PartialEq`
= note: this error originates in the macro `assert_eq` (in Nightly builds, run with -Z macro-backtrace for more info)
help: consider annotating `Foo` with `#[derive(PartialEq)]`
|
LL + #[derive(PartialEq)]
LL | enum Foo {
|

error: aborting due to previous error

For more information about this error, try `rustc --explain E0369`.
Loading