Skip to content

Commit

Permalink
On type mismatch caused by assignment, point at assignee
Browse files Browse the repository at this point in the history
* Do not emit unnecessary E0308 after E0070
* Show fewer errors on `while let` missing `let`
* Hide redundant E0308 on `while let` missing `let`
* Point at binding definition when possible on invalid assignment
* do not point at closure twice
* do not suggest `if let` for literals in lhs
* account for parameter types
  • Loading branch information
estebank committed Nov 25, 2021
1 parent 862962b commit 37a11a9
Show file tree
Hide file tree
Showing 39 changed files with 297 additions and 117 deletions.
16 changes: 12 additions & 4 deletions compiler/rustc_ast_lowering/src/expr.rs
Expand Up @@ -915,14 +915,22 @@ impl<'hir> LoweringContext<'_, 'hir> {
);
}
if !self.sess.features_untracked().destructuring_assignment {
feature_err(
let mut err = feature_err(
&self.sess.parse_sess,
sym::destructuring_assignment,
eq_sign_span,
"destructuring assignments are unstable",
)
.span_label(lhs.span, "cannot assign to this expression")
.emit();
);
err.span_label(lhs.span, "cannot assign to this expression");
if self.is_in_loop_condition {
err.span_suggestion_verbose(
lhs.span.shrink_to_lo(),
"you might have meant to use pattern destructuring",
"let ".to_string(),
rustc_errors::Applicability::MachineApplicable,
);
}
err.emit();
}

let mut assignments = vec![];
Expand Down
20 changes: 16 additions & 4 deletions compiler/rustc_typeck/src/check/coercion.rs
Expand Up @@ -1458,7 +1458,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
cause,
expected,
found,
coercion_error,
coercion_error.clone(),
fcx,
parent_id,
expression.map(|expr| (expr, blk_id)),
Expand All @@ -1472,7 +1472,7 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
cause,
expected,
found,
coercion_error,
coercion_error.clone(),
fcx,
id,
None,
Expand All @@ -1483,7 +1483,12 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}
}
_ => {
err = fcx.report_mismatched_types(cause, expected, found, coercion_error);
err = fcx.report_mismatched_types(
cause,
expected,
found,
coercion_error.clone(),
);
}
}

Expand All @@ -1492,7 +1497,14 @@ impl<'tcx, 'exprs, E: AsCoercionSite> CoerceMany<'tcx, 'exprs, E> {
}

if let Some(expr) = expression {
fcx.emit_coerce_suggestions(&mut err, expr, found, expected, None);
fcx.emit_coerce_suggestions(
&mut err,
expr,
found,
expected,
None,
coercion_error,
);
}

err.emit_unless(unsized_return);
Expand Down
81 changes: 74 additions & 7 deletions compiler/rustc_typeck/src/check/demand.rs
Expand Up @@ -10,6 +10,7 @@ use rustc_hir::lang_items::LangItem;
use rustc_hir::{is_range_literal, Node};
use rustc_middle::lint::in_external_macro;
use rustc_middle::ty::adjustment::AllowTwoPhase;
use rustc_middle::ty::error::{ExpectedFound, TypeError};
use rustc_middle::ty::print::with_no_trimmed_paths;
use rustc_middle::ty::{self, AssocItem, Ty, TypeAndMut};
use rustc_span::symbol::sym;
Expand All @@ -27,8 +28,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
expr_ty: Ty<'tcx>,
expected: Ty<'tcx>,
expected_ty_expr: Option<&'tcx hir::Expr<'tcx>>,
error: TypeError<'tcx>,
) {
self.annotate_expected_due_to_let_ty(err, expr);
self.annotate_expected_due_to_let_ty(err, expr, error);
self.suggest_box_deref(err, expr, expected, expr_ty);
self.suggest_compatible_variants(err, expr, expected, expr_ty);
self.suggest_deref_ref_or_into(err, expr, expected, expr_ty, expected_ty_expr);
Expand Down Expand Up @@ -145,9 +147,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let expr = expr.peel_drop_temps();
let cause = self.misc(expr.span);
let expr_ty = self.resolve_vars_with_obligations(checked_ty);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e);
let mut err = self.report_mismatched_types(&cause, expected, expr_ty, e.clone());

self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr);
self.emit_coerce_suggestions(&mut err, expr, expr_ty, expected, expected_ty_expr, e);

(expected, Some(err))
}
Expand All @@ -156,15 +158,80 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
err: &mut DiagnosticBuilder<'_>,
expr: &hir::Expr<'_>,
error: TypeError<'_>,
) {
let parent = self.tcx.hir().get_parent_node(expr.hir_id);
if let Some(hir::Node::Local(hir::Local { ty: Some(ty), init: Some(init), .. })) =
self.tcx.hir().find(parent)
{
if init.hir_id == expr.hir_id {
match (self.tcx.hir().find(parent), error) {
(Some(hir::Node::Local(hir::Local { ty: Some(ty), init: Some(init), .. })), _)
if init.hir_id == expr.hir_id =>
{
// Point at `let` assignment type.
err.span_label(ty.span, "expected due to this");
}
(
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Assign(lhs, rhs, _), ..
})),
TypeError::Sorts(ExpectedFound { expected, .. }),
) if rhs.hir_id == expr.hir_id && !expected.is_closure() => {
// We ignore closures explicitly because we already point at them elsewhere.
// Point at the assigned-to binding.
let mut primary_span = lhs.span;
let mut secondary_span = lhs.span;
let mut post_message = "";
if let hir::ExprKind::Path(hir::QPath::Resolved(
None,
hir::Path { res: hir::def::Res::Local(hir_id), .. },
)) = lhs.kind
{
if let Some(hir::Node::Binding(pat)) = self.tcx.hir().find(*hir_id) {
let parent = self.tcx.hir().get_parent_node(pat.hir_id);
primary_span = pat.span;
secondary_span = pat.span;
match self.tcx.hir().find(parent) {
Some(hir::Node::Local(hir::Local { ty: Some(ty), .. })) => {
primary_span = ty.span;
post_message = " type";
}
Some(hir::Node::Local(hir::Local { init: Some(init), .. })) => {
primary_span = init.span;
post_message = " value";
}
Some(hir::Node::Param(hir::Param { ty_span, .. })) => {
primary_span = *ty_span;
post_message = " parameter type";
}
_ => {}
}
}
}

if primary_span != secondary_span
&& self
.tcx
.sess
.source_map()
.is_multiline(secondary_span.shrink_to_hi().until(primary_span))
{
// We are pointing at the binding's type or initializer value, but it's pattern
// is in a different line, so we point at both.
err.span_label(secondary_span, "expected due to the type of this binding");
err.span_label(primary_span, &format!("expected due to this{}", post_message));
} else if post_message == "" {
// We are pointing at either the assignment lhs or the binding def pattern.
err.span_label(primary_span, "expected due to the type of this binding");
} else {
// We are pointing at the binding's type or initializer value.
err.span_label(primary_span, &format!("expected due to this{}", post_message));
}

if !lhs.is_syntactic_place_expr() {
// We already emitted E0070 "invalid left-hand side of assignment", so we
// silence this.
err.delay_as_bug();
}
}
_ => {}
}
}

Expand Down
57 changes: 52 additions & 5 deletions compiler/rustc_typeck/src/check/expr.rs
Expand Up @@ -833,19 +833,66 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&self,
lhs: &'tcx hir::Expr<'tcx>,
err_code: &'static str,
expr_span: &Span,
op_span: Span,
) {
if lhs.is_syntactic_place_expr() {
return;
}

// FIXME: Make this use SessionDiagnostic once error codes can be dynamically set.
let mut err = self.tcx.sess.struct_span_err_with_code(
*expr_span,
op_span,
"invalid left-hand side of assignment",
DiagnosticId::Error(err_code.into()),
);
err.span_label(lhs.span, "cannot assign to this expression");

let mut parent = self.tcx.hir().get_parent_node(lhs.hir_id);
while let Some(node) = self.tcx.hir().find(parent) {
match node {
hir::Node::Expr(hir::Expr {
kind:
hir::ExprKind::Loop(
hir::Block {
expr:
Some(hir::Expr {
kind:
hir::ExprKind::Match(expr, ..) | hir::ExprKind::If(expr, ..),
..
}),
..
},
_,
hir::LoopSource::While,
_,
),
..
}) => {
// We have a situation like `while Some(0) = value.get(0) {`, where `while let`
// was more likely intended.
err.span_suggestion_verbose(
expr.span.shrink_to_lo(),
"you might have meant to use pattern destructuring",
"let ".to_string(),
Applicability::MachineApplicable,
);
if !self.sess().features_untracked().destructuring_assignment {
// We already emit an E0658 with a suggestion for `while let`, this is
// redundant output.
err.delay_as_bug();
}
break;
}
hir::Node::Item(_)
| hir::Node::ImplItem(_)
| hir::Node::TraitItem(_)
| hir::Node::Crate(_) => break,
_ => {
parent = self.tcx.hir().get_parent_node(parent);
}
}
}

err.emit();
}

Expand Down Expand Up @@ -953,7 +1000,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
} else {
(Applicability::MaybeIncorrect, false)
};
if !lhs.is_syntactic_place_expr() {
if !lhs.is_syntactic_place_expr() && !matches!(lhs.kind, hir::ExprKind::Lit(_)) {
// Do not suggest `if let x = y` as `==` is way more likely to be the intention.
let hir = self.tcx.hir();
if let hir::Node::Expr(hir::Expr { kind: ExprKind::If { .. }, .. }) =
Expand All @@ -965,7 +1012,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
"let ".to_string(),
applicability,
);
}
};
}
if eq {
err.span_suggestion_verbose(
Expand All @@ -986,7 +1033,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return self.tcx.ty_error();
}

self.check_lhs_assignable(lhs, "E0070", span);
self.check_lhs_assignable(lhs, "E0070", *span);

let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty, Some(lhs));
Expand Down
18 changes: 18 additions & 0 deletions compiler/rustc_typeck/src/check/fn_ctxt/checks.rs
Expand Up @@ -737,6 +737,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
&mut |err| {
if let Some(expected_ty) = expected.only_has_type(self) {
self.consider_hint_about_removing_semicolon(blk, expected_ty, err);
if expected_ty == self.tcx.types.bool {
// If this is caused by a missing `let` in a `while let`,
// silence this redundant error, as we already emit E0070.
let parent = self.tcx.hir().get_parent_node(blk.hir_id);
let parent = self.tcx.hir().get_parent_node(parent);
let parent = self.tcx.hir().get_parent_node(parent);
let parent = self.tcx.hir().get_parent_node(parent);
let parent = self.tcx.hir().get_parent_node(parent);
match self.tcx.hir().find(parent) {
Some(hir::Node::Expr(hir::Expr {
kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _),
..
})) => {
err.delay_as_bug();
}
_ => {}
}
}
}
if let Some(fn_span) = fn_span {
err.span_label(
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_typeck/src/check/op.rs
Expand Up @@ -42,7 +42,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
return_ty
};

self.check_lhs_assignable(lhs, "E0067", &op.span);
self.check_lhs_assignable(lhs, "E0067", op.span);

ty
}
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/dst/dst-bad-assign-3.stderr
Expand Up @@ -2,7 +2,9 @@ error[E0308]: mismatched types
--> $DIR/dst-bad-assign-3.rs:33:12
|
LL | f5.2 = Bar1 {f: 36};
| ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
| ---- ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
| |
| expected due to the type of this binding
|
= note: expected trait object `dyn ToBar`
found struct `Bar1`
Expand Down
4 changes: 3 additions & 1 deletion src/test/ui/dst/dst-bad-assign.stderr
Expand Up @@ -2,7 +2,9 @@ error[E0308]: mismatched types
--> $DIR/dst-bad-assign.rs:35:14
|
LL | f5.ptr = Bar1 {f: 36};
| ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
| ------ ^^^^^^^^^^^^ expected trait object `dyn ToBar`, found struct `Bar1`
| |
| expected due to the type of this binding
|
= note: expected trait object `dyn ToBar`
found struct `Bar1`
Expand Down
1 change: 0 additions & 1 deletion src/test/ui/error-codes/E0070.rs
Expand Up @@ -6,7 +6,6 @@ fn some_function() {
SOME_CONST = 14; //~ ERROR E0070
1 = 3; //~ ERROR E0070
some_other_func() = 4; //~ ERROR E0070
//~^ ERROR E0308
}

fn main() {
Expand Down
11 changes: 2 additions & 9 deletions src/test/ui/error-codes/E0070.stderr
Expand Up @@ -22,13 +22,6 @@ LL | some_other_func() = 4;
| |
| cannot assign to this expression

error[E0308]: mismatched types
--> $DIR/E0070.rs:8:25
|
LL | some_other_func() = 4;
| ^ expected `()`, found integer

error: aborting due to 4 previous errors
error: aborting due to 3 previous errors

Some errors have detailed explanations: E0070, E0308.
For more information about an error, try `rustc --explain E0070`.
For more information about this error, try `rustc --explain E0070`.
@@ -1,10 +1,10 @@
// Test that we use fully-qualified type names in error messages.

fn main() {
let x: Option<usize>;
let x: //~ NOTE expected due to the type of this binding
Option<usize>; //~ NOTE expected due to this type
x = 5;
//~^ ERROR mismatched types
//~| expected enum `Option<usize>`
//~| found type `{integer}`
//~| expected enum `Option`, found integer
//~| NOTE expected enum `Option<usize>`
//~| NOTE expected enum `Option`, found integer
}
@@ -1,6 +1,10 @@
error[E0308]: mismatched types
--> $DIR/fully-qualified-type-name1.rs:5:9
--> $DIR/fully-qualified-type-name1.rs:6:9
|
LL | let x:
| - expected due to the type of this binding
LL | Option<usize>;
| ------------- expected due to this type
LL | x = 5;
| ^ expected enum `Option`, found integer
|
Expand Down

0 comments on commit 37a11a9

Please sign in to comment.