Skip to content

Commit f8992d5

Browse files
CentrilCentril
authored
Rollup merge of rust-lang#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 e1e5348 + 770725c commit f8992d5

37 files changed

+339
-106
lines changed

‎src/librustc/hir/intravisit.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -1056,9 +1056,9 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) {
10561056
walk_list!(visitor, visit_label, opt_label);
10571057
visitor.visit_block(block);
10581058
}
1059-
ExprKind::Assign(ref left_hand_expression, ref right_hand_expression) => {
1060-
visitor.visit_expr(right_hand_expression);
1061-
visitor.visit_expr(left_hand_expression)
1059+
ExprKind::Assign(ref lhs, ref rhs, _) => {
1060+
visitor.visit_expr(rhs);
1061+
visitor.visit_expr(lhs)
10621062
}
10631063
ExprKind::AssignOp(_, ref left_expression, ref right_expression) => {
10641064
visitor.visit_expr(right_expression);

‎src/librustc/hir/lowering/expr.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -112,8 +112,8 @@ impl LoweringContext<'_, '_> {
112112
opt_label.is_some()),
113113
self.lower_label(opt_label))
114114
}
115-
ExprKind::Assign(ref el, ref er) => {
116-
hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er)))
115+
ExprKind::Assign(ref el, ref er, span) => {
116+
hir::ExprKind::Assign(P(self.lower_expr(el)), P(self.lower_expr(er)), span)
117117
}
118118
ExprKind::AssignOp(op, ref el, ref er) => hir::ExprKind::AssignOp(
119119
self.lower_binop(op),
@@ -1084,7 +1084,7 @@ impl LoweringContext<'_, '_> {
10841084
let next_expr = P(self.expr_ident(pat.span, next_ident, next_pat_hid));
10851085
let assign = P(self.expr(
10861086
pat.span,
1087-
hir::ExprKind::Assign(next_expr, val_expr),
1087+
hir::ExprKind::Assign(next_expr, val_expr, pat.span),
10881088
ThinVec::new(),
10891089
));
10901090
let some_pat = self.pat_some(pat.span, val_pat);

‎src/librustc/hir/mod.rs

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

17241724
/// An assignment (e.g., `a = foo()`).
1725-
Assign(P<Expr>, P<Expr>),
1725+
/// The `Span` argument is the span of the `=` token.
1726+
Assign(P<Expr>, P<Expr>, Span),
17261727
/// An assignment with an operator.
17271728
///
17281729
/// E.g., `a += 1`.

‎src/librustc/hir/print.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1312,7 +1312,7 @@ impl<'a> State<'a> {
13121312
self.ibox(0);
13131313
self.print_block(&blk);
13141314
}
1315-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
1315+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
13161316
let prec = AssocOp::Assign.precedence() as i8;
13171317
self.print_expr_maybe_paren(&lhs, prec + 1);
13181318
self.s.space();
@@ -2282,7 +2282,7 @@ fn contains_exterior_struct_lit(value: &hir::Expr) -> bool {
22822282
match value.kind {
22832283
hir::ExprKind::Struct(..) => true,
22842284

2285-
hir::ExprKind::Assign(ref lhs, ref rhs) |
2285+
hir::ExprKind::Assign(ref lhs, ref rhs, _) |
22862286
hir::ExprKind::AssignOp(_, ref lhs, ref rhs) |
22872287
hir::ExprKind::Binary(_, ref lhs, ref rhs) => {
22882288
// `X { y: 1 } + X { y: 2 }`

‎src/librustc_lint/unused.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -512,7 +512,7 @@ impl EarlyLintPass for UnusedParens {
512512
(value, "`return` value", false, Some(left), None)
513513
}
514514

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

‎src/librustc_mir/hair/cx/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ fn make_mirror_unadjusted<'a, 'tcx>(
274274

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

277-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
277+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
278278
ExprKind::Assign {
279279
lhs: lhs.to_ref(),
280280
rhs: rhs.to_ref(),

‎src/librustc_parse/parser/expr.rs

+3-1
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,9 @@ impl<'a> Parser<'a> {
276276
let binary = self.mk_binary(source_map::respan(cur_op_span, ast_op), lhs, rhs);
277277
self.mk_expr(span, binary, AttrVec::new())
278278
}
279-
AssocOp::Assign => self.mk_expr(span, ExprKind::Assign(lhs, rhs), AttrVec::new()),
279+
AssocOp::Assign => {
280+
self.mk_expr(span, ExprKind::Assign(lhs, rhs, cur_op_span), AttrVec::new())
281+
}
280282
AssocOp::AssignOp(k) => {
281283
let aop = match k {
282284
token::Plus => BinOpKind::Add,

‎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

‎src/librustc_passes/liveness.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1096,7 +1096,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
10961096
span_bug!(expr.span, "continue to unknown label"))
10971097
}
10981098

1099-
hir::ExprKind::Assign(ref l, ref r) => {
1099+
hir::ExprKind::Assign(ref l, ref r, _) => {
11001100
// see comment on places in
11011101
// propagate_through_place_components()
11021102
let succ = self.write_place(&l, succ, ACC_WRITE);
@@ -1389,7 +1389,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> {
13891389

13901390
fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr) {
13911391
match expr.kind {
1392-
hir::ExprKind::Assign(ref l, _) => {
1392+
hir::ExprKind::Assign(ref l, ..) => {
13931393
this.check_place(&l);
13941394
}
13951395

‎src/librustc_privacy/lib.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1198,7 +1198,7 @@ impl<'a, 'tcx> Visitor<'tcx> for TypePrivacyVisitor<'a, 'tcx> {
11981198
return;
11991199
}
12001200
match expr.kind {
1201-
hir::ExprKind::Assign(.., ref rhs) | hir::ExprKind::Match(ref rhs, ..) => {
1201+
hir::ExprKind::Assign(_, ref rhs, _) | hir::ExprKind::Match(ref rhs, ..) => {
12021202
// Do not report duplicate errors for `x = y` and `match x { ... }`.
12031203
if self.check_expr_pat_type(rhs.hir_id, rhs.span) {
12041204
return;

‎src/librustc_typeck/check/demand.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -482,7 +482,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
482482
String::new()
483483
};
484484
if let Some(hir::Node::Expr(hir::Expr {
485-
kind: hir::ExprKind::Assign(left_expr, _),
485+
kind: hir::ExprKind::Assign(left_expr, ..),
486486
..
487487
})) = self.tcx.hir().find(
488488
self.tcx.hir().get_parent_node(expr.hir_id),

‎src/librustc_typeck/check/expr.rs

+43-9
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ use crate::util::common::ErrorReported;
1717
use crate::util::nodemap::FxHashMap;
1818
use crate::astconv::AstConv as _;
1919

20-
use errors::{Applicability, DiagnosticBuilder, pluralize};
20+
use errors::{Applicability, DiagnosticBuilder, DiagnosticId, pluralize};
2121
use syntax_pos::hygiene::DesugaringKind;
2222
use syntax::ast;
2323
use syntax::symbol::{Symbol, kw, sym};
@@ -230,6 +230,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
230230
ExprKind::Binary(op, ref lhs, ref rhs) => {
231231
self.check_binop(expr, op, lhs, rhs)
232232
}
233+
ExprKind::Assign(ref lhs, ref rhs, ref span) => {
234+
self.check_expr_assign(expr, expected, lhs, rhs, span)
235+
}
233236
ExprKind::AssignOp(op, ref lhs, ref rhs) => {
234237
self.check_binop_assign(expr, op, lhs, rhs)
235238
}
@@ -262,9 +265,6 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
262265
ExprKind::Ret(ref expr_opt) => {
263266
self.check_expr_return(expr_opt.as_deref(), expr)
264267
}
265-
ExprKind::Assign(ref lhs, ref rhs) => {
266-
self.check_expr_assign(expr, expected, lhs, rhs)
267-
}
268268
ExprKind::Loop(ref body, _, source) => {
269269
self.check_expr_loop(body, source, expected, expr)
270270
}
@@ -759,6 +759,42 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
759759
);
760760
}
761761

762+
fn is_destructuring_place_expr(&self, expr: &'tcx hir::Expr) -> bool {
763+
match &expr.kind {
764+
ExprKind::Array(comps) | ExprKind::Tup(comps) => {
765+
comps.iter().all(|e| self.is_destructuring_place_expr(e))
766+
}
767+
ExprKind::Struct(_path, fields, rest) => {
768+
rest.as_ref().map(|e| self.is_destructuring_place_expr(e)).unwrap_or(true) &&
769+
fields.iter().all(|f| self.is_destructuring_place_expr(&f.expr))
770+
}
771+
_ => expr.is_syntactic_place_expr(),
772+
}
773+
}
774+
775+
pub(crate) fn check_lhs_assignable(
776+
&self,
777+
lhs: &'tcx hir::Expr,
778+
err_code: &'static str,
779+
expr_span: &Span,
780+
) {
781+
if !lhs.is_syntactic_place_expr() {
782+
let mut err = self.tcx.sess.struct_span_err_with_code(
783+
*expr_span,
784+
"invalid left-hand side of assignment",
785+
DiagnosticId::Error(err_code.into()),
786+
);
787+
err.span_label(lhs.span, "cannot assign to this expression");
788+
if self.is_destructuring_place_expr(lhs) {
789+
err.note("destructuring assignments are not currently supported");
790+
err.note(
791+
"for more information, see https://github.com/rust-lang/rfcs/issues/372",
792+
);
793+
}
794+
err.emit();
795+
}
796+
}
797+
762798
/// Type check assignment expression `expr` of form `lhs = rhs`.
763799
/// The expected type is `()` and is passsed to the function for the purposes of diagnostics.
764800
fn check_expr_assign(
@@ -767,6 +803,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
767803
expected: Expectation<'tcx>,
768804
lhs: &'tcx hir::Expr,
769805
rhs: &'tcx hir::Expr,
806+
span: &Span,
770807
) -> Ty<'tcx> {
771808
let lhs_ty = self.check_expr_with_needs(&lhs, Needs::MutPlace);
772809
let rhs_ty = self.check_expr_coercable_to_type(&rhs, lhs_ty);
@@ -788,11 +825,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
788825
err.help(msg);
789826
}
790827
err.emit();
791-
} else if !lhs.is_syntactic_place_expr() {
792-
struct_span_err!(self.tcx.sess, expr.span, E0070,
793-
"invalid left-hand side expression")
794-
.span_label(expr.span, "left-hand of expression not valid")
795-
.emit();
828+
} else {
829+
self.check_lhs_assignable(lhs, "E0070", span);
796830
}
797831

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

‎src/librustc_typeck/check/op.rs

+6-13
Original file line numberDiff line numberDiff line change
@@ -19,29 +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 = if !lhs_ty.is_ty_var() && !rhs_ty.is_ty_var()
2929
&& 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, lhs_expr.span,
39-
E0067, "invalid left-hand side expression")
40-
.span_label(
41-
lhs_expr.span,
42-
"invalid expression for left-hand side")
43-
.emit();
44-
}
36+
self.check_lhs_assignable(lhs, "E0067", &op.span);
37+
4538
ty
4639
}
4740

‎src/librustc_typeck/expr_use_visitor.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -291,7 +291,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> {
291291
}
292292
}
293293

294-
hir::ExprKind::Assign(ref lhs, ref rhs) => {
294+
hir::ExprKind::Assign(ref lhs, ref rhs, _) => {
295295
self.mutate_expr(lhs);
296296
self.consume_expr(rhs);
297297
}

‎src/libsyntax/ast.rs

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

12301230
/// An assignment (`a = foo()`).
1231-
Assign(P<Expr>, P<Expr>),
1231+
/// The `Span` argument is the span of the `=` token.
1232+
Assign(P<Expr>, P<Expr>, Span),
12321233
/// An assignment with an operator.
12331234
///
12341235
/// E.g., `a += 1`.

‎src/libsyntax/mut_visit.rs

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

‎src/libsyntax/print/pprust.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -2077,7 +2077,7 @@ impl<'a> State<'a> {
20772077
self.print_expr_maybe_paren(expr, parser::PREC_POSTFIX);
20782078
self.s.word(".await");
20792079
}
2080-
ast::ExprKind::Assign(ref lhs, ref rhs) => {
2080+
ast::ExprKind::Assign(ref lhs, ref rhs, _) => {
20812081
let prec = AssocOp::Assign.precedence() as i8;
20822082
self.print_expr_maybe_paren(lhs, prec + 1);
20832083
self.s.space();

‎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 }

‎src/libsyntax/visit.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -743,9 +743,9 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
743743
visitor.visit_block(body);
744744
}
745745
ExprKind::Await(ref expr) => visitor.visit_expr(expr),
746-
ExprKind::Assign(ref left_hand_expression, ref right_hand_expression) => {
747-
visitor.visit_expr(left_hand_expression);
748-
visitor.visit_expr(right_hand_expression);
746+
ExprKind::Assign(ref lhs, ref rhs, _) => {
747+
visitor.visit_expr(lhs);
748+
visitor.visit_expr(rhs);
749749
}
750750
ExprKind::AssignOp(_, ref left_expression, ref right_expression) => {
751751
visitor.visit_expr(left_expression);

‎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"))));

‎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)