Skip to content

Commit

Permalink
Rollup merge of #110877 - compiler-errors:binop-err, r=cjgillot
Browse files Browse the repository at this point in the history
Provide better type hints when a type doesn't support a binary operator

For example, when checking whether `vec![A] == vec![A]` holds, we first evaluate the LHS's ty, then probe for any `PartialEq` implementations for that. If none is found, we report an error by evaluating `Vec<A>: PartialEq<?0>` for fulfillment errors, but the RHS is not yet evaluated and remains an inference variable `?0`!

To fix this, we evaluate the RHS and equate it to that RHS infer var `?0`, so that we are able to provide more detailed fulfillment errors for why `Vec<A>: PartialEq<Vec<A>>` doesn't hold (namely, the nested obligation `A: PartialEq<A>` doesn't hold).

Fixes #95285
Fixes #110867
  • Loading branch information
matthiaskrgr authored Apr 28, 2023
2 parents f357475 + 3125979 commit aba9fb4
Show file tree
Hide file tree
Showing 23 changed files with 215 additions and 117 deletions.
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 @@ -455,15 +455,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

0 comments on commit aba9fb4

Please # to comment.