Merge #118

118: Add AssignExpr API represention r=Niki4tap a=xFrednet

This PR adds the assign expression to the API. The implementation was a bit complicated design wise, I created rust-marker/design#35 to discuss other solutions and review this representation before v1.0.0.

Before reviewing this, I suggest checking this [Playground](https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021&gist=0da87ed83a1f80c61b026f54bf75d077) with *Show HIR* to see how rustc desugars some assign expressions. `#[clippy::dump]` and then *Tools > Clippy* can also always be helpful.

I also appreciate feedback on the design.

---

r? `@Niki4tap` Would you mind giving this a review? We can also wait a few days to see if we get any feedback on the design issue. On the other hand, we can always revert these changes :). Besides that, I'm trying to document all new nodes added to the API, we'll see how that goes ^^.

#52 

Co-authored-by: xFrednet <xFrednet@gmail.com>

marker_api/src/ast/expr.rs

@@ -49,6 +49,7 @@ pub enum ExprKind<'ast> {
     Ref(&'ast RefExpr<'ast>),
     BinaryOp(&'ast BinaryOpExpr<'ast>),
     QuestionMark(&'ast QuestionMarkExpr<'ast>),
+    Assign(&'ast AssignExpr<'ast>),
     As(&'ast AsExpr<'ast>),
     Path(&'ast PathExpr<'ast>),
     Call(&'ast CallExpr<'ast>),
@@ -79,6 +80,7 @@ pub enum ExprPrecedence {
     Lit = 0x1400_0000,
     Block = 0x1400_0001,
     Ctor = 0x1400_0002,
+    Assign = 0x1400_0003,
 
     Path = 0x1300_0000,
 
@@ -162,7 +164,7 @@ macro_rules! impl_expr_kind_fn {
         impl_expr_kind_fn!($method() -> $return_ty,
             IntLit, FloatLit, StrLit, CharLit, BoolLit,
             Block,
-            UnaryOp, Ref, BinaryOp, QuestionMark, As,
+            UnaryOp, Ref, BinaryOp, QuestionMark, As, Assign,
             Path, Index, Field,
             Call, Method,
             Array, Tuple, Ctor, Range,

marker_api/src/ast/expr/op_exprs.rs

@@ -1,4 +1,7 @@
-use crate::ast::ty::TyKind;
+use crate::{
+    ast::{pat::PatKind, ty::TyKind},
+    ffi::FfiOption,
+};
 
 use super::{CommonExprData, ExprKind, ExprPrecedence};
 
@@ -237,5 +240,65 @@ impl<'ast> AsExpr<'ast> {
     }
 }
 
-// FIXME: Add Assign expressions, these will require place expressions and a decision
-// if some cases should be represented as patterns or always as expressions.
+/// An expression assigning a value to an assignee expression.
+///
+/// ```
+///     let mut assignee = 20;
+///
+/// //  vvvvvvvv The assignee expression
+///     assignee = 10;
+/// //             ^^ The value expression
+///
+/// //  vvvvvvvvvvvvv A complex assignee expression
+///     [assignee, _] = [2, 3];
+/// //                ^ ^^^^^^ The value expression
+/// //                |
+/// //                No compound operator
+///
+/// //  vvvvvvvv The assignee expression
+///     assignee += 1;
+/// //           ^  ^ The value expression
+/// //           |
+/// //           Plus as a compound operator
+/// ```
+#[repr(C)]
+#[derive(Debug)]
+pub struct AssignExpr<'ast> {
+    data: CommonExprData<'ast>,
+    assignee: PatKind<'ast>,
+    value: ExprKind<'ast>,
+    op: FfiOption<BinaryOpKind>,
+}
+
+impl<'ast> AssignExpr<'ast> {
+    pub fn assignee(&self) -> PatKind<'ast> {
+        self.assignee
+    }
+
+    pub fn value(&self) -> ExprKind<'ast> {
+        self.value
+    }
+
+    pub fn op(&self) -> Option<BinaryOpKind> {
+        self.op.copy()
+    }
+}
+
+super::impl_expr_data!(AssignExpr<'ast>, Assign);
+
+#[cfg(feature = "driver-api")]
+impl<'ast> AssignExpr<'ast> {
+    pub fn new(
+        data: CommonExprData<'ast>,
+        assignee: PatKind<'ast>,
+        value: ExprKind<'ast>,
+        op: Option<BinaryOpKind>,
+    ) -> Self {
+        Self {
+            data,
+            assignee,
+            value,
+            op: op.into(),
+        }
+    }
+}

marker_api/src/ast/pat.rs

@@ -1,4 +1,4 @@
-use super::{Span, SpanId};
+use super::{expr::ExprKind, Span, SpanId};
 
 use std::{fmt::Debug, marker::PhantomData};
 
@@ -40,6 +40,34 @@ pub enum PatKind<'ast> {
     Tuple(&'ast TuplePat<'ast>),
     Slice(&'ast SlicePat<'ast>),
     Or(&'ast OrPat<'ast>),
+    /// Patterns are used as assignees in [`AssignExpr`](super::expr::AssignExpr)
+    /// nodes. Assign expressions can target place expressions like
+    /// variables, [`IndexExpr`](super::expr::IndexExpr)s and
+    /// [`FieldExpr`](super::expr::FieldExpr)s. These expressions would
+    /// be stored as this variant.
+    ///
+    /// ```
+    /// # fn some_fn() -> (i32, i32) { (4, 5) }
+    /// # let mut a = 1;
+    /// # let mut b = (2, 3);
+    /// # let mut c = [4, 5];
+    ///     a = 6;
+    /// //  ^ A path expression targeting the local variable `a`
+    ///
+    ///     b.1 = 7;
+    /// //  ^^^ A field expression accessing field 1 on the local variable `b`
+    ///
+    ///     c[0] = 8;
+    /// //  ^^^^ An index expression on local variable `c`
+    ///
+    ///     (a, b.0) = some_fn();
+    /// //  ^^^^^^^^ Place expressions nested in a tuple pattern
+    /// ```
+    ///
+    /// Place expressions can currently only occur from [`AssignExpr`](super::expr::AssignExpr)s.
+    /// Patterns from [`LetStmts`](super::stmt::LetStmt)s and arguments in
+    /// [`FnItem`](super::item::FnItem) will never contain place expressions.
+    Place(ExprKind<'ast>),
     Unstable(&'ast UnstablePat<'ast>),
 }
 
@@ -51,7 +79,7 @@ macro_rules! impl_pat_data_fn {
     ($method:ident () -> $return_ty:ty) => {
         impl_pat_data_fn!(
             $method() -> $return_ty,
-            Ident, Wildcard, Rest, Ref, Struct, Tuple, Slice, Or, Unstable
+            Ident, Wildcard, Rest, Ref, Struct, Tuple, Slice, Or, Place, Unstable
         );
     };
     ($method:ident () -> $return_ty:ty $(, $item:ident)+) => {
@@ -65,6 +93,16 @@ macro_rules! impl_pat_data_fn {
 
 use impl_pat_data_fn;
 
+impl<'ast> PatData<'ast> for ExprKind<'ast> {
+    fn span(&self) -> &Span<'ast> {
+        self.span()
+    }
+
+    fn as_pat(&'ast self) -> PatKind<'ast> {
+        PatKind::Place(*self)
+    }
+}
+
 #[repr(C)]
 #[derive(Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "driver-api", visibility::make(pub))]

marker_driver_rustc/src/conversion/marker/expr.rs

@@ -1,12 +1,14 @@
 use marker_api::ast::{
     expr::{
-        ArrayExpr, BinaryOpExpr, BinaryOpKind, BlockExpr, BoolLitExpr, CallExpr, CharLitExpr, CommonExprData, CtorExpr,
-        CtorField, ExprKind, ExprPrecedence, FieldExpr, FloatLitExpr, FloatSuffix, IfExpr, IndexExpr, IntLitExpr,
-        IntSuffix, LetExpr, MatchArm, MatchExpr, MethodExpr, PathExpr, RangeExpr, RefExpr, StrLitData, StrLitExpr,
-        TupleExpr, UnaryOpExpr, UnaryOpKind, UnstableExpr,
+        ArrayExpr, AssignExpr, BinaryOpExpr, BinaryOpKind, BlockExpr, BoolLitExpr, CallExpr, CharLitExpr,
+        CommonExprData, CtorExpr, CtorField, ExprKind, ExprPrecedence, FieldExpr, FloatLitExpr, FloatSuffix, IfExpr,
+        IndexExpr, IntLitExpr, IntSuffix, LetExpr, MatchArm, MatchExpr, MethodExpr, PathExpr, RangeExpr, RefExpr,
+        StrLitData, StrLitExpr, TupleExpr, UnaryOpExpr, UnaryOpKind, UnstableExpr,
     },
+    pat::PatKind,
     Ident,
 };
+use rustc_hash::FxHashMap;
 use rustc_hir as hir;
 use std::str::FromStr;
 
@@ -56,7 +58,16 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
                 self.to_expr(inner),
                 matches!(muta, hir::Mutability::Mut),
             ))),
-            hir::ExprKind::Block(block, None) => ExprKind::Block(self.alloc(self.to_block_expr(data, block))),
+            hir::ExprKind::Block(block, None) => {
+                if let [local, ..] = block.stmts
+                    && let hir::StmtKind::Local(local) = local.kind
+                    && let hir::LocalSource::AssignDesugar(_) = local.source
+                {
+                    ExprKind::Assign(self.alloc(self.to_assign_expr_from_desugar(block)))
+                } else {
+                    ExprKind::Block(self.alloc(self.to_block_expr(data, block)))
+                }
+            },
             hir::ExprKind::Call(operand, args) => match &operand.kind {
                 hir::ExprKind::Path(hir::QPath::LangItem(hir::LangItem::RangeInclusiveNew, _, _)) => {
                     ExprKind::Range(self.alloc({
@@ -178,6 +189,20 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
             hir::ExprKind::Match(scrutinee, arms, hir::MatchSource::Normal) => {
                 ExprKind::Match(self.alloc(MatchExpr::new(data, self.to_expr(scrutinee), self.to_match_arms(arms))))
             },
+            hir::ExprKind::Assign(assignee, value, _span) => {
+                ExprKind::Assign(self.alloc(AssignExpr::new(
+                    data,
+                    PatKind::Place(self.to_expr(assignee)),
+                    self.to_expr(value),
+                    None,
+                )))
+            },
+            hir::ExprKind::AssignOp(op, assignee, value) => ExprKind::Assign(self.alloc(AssignExpr::new(
+                data,
+                PatKind::Place(self.to_expr(assignee)),
+                self.to_expr(value),
+                Some(self.to_bin_op_kind(op)),
+            ))),
             // `DropTemps` is an rustc internal construct to tweak the drop
             // order during HIR lowering. Marker can for now ignore this and
             // convert the inner expression directly
@@ -314,4 +339,56 @@ impl<'ast, 'tcx> MarkerConverterInner<'ast, 'tcx> {
         let data = CommonExprData::new(self.to_expr_id(lets.hir_id), self.to_span_id(lets.span));
         ExprKind::Let(self.alloc(LetExpr::new(data, self.to_pat(lets.pat), self.to_expr(lets.init))))
     }
+
+    /// Rustc desugars assignments with tuples, arrays and structs in the
+    /// assignee as a block, which consists of a `let` statement, that assigns
+    /// the value expression to temporary variables called `lhs` which are
+    /// then assigned to the appropriate local variables
+    ///
+    /// The "Show HIR" option on the [Playground] is a great resource to
+    /// understand how this desugaring works. Here is a simple example to
+    /// illustrate the current desugar:
+    ///
+    /// ```
+    /// # let mut a = 0;
+    /// # let mut b = 0;
+    /// // This expression
+    /// [a, b] = [1, 2];
+    /// // Is desugared to:
+    /// { let [lhs, lhs] = [1, 2]; a = lhs; b = lhs; };
+    /// // Note that both `lhs` have different IDs
+    /// ```
+    ///
+    /// [Playground]: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2021
+    fn to_assign_expr_from_desugar(&self, block: &hir::Block<'tcx>) -> AssignExpr<'ast> {
+        let lhs_map: FxHashMap<_, _> = block
+            .stmts
+            .iter()
+            .skip(1)
+            .map(|stmt| {
+                if let hir::StmtKind::Expr(expr) = stmt.kind
+                    && let hir::ExprKind::Assign(assign_expr, value, _span) = expr.kind
+                    && let hir::ExprKind::Path(hir::QPath::Resolved(None, path)) = value.kind
+                    && let hir::def::Res::Local(local_id) = path.res
+                {
+                    (local_id, self.to_expr(assign_expr))
+                } else {
+                    unreachable!("unexpected statement while resugaring {stmt:?}")
+                }
+            })
+            .collect();
+        if let [local, ..] = block.stmts
+            && let hir::StmtKind::Local(local) = local.kind
+            && let hir::LocalSource::AssignDesugar(_) = local.source
+        {
+            AssignExpr::new(
+                CommonExprData::new(self.to_expr_id(local.hir_id), self.to_span_id(local.span)),
+                self.to_pat_with_hls(local.pat, &lhs_map),
+                self.to_expr(local.init.unwrap()),
+                None
+            )
+        } else {
+            unreachable!("assignment expr desugar always has a local as the first statement")
+        }
+    }
 }