From 9cc8d8619062278046e678bbc08401b733a17236 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Esteban=20K=C3=BCber?= Date: Tue, 3 Jan 2023 20:20:31 -0800 Subject: [PATCH] Tweak output - Only point at a the single expression where the found type was first inferred. - Find method call argument that might have caused the found type to be inferred. - Provide structured suggestion. - Apply some review comments. - Tweak wording. --- compiler/rustc_hir_typeck/src/demand.rs | 165 +++++++++++++----- .../type/type-check/assignment-in-if.stderr | 15 +- .../type/type-check/point-at-inference-2.rs | 13 ++ .../type-check/point-at-inference-2.stderr | 56 ++++++ .../type/type-check/point-at-inference.fixed | 13 ++ .../ui/type/type-check/point-at-inference.rs | 13 ++ .../type/type-check/point-at-inference.stderr | 26 +++ 7 files changed, 242 insertions(+), 59 deletions(-) create mode 100644 src/test/ui/type/type-check/point-at-inference-2.rs create mode 100644 src/test/ui/type/type-check/point-at-inference-2.stderr create mode 100644 src/test/ui/type/type-check/point-at-inference.fixed create mode 100644 src/test/ui/type/type-check/point-at-inference.rs create mode 100644 src/test/ui/type/type-check/point-at-inference.stderr diff --git a/compiler/rustc_hir_typeck/src/demand.rs b/compiler/rustc_hir_typeck/src/demand.rs index c50d03b944f..317e9b5a7a1 100644 --- a/compiler/rustc_hir_typeck/src/demand.rs +++ b/compiler/rustc_hir_typeck/src/demand.rs @@ -1,5 +1,6 @@ use crate::FnCtxt; use rustc_ast::util::parser::PREC_POSTFIX; +use rustc_data_structures::fx::FxHashMap; use rustc_errors::MultiSpan; use rustc_errors::{Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed}; use rustc_hir as hir; @@ -14,12 +15,14 @@ use rustc_middle::ty::error::{ExpectedFound, TypeError}; use rustc_middle::ty::fold::TypeFolder; use rustc_middle::ty::print::{with_forced_trimmed_paths, with_no_trimmed_paths}; +use rustc_middle::ty::relate::TypeRelation; use rustc_middle::ty::{ self, Article, AssocItem, Ty, TyCtxt, TypeAndMut, TypeSuperFoldable, TypeVisitable, }; use rustc_span::symbol::{sym, Symbol}; use rustc_span::{BytePos, Span}; use rustc_trait_selection::infer::InferCtxtExt as _; +use rustc_trait_selection::traits::error_reporting::method_chain::CollectAllMismatches; use rustc_trait_selection::traits::ObligationCause; use super::method::probe; @@ -44,7 +47,7 @@ pub fn emit_type_mismatch_suggestions( self.annotate_alternative_method_deref(err, expr, error); // Use `||` to give these suggestions a precedence - let _ = self.suggest_missing_parentheses(err, expr) + let suggested = self.suggest_missing_parentheses(err, expr) || self.suggest_remove_last_method_call(err, expr, expected) || self.suggest_associated_const(err, expr, expected) || self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr) @@ -57,8 +60,10 @@ pub fn emit_type_mismatch_suggestions( || self.suggest_block_to_brackets_peeling_refs(err, expr, expr_ty, expected) || self.suggest_copied_or_cloned(err, expr, expr_ty, expected) || self.suggest_into(err, expr, expr_ty, expected) - || self.suggest_floating_point_literal(err, expr, expected) - || self.point_inference_types(err, expr); + || self.suggest_floating_point_literal(err, expr, expected); + if !suggested { + self.point_at_expr_source_of_inferred_type(err, expr, expr_ty, expected); + } } pub fn emit_coerce_suggestions( @@ -210,7 +215,13 @@ pub fn demand_coerce_diag( (expected, Some(err)) } - fn point_inference_types(&self, err: &mut Diagnostic, expr: &hir::Expr<'_>) -> bool { + fn point_at_expr_source_of_inferred_type( + &self, + err: &mut Diagnostic, + expr: &hir::Expr<'_>, + found: Ty<'tcx>, + expected: Ty<'tcx>, + ) -> bool { let tcx = self.tcx; let map = self.tcx.hir(); @@ -250,25 +261,17 @@ fn fold_const(&mut self, ct: ty::Const<'tcx>) -> ty::Const<'tcx> { let hir::ExprKind::Path(hir::QPath::Resolved(None, p)) = expr.kind else { return false; }; let [hir::PathSegment { ident, args: None, .. }] = p.segments else { return false; }; let hir::def::Res::Local(hir_id) = p.res else { return false; }; - let Some(node) = map.find(hir_id) else { return false; }; - let hir::Node::Pat(pat) = node else { return false; }; + let Some(hir::Node::Pat(pat)) = map.find(hir_id) else { return false; }; let parent = map.get_parent_node(pat.hir_id); let Some(hir::Node::Local(hir::Local { ty: None, init: Some(init), .. })) = map.find(parent) else { return false; }; - - let ty = self.node_ty(init.hir_id); + let Some(ty) = self.node_ty_opt(init.hir_id) else { return false; }; if ty.is_closure() || init.span.overlaps(expr.span) || pat.span.from_expansion() { return false; } - let mut span_labels = vec![( - init.span, - with_forced_trimmed_paths!(format!( - "here the type of `{ident}` is inferred to be `{ty}`", - )), - )]; // Locate all the usages of the relevant binding. struct FindExprs<'hir> { @@ -296,71 +299,139 @@ fn visit_expr(&mut self, ex: &'v hir::Expr<'v>) { expr_finder.visit_expr(body.value); let mut eraser = TypeEraser { tcx }; let mut prev = eraser.fold_ty(ty); + let mut prev_span = None; - for ex in expr_finder.uses { - if ex.span.overlaps(expr.span) { break; } - let parent = map.get_parent_node(ex.hir_id); + for binding in expr_finder.uses { + // In every expression where the binding is referenced, we will look at that + // expression's type and see if it is where the incorrect found type was fully + // "materialized" and point at it. We will also try to provide a suggestion there. + let parent = map.get_parent_node(binding.hir_id); if let Some(hir::Node::Expr(expr)) | Some(hir::Node::Stmt(hir::Stmt { kind: hir::StmtKind::Expr(expr) | hir::StmtKind::Semi(expr), .. })) = &map.find(parent) - && let hir::ExprKind::MethodCall(s, rcvr, args, span) = expr.kind - && rcvr.hir_id == ex.hir_id + && let hir::ExprKind::MethodCall(s, rcvr, args, _span) = expr.kind + && rcvr.hir_id == binding.hir_id + && let Some(def_id) = self.typeck_results.borrow().type_dependent_def_id(expr.hir_id) { - let ty = if let Ok(m) = self.lookup_method(ty, s, span, expr, rcvr, args) { - // We get the self type from `lookup_method` because the `rcvr` node - // type will not have had any adjustments from the fn arguments. - let ty = m.sig.inputs_and_output[0]; - match ty.kind() { - // Remove one layer of references to account for `&mut self` and - // `&self`, so that we can compare it against the binding. - ty::Ref(_, ty, _) => *ty, - _ => ty, + // We special case methods, because they can influence inference through the + // call's arguments and we can provide a more explicit span. + let sig = self.tcx.fn_sig(def_id); + let def_self_ty = sig.input(0).skip_binder(); + let rcvr_ty = self.node_ty(rcvr.hir_id); + // Get the evaluated type *after* calling the method call, so that the influence + // of the arguments can be reflected in the receiver type. The receiver + // expression has the type *before* theis analysis is done. + let ty = match self.lookup_probe(s.ident, rcvr_ty, expr, probe::ProbeScope::TraitsInScope) { + Ok(pick) => pick.self_ty, + Err(_) => rcvr_ty, + }; + // Remove one layer of references to account for `&mut self` and + // `&self`, so that we can compare it against the binding. + let (ty, def_self_ty) = match (ty.kind(), def_self_ty.kind()) { + (ty::Ref(_, ty, a), ty::Ref(_, self_ty, b)) if a == b => (*ty, *self_ty), + _ => (ty, def_self_ty), + }; + let mut param_args = FxHashMap::default(); + let mut param_expected = FxHashMap::default(); + let mut param_found = FxHashMap::default(); + if self.can_eq(self.param_env, ty, found).is_ok() { + // We only point at the first place where the found type was inferred. + for (i, param_ty) in sig.inputs().skip_binder().iter().skip(1).enumerate() { + if def_self_ty.contains(*param_ty) && let ty::Param(_) = param_ty.kind() { + // We found an argument that references a type parameter in `Self`, + // so we assume that this is the argument that caused the found + // type, which we know already because of `can_eq` above was first + // inferred in this method call. + let arg = &args[i]; + let arg_ty = self.node_ty(arg.hir_id); + err.span_label( + arg.span, + &format!( + "this is of type `{arg_ty}`, which makes `{ident}` to be \ + inferred as `{ty}`", + ), + ); + param_args.insert(param_ty, (arg, arg_ty)); + } } - } else { - self.node_ty(rcvr.hir_id) + } + + // Here we find, for a type param `T`, the type that `T` is in the current + // method call *and* in the original expected type. That way, we can see if we + // can give any structured suggestion for the function argument. + let mut c = CollectAllMismatches { + infcx: &self.infcx, + param_env: self.param_env, + errors: vec![], }; + let _ = c.relate(def_self_ty, ty); + for error in c.errors { + if let TypeError::Sorts(error) = error { + param_found.insert(error.expected, error.found); + } + } + c.errors = vec![]; + let _ = c.relate(def_self_ty, expected); + for error in c.errors { + if let TypeError::Sorts(error) = error { + param_expected.insert(error.expected, error.found); + } + } + for (param, (arg,arg_ty)) in param_args.iter() { + let Some(expected) = param_expected.get(param) else { continue; }; + let Some(found) = param_found.get(param) else { continue; }; + if self.can_eq(self.param_env, *arg_ty, *found).is_err() { continue; } + self.suggest_deref_ref_or_into(err, arg, *expected, *found, None); + } + let ty = eraser.fold_ty(ty); if ty.references_error() { break; } - if ty != prev { - span_labels.push(( + if ty != prev + && param_args.is_empty() + && self.can_eq(self.param_env, ty, found).is_ok() + { + // We only point at the first place where the found type was inferred. + err.span_label( s.ident.span, with_forced_trimmed_paths!(format!( "here the type of `{ident}` is inferred to be `{ty}`", )), - )); - prev = ty; + ); + break; } + prev = ty; } else { - let ty = eraser.fold_ty(self.node_ty(ex.hir_id)); + let ty = eraser.fold_ty(self.node_ty(binding.hir_id)); if ty.references_error() { break; } - if ty != prev { - span_labels.push(( - ex.span, + if ty != prev && let Some(span) = prev_span && self.can_eq(self.param_env, ty, found).is_ok() { + // We only point at the first place where the found type was inferred. + // We use the *previous* span because if the type is known *here* it means + // it was *evaluated earlier*. We don't do this for method calls because we + // evaluate the method's self type eagerly, but not in any other case. + err.span_label( + span, with_forced_trimmed_paths!(format!( "here the type of `{ident}` is inferred to be `{ty}`", )), - )); + ); + break; } prev = ty; } - if ex.hir_id == expr.hir_id { - // Stop showing spans after the error type was emitted. + if binding.hir_id == expr.hir_id { + // Do not look at expressions that come after the expression we were originally + // evaluating and had a type error. break; } + prev_span = Some(binding.span); } } - if span_labels.len() < 2 { - return false; - } - for (sp, label) in span_labels { - err.span_label(sp, &label); - } true } diff --git a/src/test/ui/type/type-check/assignment-in-if.stderr b/src/test/ui/type/type-check/assignment-in-if.stderr index fb11d6160bb..de133e5599c 100644 --- a/src/test/ui/type/type-check/assignment-in-if.stderr +++ b/src/test/ui/type/type-check/assignment-in-if.stderr @@ -67,10 +67,7 @@ LL | x == 5 error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:44:18 | -LL | let x = 1; - | - here the type of `x` is inferred to be `{integer}` -... -LL | println!("{}", x); +LL | if y = (Foo { foo: x }) { | - here the type of `x` is inferred to be `usize` ... LL | if x == x && x = x && x == x { @@ -81,10 +78,7 @@ LL | if x == x && x = x && x == x { error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:44:22 | -LL | let x = 1; - | - here the type of `x` is inferred to be `{integer}` -... -LL | println!("{}", x); +LL | if y = (Foo { foo: x }) { | - here the type of `x` is inferred to be `usize` ... LL | if x == x && x = x && x == x { @@ -104,10 +98,7 @@ LL | if x == x && x == x && x == x { error[E0308]: mismatched types --> $DIR/assignment-in-if.rs:51:28 | -LL | let x = 1; - | - here the type of `x` is inferred to be `{integer}` -... -LL | println!("{}", x); +LL | if y = (Foo { foo: x }) { | - here the type of `x` is inferred to be `usize` ... LL | if x == x && x == x && x = x { diff --git a/src/test/ui/type/type-check/point-at-inference-2.rs b/src/test/ui/type/type-check/point-at-inference-2.rs new file mode 100644 index 00000000000..6557d7fa191 --- /dev/null +++ b/src/test/ui/type/type-check/point-at-inference-2.rs @@ -0,0 +1,13 @@ +fn bar(_: Vec) {} +fn baz(_: &Vec<&i32>) {} +fn main() { + let v = vec![&1]; + bar(v); //~ ERROR E0308 + let v = vec![]; + baz(&v); + baz(&v); + bar(v); //~ ERROR E0308 + let v = vec![]; + baz(&v); + bar(v); //~ ERROR E0308 +} diff --git a/src/test/ui/type/type-check/point-at-inference-2.stderr b/src/test/ui/type/type-check/point-at-inference-2.stderr new file mode 100644 index 00000000000..13227c5e245 --- /dev/null +++ b/src/test/ui/type/type-check/point-at-inference-2.stderr @@ -0,0 +1,56 @@ +error[E0308]: mismatched types + --> $DIR/point-at-inference-2.rs:5:9 + | +LL | bar(v); + | --- ^ expected `i32`, found `&{integer}` + | | + | arguments to this function are incorrect + | + = note: expected struct `Vec` + found struct `Vec<&{integer}>` +note: function defined here + --> $DIR/point-at-inference-2.rs:1:4 + | +LL | fn bar(_: Vec) {} + | ^^^ ----------- + +error[E0308]: mismatched types + --> $DIR/point-at-inference-2.rs:9:9 + | +LL | baz(&v); + | - here the type of `v` is inferred to be `Vec<&i32>` +LL | baz(&v); +LL | bar(v); + | --- ^ expected `i32`, found `&i32` + | | + | arguments to this function are incorrect + | + = note: expected struct `Vec` + found struct `Vec<&i32>` +note: function defined here + --> $DIR/point-at-inference-2.rs:1:4 + | +LL | fn bar(_: Vec) {} + | ^^^ ----------- + +error[E0308]: mismatched types + --> $DIR/point-at-inference-2.rs:12:9 + | +LL | baz(&v); + | - here the type of `v` is inferred to be `Vec<&i32>` +LL | bar(v); + | --- ^ expected `i32`, found `&i32` + | | + | arguments to this function are incorrect + | + = note: expected struct `Vec` + found struct `Vec<&i32>` +note: function defined here + --> $DIR/point-at-inference-2.rs:1:4 + | +LL | fn bar(_: Vec) {} + | ^^^ ----------- + +error: aborting due to 3 previous errors + +For more information about this error, try `rustc --explain E0308`. diff --git a/src/test/ui/type/type-check/point-at-inference.fixed b/src/test/ui/type/type-check/point-at-inference.fixed new file mode 100644 index 00000000000..f41fbe59fba --- /dev/null +++ b/src/test/ui/type/type-check/point-at-inference.fixed @@ -0,0 +1,13 @@ +// run-rustfix +fn bar(_: Vec) {} +fn baz(_: &impl std::any::Any) {} +fn main() { + let v = vec![1, 2, 3, 4, 5]; + let mut foo = vec![]; + baz(&foo); + for i in &v { + foo.push(*i); + } + baz(&foo); + bar(foo); //~ ERROR E0308 +} diff --git a/src/test/ui/type/type-check/point-at-inference.rs b/src/test/ui/type/type-check/point-at-inference.rs new file mode 100644 index 00000000000..6419e42e70d --- /dev/null +++ b/src/test/ui/type/type-check/point-at-inference.rs @@ -0,0 +1,13 @@ +// run-rustfix +fn bar(_: Vec) {} +fn baz(_: &impl std::any::Any) {} +fn main() { + let v = vec![1, 2, 3, 4, 5]; + let mut foo = vec![]; + baz(&foo); + for i in &v { + foo.push(i); + } + baz(&foo); + bar(foo); //~ ERROR E0308 +} diff --git a/src/test/ui/type/type-check/point-at-inference.stderr b/src/test/ui/type/type-check/point-at-inference.stderr new file mode 100644 index 00000000000..197511bf64e --- /dev/null +++ b/src/test/ui/type/type-check/point-at-inference.stderr @@ -0,0 +1,26 @@ +error[E0308]: mismatched types + --> $DIR/point-at-inference.rs:12:9 + | +LL | foo.push(i); + | - this is of type `&{integer}`, which makes `foo` to be inferred as `Vec<&{integer}>` +... +LL | bar(foo); + | --- ^^^ expected `i32`, found `&{integer}` + | | + | arguments to this function are incorrect + | + = note: expected struct `Vec` + found struct `Vec<&{integer}>` +note: function defined here + --> $DIR/point-at-inference.rs:2:4 + | +LL | fn bar(_: Vec) {} + | ^^^ ----------- +help: consider dereferencing the borrow + | +LL | foo.push(*i); + | + + +error: aborting due to previous error + +For more information about this error, try `rustc --explain E0308`. -- 2.44.0