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.

Author: nnethercote

compiler/rustc_ast/src/ast.rs

@@ -989,6 +989,75 @@ impl BinOpKind {
 
 pub type BinOp = Spanned<BinOpKind>;
 
+// Sometimes `BinOpKind` and `AssignOpKind` need the same treatment. The
+// operations covered by `AssignOpKind` are a subset of those covered by
+// `BinOpKind`, so it makes sense to convert `AssignOpKind` to `BinOpKind`.
+impl From<AssignOpKind> for BinOpKind {
+    fn from(op: AssignOpKind) -> BinOpKind {
+        match op {
+            AssignOpKind::AddAssign => BinOpKind::Add,
+            AssignOpKind::SubAssign => BinOpKind::Sub,
+            AssignOpKind::MulAssign => BinOpKind::Mul,
+            AssignOpKind::DivAssign => BinOpKind::Div,
+            AssignOpKind::RemAssign => BinOpKind::Rem,
+            AssignOpKind::BitXorAssign => BinOpKind::BitXor,
+            AssignOpKind::BitAndAssign => BinOpKind::BitAnd,
+            AssignOpKind::BitOrAssign => BinOpKind::BitOr,
+            AssignOpKind::ShlAssign => BinOpKind::Shl,
+            AssignOpKind::ShrAssign => BinOpKind::Shr,
+        }
+    }
+}
+
+#[derive(Clone, Copy, Debug, PartialEq, Encodable, Decodable, HashStable_Generic)]
+pub enum AssignOpKind {
+    /// The `+=` operator (addition)
+    AddAssign,
+    /// The `-=` operator (subtraction)
+    SubAssign,
+    /// The `*=` operator (multiplication)
+    MulAssign,
+    /// The `/=` operator (division)
+    DivAssign,
+    /// The `%=` operator (modulus)
+    RemAssign,
+    /// The `^=` operator (bitwise xor)
+    BitXorAssign,
+    /// The `&=` operator (bitwise and)
+    BitAndAssign,
+    /// The `|=` operator (bitwise or)
+    BitOrAssign,
+    /// The `<<=` operator (shift left)
+    ShlAssign,
+    /// The `>>=` operator (shift right)
+    ShrAssign,
+}
+
+impl AssignOpKind {
+    pub fn as_str(&self) -> &'static str {
+        use AssignOpKind::*;
+        match self {
+            AddAssign => "+=",
+            SubAssign => "-=",
+            MulAssign => "*=",
+            DivAssign => "/=",
+            RemAssign => "%=",
+            BitXorAssign => "^=",
+            BitAndAssign => "&=",
+            BitOrAssign => "|=",
+            ShlAssign => "<<=",
+            ShrAssign => ">>=",
+        }
+    }
+
+    /// AssignOps are always by value.
+    pub fn is_by_value(self) -> bool {
+        true
+    }
+}
+
+pub type AssignOp = Spanned<AssignOpKind>;
+
 /// Unary operator.
 ///
 /// Note that `&data` is not an operator, it's an `AddrOf` expression.
@@ -1601,7 +1670,7 @@ pub enum ExprKind {
     /// An assignment with an operator.
     ///
     /// E.g., `a += 1`.
-    AssignOp(BinOp, P<Expr>, P<Expr>),
+    AssignOp(AssignOp, P<Expr>, P<Expr>),
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct field.
     Field(P<Expr>, Ident),
     /// An indexing operation (e.g., `foo[2]`).

compiler/rustc_ast/src/util/parser.rs

@@ -1,6 +1,6 @@
 use rustc_span::kw;
 
-use crate::ast::{self, BinOpKind, RangeLimits};
+use crate::ast::{self, AssignOpKind, BinOpKind, RangeLimits};
 use crate::token::{self, Token};
 
 /// Associative operator.
@@ -9,7 +9,7 @@ pub enum AssocOp {
     /// A binary op.
     Binary(BinOpKind),
     /// `?=` where ? is one of the assignable BinOps
-    AssignOp(BinOpKind),
+    AssignOp(AssignOpKind),
     /// `=`
     Assign,
     /// `as`
@@ -44,16 +44,16 @@ impl AssocOp {
             token::Or => Some(Binary(BinOpKind::BitOr)),
             token::Shl => Some(Binary(BinOpKind::Shl)),
             token::Shr => Some(Binary(BinOpKind::Shr)),
-            token::PlusEq => Some(AssignOp(BinOpKind::Add)),
-            token::MinusEq => Some(AssignOp(BinOpKind::Sub)),
-            token::StarEq => Some(AssignOp(BinOpKind::Mul)),
-            token::SlashEq => Some(AssignOp(BinOpKind::Div)),
-            token::PercentEq => Some(AssignOp(BinOpKind::Rem)),
-            token::CaretEq => Some(AssignOp(BinOpKind::BitXor)),
-            token::AndEq => Some(AssignOp(BinOpKind::BitAnd)),
-            token::OrEq => Some(AssignOp(BinOpKind::BitOr)),
-            token::ShlEq => Some(AssignOp(BinOpKind::Shl)),
-            token::ShrEq => Some(AssignOp(BinOpKind::Shr)),
+            token::PlusEq => Some(AssignOp(AssignOpKind::AddAssign)),
+            token::MinusEq => Some(AssignOp(AssignOpKind::SubAssign)),
+            token::StarEq => Some(AssignOp(AssignOpKind::MulAssign)),
+            token::SlashEq => Some(AssignOp(AssignOpKind::DivAssign)),
+            token::PercentEq => Some(AssignOp(AssignOpKind::RemAssign)),
+            token::CaretEq => Some(AssignOp(AssignOpKind::BitXorAssign)),
+            token::AndEq => Some(AssignOp(AssignOpKind::BitAndAssign)),
+            token::OrEq => Some(AssignOp(AssignOpKind::BitOrAssign)),
+            token::ShlEq => Some(AssignOp(AssignOpKind::ShlAssign)),
+            token::ShrEq => Some(AssignOp(AssignOpKind::ShrAssign)),
             token::Lt => Some(Binary(BinOpKind::Lt)),
             token::Le => Some(Binary(BinOpKind::Le)),
             token::Ge => Some(Binary(BinOpKind::Ge)),

compiler/rustc_ast_lowering/src/expr.rs

@@ -274,7 +274,7 @@ impl<'hir> LoweringContext<'_, 'hir> {
                 }
                 ExprKind::Assign(el, er, span) => self.lower_expr_assign(el, er, *span, e.span),
                 ExprKind::AssignOp(op, el, er) => hir::ExprKind::AssignOp(
-                    self.lower_binop(*op),
+                    self.lower_assign_op(*op),
                     self.lower_expr(el),
                     self.lower_expr(er),
                 ),
@@ -443,6 +443,10 @@ impl<'hir> LoweringContext<'_, 'hir> {
         Spanned { node: b.node, span: self.lower_span(b.span) }
     }
 
+    fn lower_assign_op(&mut self, a: AssignOp) -> AssignOp {
+        Spanned { node: a.node, span: self.lower_span(a.span) }
+    }
+
     fn lower_legacy_const_generics(
         &mut self,
         mut f: Expr,

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

@@ -605,8 +605,7 @@ impl<'a> State<'a> {
                     fixup.leftmost_subexpression(),
                 );
                 self.space();
-                self.word(op.node.as_str());
-                self.word_space("=");
+                self.word_space(op.node.as_str());
                 self.print_expr_cond_paren(
                     rhs,
                     rhs.precedence() < ExprPrecedence::Assign,

compiler/rustc_hir/src/hir.rs

@@ -10,9 +10,9 @@ use rustc_ast::{
     LitKind, TraitObjectSyntax, UintTy, UnsafeBinderCastKind,
 };
 pub use rustc_ast::{
-    AttrId, AttrStyle, BinOp, BinOpKind, BindingMode, BorrowKind, BoundConstness, BoundPolarity,
-    ByRef, CaptureBy, DelimArgs, ImplPolarity, IsAuto, MetaItemInner, MetaItemLit, Movability,
-    Mutability, UnOp,
+    AssignOp, AssignOpKind, AttrId, AttrStyle, BinOp, BinOpKind, BindingMode, BorrowKind,
+    BoundConstness, BoundPolarity, ByRef, CaptureBy, DelimArgs, ImplPolarity, IsAuto,
+    MetaItemInner, MetaItemLit, Movability, Mutability, UnOp,
 };
 use rustc_attr_data_structures::AttributeKind;
 use rustc_data_structures::fingerprint::Fingerprint;
@@ -2609,7 +2609,7 @@ pub enum ExprKind<'hir> {
     /// An assignment with an operator.
     ///
     /// E.g., `a += 1`.
-    AssignOp(BinOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
+    AssignOp(AssignOp, &'hir Expr<'hir>, &'hir Expr<'hir>),
     /// Access of a named (e.g., `obj.foo`) or unnamed (e.g., `obj.0`) struct or tuple field.
     Field(&'hir Expr<'hir>, Ident),
     /// An indexing operation (`foo[2]`).

compiler/rustc_hir_pretty/src/lib.rs

@@ -1584,8 +1584,7 @@ impl<'a> State<'a> {
             hir::ExprKind::AssignOp(op, lhs, rhs) => {
                 self.print_expr_cond_paren(lhs, lhs.precedence() <= ExprPrecedence::Assign);
                 self.space();
-                self.word(op.node.as_str());
-                self.word_space("=");
+                self.word_space(op.node.as_str());
                 self.print_expr_cond_paren(rhs, rhs.precedence() < ExprPrecedence::Assign);
             }
             hir::ExprKind::Field(expr, ident) => {

compiler/rustc_hir_typeck/src/expr.rs

@@ -512,7 +512,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
                 self.check_expr_assign(expr, expected, lhs, rhs, span)
             }
             ExprKind::AssignOp(op, lhs, rhs) => {
-                self.check_expr_binop_assign(expr, op, lhs, rhs, expected)
+                self.check_expr_assign_op(expr, op, lhs, rhs, expected)
             }
             ExprKind::Unary(unop, oprnd) => self.check_expr_unop(unop, oprnd, expected, expr),
             ExprKind::AddrOf(kind, mutbl, oprnd) => {

compiler/rustc_hir_typeck/src/fn_ctxt/suggestions.rs

@@ -3478,30 +3478,24 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
         lhs_ty: Ty<'tcx>,
         rhs_expr: &'tcx hir::Expr<'tcx>,
         lhs_expr: &'tcx hir::Expr<'tcx>,
-        op: hir::BinOpKind,
     ) {
-        match op {
-            hir::BinOpKind::Eq => {
-                if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
-                    && self
-                        .infcx
-                        .type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
-                        .must_apply_modulo_regions()
-                {
-                    let sm = self.tcx.sess.source_map();
-                    if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
-                        && let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
-                    {
-                        err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
-                        err.multipart_suggestion(
-                            "consider swapping the equality",
-                            vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
-                            Applicability::MaybeIncorrect,
-                        );
-                    }
-                }
+        if let Some(partial_eq_def_id) = self.infcx.tcx.lang_items().eq_trait()
+            && self
+                .infcx
+                .type_implements_trait(partial_eq_def_id, [rhs_ty, lhs_ty], self.param_env)
+                .must_apply_modulo_regions()
+        {
+            let sm = self.tcx.sess.source_map();
+            if let Ok(rhs_snippet) = sm.span_to_snippet(rhs_expr.span)
+                && let Ok(lhs_snippet) = sm.span_to_snippet(lhs_expr.span)
+            {
+                err.note(format!("`{rhs_ty}` implements `PartialEq<{lhs_ty}>`"));
+                err.multipart_suggestion(
+                    "consider swapping the equality",
+                    vec![(lhs_expr.span, rhs_snippet), (rhs_expr.span, lhs_snippet)],
+                    Applicability::MaybeIncorrect,
+                );
             }
-            _ => {}
         }
     }
 }