Skip to content

Commit d92946a

Browse files
nnethercotennethercote
committed
Introduce ast::AssignOpKind.
Currently we use `BinOpKind` within `ExprKind::AssignOp` and `AssocOp::AssignOp`, even though there are some nonsensical combinations. E.g. there is no `&&=` operator. This commit introduces `AssignOpKind` which only includes the 10 assignable operators, and uses it in `ExprKind::AssignOp` and `AssocOp::AssignOp`. 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` - `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 d0fe5b5 commit d92946a

File tree

24 files changed

+364
-228
lines changed

24 files changed

+364
-228
lines changed

‎compiler/rustc_ast/src/ast.rs

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

985985
pub type BinOp = Spanned<BinOpKind>;
986986

987+
// Sometimes `BinOpKind` and `AssignOpKind` need the same treatment. The
988+
// operations covered by `AssignOpKind` are a subset of those covered by
989+
// `BitOpKind`, so it makes sense to convert `AssignOpKind` to `BinOpKind`.
990+
impl From<AssignOpKind> for BinOpKind {
991+
fn from(op: AssignOpKind) -> BinOpKind {
992+
match op {
993+
AssignOpKind::AddAssign => BinOpKind::Add,
994+
AssignOpKind::SubAssign => BinOpKind::Sub,
995+
AssignOpKind::MulAssign => BinOpKind::Mul,
996+
AssignOpKind::DivAssign => BinOpKind::Div,
997+
AssignOpKind::RemAssign => BinOpKind::Rem,
998+
AssignOpKind::BitXorAssign => BinOpKind::BitXor,
999+
AssignOpKind::BitAndAssign => BinOpKind::BitAnd,
1000+
AssignOpKind::BitOrAssign => BinOpKind::BitOr,
1001+
AssignOpKind::ShlAssign => BinOpKind::Shl,
1002+
AssignOpKind::ShrAssign => BinOpKind::Shr,
1003+
}
1004+
}
1005+
}
1006+
1007+
#[derive(Clone, Copy, Debug, PartialEq, Encodable, Decodable, HashStable_Generic)]
1008+
pub enum AssignOpKind {
1009+
/// The `+=` operator (addition)
1010+
AddAssign,
1011+
/// The `-=` operator (subtraction)
1012+
SubAssign,
1013+
/// The `*=` operator (multiplication)
1014+
MulAssign,
1015+
/// The `/=` operator (division)
1016+
DivAssign,
1017+
/// The `%=` operator (modulus)
1018+
RemAssign,
1019+
/// The `^=` operator (bitwise xor)
1020+
BitXorAssign,
1021+
/// The `&=` operator (bitwise and)
1022+
BitAndAssign,
1023+
/// The `|=` operator (bitwise or)
1024+
BitOrAssign,
1025+
/// The `<<=` operator (shift left)
1026+
ShlAssign,
1027+
/// The `>>=` operator (shift right)
1028+
ShrAssign,
1029+
}
1030+
1031+
impl AssignOpKind {
1032+
pub fn as_str(&self) -> &'static str {
1033+
use AssignOpKind::*;
1034+
match self {
1035+
AddAssign => "+=",
1036+
SubAssign => "-=",
1037+
MulAssign => "*=",
1038+
DivAssign => "/=",
1039+
RemAssign => "%=",
1040+
BitXorAssign => "^=",
1041+
BitAndAssign => "&=",
1042+
BitOrAssign => "|=",
1043+
ShlAssign => "<<=",
1044+
ShrAssign => ">>=",
1045+
}
1046+
}
1047+
1048+
/// AssignOps are always by value.
1049+
pub fn is_by_value(self) -> bool {
1050+
true
1051+
}
1052+
}
1053+
1054+
pub type AssignOp = Spanned<AssignOpKind>;
1055+
9871056
/// Unary operator.
9881057
///
9891058
/// Note that `&data` is not an operator, it's an `AddrOf` expression.
@@ -1579,7 +1648,7 @@ pub enum ExprKind {
15791648
/// An assignment with an operator.
15801649
///
15811650
/// E.g., `a += 1`.
1582-
AssignOp(BinOp, P<Expr>, P<Expr>),
1651+
AssignOp(AssignOp, P<Expr>, P<Expr>),
15831652
/// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field.
15841653
Field(P<Expr>, Ident),
15851654
/// 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
@@ -295,7 +295,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
295295
}
296296
ExprKind::Assign(el, er, span) => self.lower_expr_assign(el, er, *span, e.span),
297297
ExprKind::AssignOp(op, el, er) => hir::ExprKind::AssignOp(
298-
self.lower_binop(*op),
298+
self.lower_assign_op(*op),
299299
self.lower_expr(el),
300300
self.lower_expr(er),
301301
),
@@ -415,6 +415,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
415415
Spanned { node: b.node, span: self.lower_span(b.span) }
416416
}
417417

418+
fn lower_assign_op(&mut self, a: AssignOp) -> AssignOp {
419+
Spanned { node: a.node, span: self.lower_span(a.span) }
420+
}
421+
418422
fn lower_legacy_const_generics(
419423
&mut self,
420424
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
@@ -593,8 +593,7 @@ impl<'a> State<'a> {
593593
fixup.leftmost_subexpression(),
594594
);
595595
self.space();
596-
self.word(op.node.as_str());
597-
self.word_space("=");
596+
self.word_space(op.node.as_str());
598597
self.print_expr_cond_paren(
599598
rhs,
600599
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, 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;
@@ -2369,7 +2370,7 @@ pub enum ExprKind<'hir> {
23692370
/// An assignment with an operator.
23702371
///
23712372
/// E.g., `a += 1`.
2372-
AssignOp(BinOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
2373+
AssignOp(AssignOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
23732374
/// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct or tuple field.
23742375
Field(&'hir Expr<'hir>, Ident),
23752376
/// 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
@@ -1582,8 +1582,7 @@ impl<'a> State<'a> {
15821582
hir::ExprKind::AssignOp(op, lhs, rhs) => {
15831583
self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
15841584
self.space();
1585-
self.word(op.node.as_str());
1586-
self.word_space("=");
1585+
self.word_space(op.node.as_str());
15871586
self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
15881587
}
15891588
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
@@ -505,7 +505,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
505505
self.check_expr_assign(expr, expected, lhs, rhs, span)
506506
}
507507
ExprKind::AssignOp(op, lhs, rhs) => {
508-
self.check_expr_binop_assign(expr, op, lhs, rhs, expected)
508+
self.check_expr_assign_op(expr, op, lhs, rhs, expected)
509509
}
510510
ExprKind::Unary(unop, oprnd) => self.check_expr_unop(unop, oprnd, expected, expr),
511511
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
@@ -3407,30 +3407,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
34073407
lhs_ty: Ty<'tcx>,
34083408
rhs_expr: &'tcx hir::Expr<'tcx>,
34093409
lhs_expr: &'tcx hir::Expr<'tcx>,
3410-
op: hir::BinOpKind,
34113410
) {
3412-
match op {
3413-
hir::BinOpKind::Eq => {
3414-
if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
3415-
&& self
3416-
.infcx
3417-
.type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
3418-
.must_apply_modulo_regions()
3419-
{
3420-
let sm = self.tcx.sess.source_map();
3421-
if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
3422-
&& let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
3423-
{
3424-
err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
3425-
err.multipart_suggestion(
3426-
"consider swapping the equality",
3427-
vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
3428-
Applicability::MaybeIncorrect,
3429-
);
3430-
}
3431-
}
3411+
if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
3412+
&& self
3413+
.infcx
3414+
.type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
3415+
.must_apply_modulo_regions()
3416+
{
3417+
let sm = self.tcx.sess.source_map();
3418+
if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
3419+
&& let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
3420+
{
3421+
err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
3422+
err.multipart_suggestion(
3423+
"consider swapping the equality",
3424+
vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
3425+
Applicability::MaybeIncorrect,
3426+
);
34323427
}
3433-
_ => {}
34343428
}
34353429
}
34363430
}

0 commit comments

Comments
 (0)