Skip to content

Commit 4164761

Browse files
CentrilCentril
authored
Rollup merge of #67538 - varkor:lhs-assign-diagnostics, r=Centril
Improve diagnostics for invalid assignment - Improve wording and span information for invalid assignment diagnostics. - Link to rust-lang/rfcs#372 when it appears the user is trying a destructuring assignment. - Make the equality constraint in `where` clauses error consistent with the invalid assignment error.
2 parents 3a07f3b + 9e50813 commit 4164761

37 files changed

+341
-105
lines changed

Diff for: ‎src/librustc/hir/intravisit.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1043,9 +1043,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
10431043
walk_list!(visitor, visit_label, opt_label);
10441044
visitor.visit_block(block);
10451045
}
1046-
ExprKind::Assign(ref left_hand_expression, ref right_hand_expression) => {
1047-
visitor.visit_expr(right_hand_expression);
1048-
visitor.visit_expr(left_hand_expression)
1046+
ExprKind::Assign(ref lhs, ref rhs, _) => {
1047+
visitor.visit_expr(rhs);
1048+
visitor.visit_expr(lhs)
10491049
}
10501050
ExprKind::AssignOp(_, ref left_expression, ref right_expression) => {
10511051
visitor.visit_expr(right_expression);

Diff for: ‎src/librustc/hir/lowering/expr.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -122,8 +122,8 @@ impl LoweringContext<'_, '_> {
122122
self.lower_block(blk, opt_label.is_some()),
123123
self.lower_label(opt_label),
124124
),
125-
ExprKind::Assign(ref el, ref er) => {
126-
hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er)))
125+
ExprKind::Assign(ref el, ref er, span) => {
126+
hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er)), span)
127127
}
128128
ExprKind::AssignOp(op, ref el, ref er) => hir::ExprKind::AssignOp(
129129
self.lower_binop(op),
@@ -994,8 +994,11 @@ impl LoweringContext<'_, '_> {
994994
let (val_pat, val_pat_hid) = self.pat_ident(pat.span, val_ident);
995995
let val_expr = P(self.expr_ident(pat.span, val_ident, val_pat_hid));
996996
let next_expr = P(self.expr_ident(pat.span, next_ident, next_pat_hid));
997-
let assign =
998-
P(self.expr(pat.span, hir::ExprKind::Assign(next_expr, val_expr), ThinVec::new()));
997+
let assign = P(self.expr(
998+
pat.span,
999+
hir::ExprKind::Assign(next_expr, val_expr, pat.span),
1000+
ThinVec::new(),
1001+
));
9991002
let some_pat = self.pat_some(pat.span, val_pat);
10001003
self.arm(some_pat, assign)
10011004
};

Diff for: ‎src/librustc/hir/mod.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1690,7 +1690,8 @@ pub enum ExprKind {
16901690
Block(P<Block>, Option<Label>),
16911691

16921692
/// An assignment (e.g., `a = foo()`).
1693-
Assign(P<Expr>, P<Expr>),
1693+
/// The `Span` argument is the span of the `=` token.
1694+
Assign(P<Expr>, P<Expr>, Span),
16941695
/// An assignment with an operator.
16951696
///
16961697
/// E.g., `a += 1`.

Diff for: ‎src/librustc/hir/print.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1289,7 +1289,7 @@ impl<'a> State<'a> {
12891289
self.ibox(0);
12901290
self.print_block(&blk);
12911291
}
1292-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
1292+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
12931293
let prec = AssocOp::Assign.precedence() as i8;
12941294
self.print_expr_maybe_paren(&lhs, prec + 1);
12951295
self.s.space();
@@ -2265,7 +2265,7 @@ fn contains_exterior_struct_lit(value: &hir::Expr) -> bool {
22652265
match value.kind {
22662266
hir::ExprKind::Struct(..) => true,
22672267

2268-
hir::ExprKind::Assign(ref lhs, ref rhs)
2268+
hir::ExprKind::Assign(ref lhs, ref rhs, _)
22692269
| hir::ExprKind::AssignOp(_, ref lhs, ref rhs)
22702270
| hir::ExprKind::Binary(_, ref lhs, ref rhs) => {
22712271
// `X { y: 1 } + X { y: 2 }`

Diff for: ‎src/librustc_lint/unused.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ impl EarlyLintPass for UnusedParens {
490490
(value, "`return` value", false, Some(left), None)
491491
}
492492

493-
Assign(_, ref value) => (value, "assigned value", false, None, None),
493+
Assign(_, ref value, _) => (value, "assigned value", false, None, None),
494494
AssignOp(.., ref value) => (value, "assigned value", false, None, None),
495495
// either function/method call, or something this lint doesn't care about
496496
ref call_or_other => {

Diff for: ‎src/librustc_mir/hair/cx/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(cx: &mut Cx<'a, 'tcx>, expr: &'tcx hir::Expr
227227

228228
hir::ExprKind::Block(ref blk, _) => ExprKind::Block { body: &blk },
229229

230-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
230+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
231231
ExprKind::Assign { lhs: lhs.to_ref(), rhs: rhs.to_ref() }
232232
}
233233

Diff for: ‎src/librustc_parse/parser/expr.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -281,7 +281,9 @@ impl<'a> Parser<'a> {
281281
let binary = self.mk_binary(source_map::respan(cur_op_span, ast_op), lhs, rhs);
282282
self.mk_expr(span, binary, AttrVec::new())
283283
}
284-
AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs), AttrVec::new()),
284+
AssocOp::Assign => {
285+
self.mk_expr(span, ExprKind::Assign(lhs, rhs, cur_op_span), AttrVec::new())
286+
}
285287
AssocOp::AssignOp(k) => {
286288
let aop = match k {
287289
token::Plus => BinOpKind::Add,

Diff for: ‎src/librustc_passes/ast_validation.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -737,8 +737,14 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
737737
for predicate in &generics.where_clause.predicates {
738738
if let WherePredicate::EqPredicate(ref predicate) = *predicate {
739739
self.err_handler()
740-
.span_err(predicate.span, "equality constraints are not yet \
741-
supported in where clauses (see #20041)");
740+
.struct_span_err(
741+
predicate.span,
742+
"equality constraints are not yet supported in `where` clauses",
743+
)
744+
.note(
745+
"for more information, see https://github.com/rust-lang/rust/issues/20041",
746+
)
747+
.emit();
742748
}
743749
}
744750

Diff for: ‎src/librustc_passes/liveness.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10791079
.unwrap_or_else(|| span_bug!(expr.span, "continue to unknown label"))
10801080
}
10811081

1082-
hir::ExprKind::Assign(ref l, ref r) => {
1082+
hir::ExprKind::Assign(ref l, ref r, _) => {
10831083
// see comment on places in
10841084
// propagate_through_place_components()
10851085
let succ = self.write_place(&l, succ, ACC_WRITE);
@@ -1373,7 +1373,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> {
13731373

13741374
fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr) {
13751375
match expr.kind {
1376-
hir::ExprKind::Assign(ref l, _) => {
1376+
hir::ExprKind::Assign(ref l, ..) => {
13771377
this.check_place(&l);
13781378
}
13791379

Diff for: ‎src/librustc_privacy/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1251,7 +1251,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
12511251
return;
12521252
}
12531253
match expr.kind {
1254-
hir::ExprKind::Assign(.., ref rhs) | hir::ExprKind::Match(ref rhs, ..) => {
1254+
hir::ExprKind::Assign(_, ref rhs, _) | hir::ExprKind::Match(ref rhs, ..) => {
12551255
// Do not report duplicate errors for `x = y` and `match x { ... }`.
12561256
if self.check_expr_pat_type(rhs.hir_id, rhs.span) {
12571257
return;

Diff for: ‎src/librustc_typeck/check/demand.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
490490
String::new()
491491
};
492492
if let Some(hir::Node::Expr(hir::Expr {
493-
kind: hir::ExprKind::Assign(left_expr, _),
493+
kind: hir::ExprKind::Assign(left_expr, ..),
494494
..
495495
})) = self.tcx.hir().find(self.tcx.hir().get_parent_node(expr.hir_id))
496496
{

Diff for: ‎src/librustc_typeck/check/expr.rs

+41-6
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::check::TupleArgumentsFlag::DontTupleArguments;
1717
use crate::util::common::ErrorReported;
1818
use crate::util::nodemap::FxHashMap;
1919

20-
use errors::{pluralize, Applicability, DiagnosticBuilder};
20+
use errors::{pluralize, Applicability, DiagnosticBuilder, DiagnosticId};
2121
use rustc::hir;
2222
use rustc::hir::def::{CtorKind, DefKind, Res};
2323
use rustc::hir::def_id::DefId;
@@ -219,6 +219,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
219219
ExprKind::Box(ref subexpr) => self.check_expr_box(subexpr, expected),
220220
ExprKind::Lit(ref lit) => self.check_lit(&lit, expected),
221221
ExprKind::Binary(op, ref lhs, ref rhs) => self.check_binop(expr, op, lhs, rhs),
222+
ExprKind::Assign(ref lhs, ref rhs, ref span) => {
223+
self.check_expr_assign(expr, expected, lhs, rhs, span)
224+
}
222225
ExprKind::AssignOp(op, ref lhs, ref rhs) => self.check_binop_assign(expr, op, lhs, rhs),
223226
ExprKind::Unary(unop, ref oprnd) => {
224227
self.check_expr_unary(unop, oprnd, expected, needs, expr)
@@ -245,7 +248,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
245248
}
246249
}
247250
ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr),
248-
ExprKind::Assign(ref lhs, ref rhs) => self.check_expr_assign(expr, expected, lhs, rhs),
249251
ExprKind::Loop(ref body, _, source) => {
250252
self.check_expr_loop(body, source, expected, expr)
251253
}
@@ -723,6 +725,40 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
723725
);
724726
}
725727

728+
fn is_destructuring_place_expr(&self, expr: &'tcx hir::Expr) -> bool {
729+
match &expr.kind {
730+
ExprKind::Array(comps) | ExprKind::Tup(comps) => {
731+
comps.iter().all(|e| self.is_destructuring_place_expr(e))
732+
}
733+
ExprKind::Struct(_path, fields, rest) => {
734+
rest.as_ref().map(|e| self.is_destructuring_place_expr(e)).unwrap_or(true)
735+
&& fields.iter().all(|f| self.is_destructuring_place_expr(&f.expr))
736+
}
737+
_ => expr.is_syntactic_place_expr(),
738+
}
739+
}
740+
741+
pub(crate) fn check_lhs_assignable(
742+
&self,
743+
lhs: &'tcx hir::Expr,
744+
err_code: &'static str,
745+
expr_span: &Span,
746+
) {
747+
if !lhs.is_syntactic_place_expr() {
748+
let mut err = self.tcx.sess.struct_span_err_with_code(
749+
*expr_span,
750+
"invalid left-hand side of assignment",
751+
DiagnosticId::Error(err_code.into()),
752+
);
753+
err.span_label(lhs.span, "cannot assign to this expression");
754+
if self.is_destructuring_place_expr(lhs) {
755+
err.note("destructuring assignments are not currently supported");
756+
err.note("for more information, see https://github.com/rust-lang/rfcs/issues/372");
757+
}
758+
err.emit();
759+
}
760+
}
761+
726762
/// Type check assignment expression `expr` of form `lhs = rhs`.
727763
/// The expected type is `()` and is passsed to the function for the purposes of diagnostics.
728764
fn check_expr_assign(
@@ -731,6 +767,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
731767
expected: Expectation<'tcx>,
732768
lhs: &'tcx hir::Expr,
733769
rhs: &'tcx hir::Expr,
770+
span: &Span,
734771
) -> Ty<'tcx> {
735772
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
736773
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);
@@ -752,10 +789,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
752789
err.help(msg);
753790
}
754791
err.emit();
755-
} else if !lhs.is_syntactic_place_expr() {
756-
struct_span_err!(self.tcx.sess, expr.span, E0070, "invalid left-hand side expression")
757-
.span_label(expr.span, "left-hand of expression not valid")
758-
.emit();
792+
} else {
793+
self.check_lhs_assignable(lhs, "E0070", span);
759794
}
760795

761796
self.require_type_is_sized(lhs_ty, lhs.span, traits::AssignmentLhsSized);

Diff for: ‎src/librustc_typeck/check/op.rs

+6-14
Original file line numberDiff line numberDiff line change
@@ -19,30 +19,22 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
1919
&self,
2020
expr: &'tcx hir::Expr,
2121
op: hir::BinOp,
22-
lhs_expr: &'tcx hir::Expr,
23-
rhs_expr: &'tcx hir::Expr,
22+
lhs: &'tcx hir::Expr,
23+
rhs: &'tcx hir::Expr,
2424
) -> Ty<'tcx> {
2525
let (lhs_ty, rhs_ty, return_ty) =
26-
self.check_overloaded_binop(expr, lhs_expr, rhs_expr, op, IsAssign::Yes);
26+
self.check_overloaded_binop(expr, lhs, rhs, op, IsAssign::Yes);
2727

2828
let ty =
2929
if !lhs_ty.is_ty_var() && !rhs_ty.is_ty_var() && is_builtin_binop(lhs_ty, rhs_ty, op) {
30-
self.enforce_builtin_binop_types(lhs_expr, lhs_ty, rhs_expr, rhs_ty, op);
30+
self.enforce_builtin_binop_types(lhs, lhs_ty, rhs, rhs_ty, op);
3131
self.tcx.mk_unit()
3232
} else {
3333
return_ty
3434
};
3535

36-
if !lhs_expr.is_syntactic_place_expr() {
37-
struct_span_err!(
38-
self.tcx.sess,
39-
lhs_expr.span,
40-
E0067,
41-
"invalid left-hand side expression"
42-
)
43-
.span_label(lhs_expr.span, "invalid expression for left-hand side")
44-
.emit();
45-
}
36+
self.check_lhs_assignable(lhs, "E0067", &op.span);
37+
4638
ty
4739
}
4840

Diff for: ‎src/librustc_typeck/expr_use_visitor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
286286
}
287287
}
288288

289-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
289+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
290290
self.mutate_expr(lhs);
291291
self.consume_expr(rhs);
292292
}

Diff for: ‎src/libsyntax/ast.rs

+2-1
Original file line numberDiff line numberDiff line change
@@ -1216,7 +1216,8 @@ pub enum ExprKind {
12161216
TryBlock(P<Block>),
12171217

12181218
/// An assignment (`a = foo()`).
1219-
Assign(P<Expr>, P<Expr>),
1219+
/// The `Span` argument is the span of the `=` token.
1220+
Assign(P<Expr>, P<Expr>, Span),
12201221
/// An assignment with an operator.
12211222
///
12221223
/// E.g., `a += 1`.

Diff for: ‎src/libsyntax/mut_visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1168,7 +1168,7 @@ pub fn noop_visit_expr<T: MutVisitor>(Expr { kind, id, span, attrs }: &mut Expr,
11681168
vis.visit_block(body);
11691169
}
11701170
ExprKind::Await(expr) => vis.visit_expr(expr),
1171-
ExprKind::Assign(el, er) => {
1171+
ExprKind::Assign(el, er, _) => {
11721172
vis.visit_expr(el);
11731173
vis.visit_expr(er);
11741174
}

Diff for: ‎src/libsyntax/print/pprust.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2041,7 +2041,7 @@ impl<'a> State<'a> {
20412041
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
20422042
self.s.word(".await");
20432043
}
2044-
ast::ExprKind::Assign(ref lhs, ref rhs) => {
2044+
ast::ExprKind::Assign(ref lhs, ref rhs, _) => {
20452045
let prec = AssocOp::Assign.precedence() as i8;
20462046
self.print_expr_maybe_paren(lhs, prec + 1);
20472047
self.s.space();

Diff for: ‎src/libsyntax/util/parser.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ pub fn contains_exterior_struct_lit(value: &ast::Expr) -> bool {
378378
match value.kind {
379379
ast::ExprKind::Struct(..) => true,
380380

381-
ast::ExprKind::Assign(ref lhs, ref rhs)
381+
ast::ExprKind::Assign(ref lhs, ref rhs, _)
382382
| ast::ExprKind::AssignOp(_, ref lhs, ref rhs)
383383
| ast::ExprKind::Binary(_, ref lhs, ref rhs) => {
384384
// X { y: 1 } + X { y: 2 }

Diff for: ‎src/libsyntax/visit.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -766,9 +766,9 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
766766
visitor.visit_block(body);
767767
}
768768
ExprKind::Await(ref expr) => visitor.visit_expr(expr),
769-
ExprKind::Assign(ref left_hand_expression, ref right_hand_expression) => {
770-
visitor.visit_expr(left_hand_expression);
771-
visitor.visit_expr(right_hand_expression);
769+
ExprKind::Assign(ref lhs, ref rhs, _) => {
770+
visitor.visit_expr(lhs);
771+
visitor.visit_expr(rhs);
772772
}
773773
ExprKind::AssignOp(_, ref left_expression, ref right_expression) => {
774774
visitor.visit_expr(left_expression);

Diff for: ‎src/test/ui-fulldeps/pprust-expr-roundtrip.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,8 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P<Expr>)) {
126126
DUMMY_SP)));
127127
},
128128
12 => {
129-
iter_exprs(depth - 1, &mut |e| g(ExprKind::Assign(e, make_x())));
130-
iter_exprs(depth - 1, &mut |e| g(ExprKind::Assign(make_x(), e)));
129+
iter_exprs(depth - 1, &mut |e| g(ExprKind::Assign(e, make_x(), DUMMY_SP)));
130+
iter_exprs(depth - 1, &mut |e| g(ExprKind::Assign(make_x(), e, DUMMY_SP)));
131131
},
132132
13 => {
133133
iter_exprs(depth - 1, &mut |e| g(ExprKind::Field(e, Ident::from_str("f"))));

Diff for: ‎src/test/ui/bad/bad-expr-lhs.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
fn main() {
2-
1 = 2; //~ ERROR invalid left-hand side expression
3-
1 += 2; //~ ERROR invalid left-hand side expression
4-
(1, 2) = (3, 4); //~ ERROR invalid left-hand side expression
2+
1 = 2; //~ ERROR invalid left-hand side of assignment
3+
1 += 2; //~ ERROR invalid left-hand side of assignment
4+
(1, 2) = (3, 4); //~ ERROR invalid left-hand side of assignment
55

66
let (a, b) = (1, 2);
7-
(a, b) = (3, 4); //~ ERROR invalid left-hand side expression
7+
(a, b) = (3, 4); //~ ERROR invalid left-hand side of assignment
88

9-
None = Some(3); //~ ERROR invalid left-hand side expression
9+
None = Some(3); //~ ERROR invalid left-hand side of assignment
1010
}

0 commit comments

Comments
 (0)