Skip to content

Commit 4dde7b4

Browse files
nnethercotennethercote
committed
Tighten up assignment operator representations.
In the AST, currently we use `BinOpKind` within `ExprKind::AssignOp` and `AssocOp::AssignOp`, even though this allows some nonsensical combinations. E.g. there is no `&&=` operator. Likewise for HIR and THIR. This commit introduces `AssignOpKind` which only includes the ten assignable operators, and uses it in `ExprKind::AssignOp` and `AssocOp::AssignOp`. (And does similar things for `hir::ExprKind` and `thir::ExprKind`.) This avoids the possibility of nonsensical combinations, as seen by the removal of the `bug!` case in `lang_item_for_binop`. The commit is mostly plumbing, including: - Adds an `impl From<AssignOpKind> for BinOpKind` (AST) and `impl From<AssignOp> for BinOp` (MIR/THIR). - `BinOpCategory` can now be created from both `BinOpKind` and `AssignOpKind`. - Replaces the `IsAssign` type with `Op`, which has more information and a few methods. - `suggest_swapping_lhs_and_rhs`: moves the condition to the call site, it's easier that way. - `check_expr_inner`: had to factor out some code into a separate method. I'm on the fence about whether avoiding the nonsensical combinations is worth the extra code.
1 parent cf0d322 commit 4dde7b4

File tree

26 files changed

+394
-233
lines changed

26 files changed

+394
-233
lines changed

‎compiler/rustc_ast/src/ast.rs

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -978,6 +978,75 @@ impl BinOpKind {
978978

979979
pub type BinOp = Spanned<BinOpKind>;
980980

981+
// Sometimes `BinOpKind` and `AssignOpKind` need the same treatment. The
982+
// operations covered by `AssignOpKind` are a subset of those covered by
983+
// `BinOpKind`, so it makes sense to convert `AssignOpKind` to `BinOpKind`.
984+
impl From<AssignOpKind> for BinOpKind {
985+
fn from(op: AssignOpKind) -> BinOpKind {
986+
match op {
987+
AssignOpKind::AddAssign => BinOpKind::Add,
988+
AssignOpKind::SubAssign => BinOpKind::Sub,
989+
AssignOpKind::MulAssign => BinOpKind::Mul,
990+
AssignOpKind::DivAssign => BinOpKind::Div,
991+
AssignOpKind::RemAssign => BinOpKind::Rem,
992+
AssignOpKind::BitXorAssign => BinOpKind::BitXor,
993+
AssignOpKind::BitAndAssign => BinOpKind::BitAnd,
994+
AssignOpKind::BitOrAssign => BinOpKind::BitOr,
995+
AssignOpKind::ShlAssign => BinOpKind::Shl,
996+
AssignOpKind::ShrAssign => BinOpKind::Shr,
997+
}
998+
}
999+
}
1000+
1001+
#[derive(Clone, Copy, Debug, PartialEq, Encodable, Decodable, HashStable_Generic)]
1002+
pub enum AssignOpKind {
1003+
/// The `+=` operator (addition)
1004+
AddAssign,
1005+
/// The `-=` operator (subtraction)
1006+
SubAssign,
1007+
/// The `*=` operator (multiplication)
1008+
MulAssign,
1009+
/// The `/=` operator (division)
1010+
DivAssign,
1011+
/// The `%=` operator (modulus)
1012+
RemAssign,
1013+
/// The `^=` operator (bitwise xor)
1014+
BitXorAssign,
1015+
/// The `&=` operator (bitwise and)
1016+
BitAndAssign,
1017+
/// The `|=` operator (bitwise or)
1018+
BitOrAssign,
1019+
/// The `<<=` operator (shift left)
1020+
ShlAssign,
1021+
/// The `>>=` operator (shift right)
1022+
ShrAssign,
1023+
}
1024+
1025+
impl AssignOpKind {
1026+
pub fn as_str(&self) -> &'static str {
1027+
use AssignOpKind::*;
1028+
match self {
1029+
AddAssign => "+=",
1030+
SubAssign => "-=",
1031+
MulAssign => "*=",
1032+
DivAssign => "/=",
1033+
RemAssign => "%=",
1034+
BitXorAssign => "^=",
1035+
BitAndAssign => "&=",
1036+
BitOrAssign => "|=",
1037+
ShlAssign => "<<=",
1038+
ShrAssign => ">>=",
1039+
}
1040+
}
1041+
1042+
/// AssignOps are always by value.
1043+
pub fn is_by_value(self) -> bool {
1044+
true
1045+
}
1046+
}
1047+
1048+
pub type AssignOp = Spanned<AssignOpKind>;
1049+
9811050
/// Unary operator.
9821051
///
9831052
/// Note that `&data` is not an operator, it's an `AddrOf` expression.
@@ -1578,7 +1647,7 @@ pub enum ExprKind {
15781647
/// An assignment with an operator.
15791648
///
15801649
/// E.g., `a += 1`.
1581-
AssignOp(BinOp, P<Expr>, P<Expr>),
1650+
AssignOp(AssignOp, P<Expr>, P<Expr>),
15821651
/// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field.
15831652
Field(P<Expr>, Ident),
15841653
/// An indexing operation (e.g., `foo[2]`).

‎compiler/rustc_ast/src/util/parser.rs

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use rustc_span::kw;
22

3-
use crate::ast::{self, BinOpKind, RangeLimits};
3+
use crate::ast::{self, AssignOpKind, BinOpKind, RangeLimits};
44
use crate::token::{self, Token};
55

66
/// Associative operator.
@@ -9,7 +9,7 @@ pub enum AssocOp {
99
/// A binary op.
1010
Binary(BinOpKind),
1111
/// `?=` where ? is one of the assignable BinOps
12-
AssignOp(BinOpKind),
12+
AssignOp(AssignOpKind),
1313
/// `=`
1414
Assign,
1515
/// `as`
@@ -44,16 +44,16 @@ impl AssocOp {
4444
token::Or => Some(Binary(BinOpKind::BitOr)),
4545
token::Shl => Some(Binary(BinOpKind::Shl)),
4646
token::Shr => Some(Binary(BinOpKind::Shr)),
47-
token::PlusEq => Some(AssignOp(BinOpKind::Add)),
48-
token::MinusEq => Some(AssignOp(BinOpKind::Sub)),
49-
token::StarEq => Some(AssignOp(BinOpKind::Mul)),
50-
token::SlashEq => Some(AssignOp(BinOpKind::Div)),
51-
token::PercentEq => Some(AssignOp(BinOpKind::Rem)),
52-
token::CaretEq => Some(AssignOp(BinOpKind::BitXor)),
53-
token::AndEq => Some(AssignOp(BinOpKind::BitAnd)),
54-
token::OrEq => Some(AssignOp(BinOpKind::BitOr)),
55-
token::ShlEq => Some(AssignOp(BinOpKind::Shl)),
56-
token::ShrEq => Some(AssignOp(BinOpKind::Shr)),
47+
token::PlusEq => Some(AssignOp(AssignOpKind::AddAssign)),
48+
token::MinusEq => Some(AssignOp(AssignOpKind::SubAssign)),
49+
token::StarEq => Some(AssignOp(AssignOpKind::MulAssign)),
50+
token::SlashEq => Some(AssignOp(AssignOpKind::DivAssign)),
51+
token::PercentEq => Some(AssignOp(AssignOpKind::RemAssign)),
52+
token::CaretEq => Some(AssignOp(AssignOpKind::BitXorAssign)),
53+
token::AndEq => Some(AssignOp(AssignOpKind::BitAndAssign)),
54+
token::OrEq => Some(AssignOp(AssignOpKind::BitOrAssign)),
55+
token::ShlEq => Some(AssignOp(AssignOpKind::ShlAssign)),
56+
token::ShrEq => Some(AssignOp(AssignOpKind::ShrAssign)),
5757
token::Lt => Some(Binary(BinOpKind::Lt)),
5858
token::Le => Some(Binary(BinOpKind::Le)),
5959
token::Ge => Some(Binary(BinOpKind::Ge)),

‎compiler/rustc_ast_lowering/src/expr.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
274274
}
275275
ExprKind::Assign(el, er, span) => self.lower_expr_assign(el, er, *span, e.span),
276276
ExprKind::AssignOp(op, el, er) => hir::ExprKind::AssignOp(
277-
self.lower_binop(*op),
277+
self.lower_assign_op(*op),
278278
self.lower_expr(el),
279279
self.lower_expr(er),
280280
),
@@ -420,6 +420,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
420420
Spanned { node: b.node, span: self.lower_span(b.span) }
421421
}
422422

423+
fn lower_assign_op(&mut self, a: AssignOp) -> AssignOp {
424+
Spanned { node: a.node, span: self.lower_span(a.span) }
425+
}
426+
423427
fn lower_legacy_const_generics(
424428
&mut self,
425429
mut f: Expr,

‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -597,8 +597,7 @@ impl<'a> State<'a> {
597597
fixup.leftmost_subexpression(),
598598
);
599599
self.space();
600-
self.word(op.node.as_str());
601-
self.word_space("=");
600+
self.word_space(op.node.as_str());
602601
self.print_expr_cond_paren(
603602
rhs,
604603
rhs.precedence() < ExprPrecedence::Assign,

‎compiler/rustc_hir/src/hir.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ use rustc_ast::{
1010
IntTy, Label, LitIntType, LitKind, MetaItemInner, MetaItemLit, TraitObjectSyntax, UintTy,
1111
};
1212
pub use rustc_ast::{
13-
BinOp, BinOpKind, BindingMode, BorrowKind, BoundConstness, BoundPolarity, ByRef, CaptureBy,
14-
ImplPolarity, IsAuto, Movability, Mutability, UnOp, UnsafeBinderCastKind,
13+
AssignOp, AssignOpKind, BinOp, BinOpKind, BindingMode, BorrowKind, BoundConstness,
14+
BoundPolarity, ByRef, CaptureBy, ImplPolarity, IsAuto, Movability, Mutability, UnOp,
15+
UnsafeBinderCastKind,
1516
};
1617
use rustc_data_structures::fingerprint::Fingerprint;
1718
use rustc_data_structures::sorted_map::SortedMap;
@@ -2468,7 +2469,7 @@ pub enum ExprKind<'hir> {
24682469
/// An assignment with an operator.
24692470
///
24702471
/// E.g., `a += 1`.
2471-
AssignOp(BinOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
2472+
AssignOp(AssignOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
24722473
/// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct or tuple field.
24732474
Field(&'hir Expr<'hir>, Ident),
24742475
/// An indexing operation (`foo[2]`).

‎compiler/rustc_hir_pretty/src/lib.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1586,8 +1586,7 @@ impl<'a> State<'a> {
15861586
hir::ExprKind::AssignOp(op, lhs, rhs) => {
15871587
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
15881588
self.space();
1589-
self.word(op.node.as_str());
1590-
self.word_space("=");
1589+
self.word_space(op.node.as_str());
15911590
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
15921591
}
15931592
hir::ExprKind::Field(expr, ident) => {

‎compiler/rustc_hir_typeck/src/expr.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
508508
self.check_expr_assign(expr, expected, lhs, rhs, span)
509509
}
510510
ExprKind::AssignOp(op, lhs, rhs) => {
511-
self.check_expr_binop_assign(expr, op, lhs, rhs, expected)
511+
self.check_expr_assign_op(expr, op, lhs, rhs, expected)
512512
}
513513
ExprKind::Unary(unop, oprnd) => self.check_expr_unop(unop, oprnd, expected, expr),
514514
ExprKind::AddrOf(kind, mutbl, oprnd) => {

‎compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3483,30 +3483,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
34833483
lhs_ty: Ty<'tcx>,
34843484
rhs_expr: &'tcx hir::Expr<'tcx>,
34853485
lhs_expr: &'tcx hir::Expr<'tcx>,
3486-
op: hir::BinOpKind,
34873486
) {
3488-
match op {
3489-
hir::BinOpKind::Eq => {
3490-
if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
3491-
&& self
3492-
.infcx
3493-
.type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
3494-
.must_apply_modulo_regions()
3495-
{
3496-
let sm = self.tcx.sess.source_map();
3497-
if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
3498-
&& let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
3499-
{
3500-
err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
3501-
err.multipart_suggestion(
3502-
"consider swapping the equality",
3503-
vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
3504-
Applicability::MaybeIncorrect,
3505-
);
3506-
}
3507-
}
3487+
if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
3488+
&& self
3489+
.infcx
3490+
.type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
3491+
.must_apply_modulo_regions()
3492+
{
3493+
let sm = self.tcx.sess.source_map();
3494+
if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
3495+
&& let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
3496+
{
3497+
err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
3498+
err.multipart_suggestion(
3499+
"consider swapping the equality",
3500+
vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
3501+
Applicability::MaybeIncorrect,
3502+
);
35083503
}
3509-
_ => {}
35103504
}
35113505
}
35123506
}

0 commit comments

Comments
 (0)