Restrict var decl target to name or tuple

analyzer/src/traversal/declarations.rs

@@ -14,7 +14,10 @@ pub fn var_decl(
     stmt: &Node<fe::FuncStmt>,
 ) -> Result<(), SemanticError> {
     if let fe::FuncStmt::VarDecl { target, typ, value } = &stmt.kind {
-        let name = expressions::expr_name_string(target)?;
+        let name = match &target.kind {
+            fe::VarDeclTarget::Name(name) => name,
+            fe::VarDeclTarget::Tuple(_) => todo!("tuple destructuring variable declaration"),
+        };
         let declared_type =
             types::type_desc_fixed_size(Scope::Block(Rc::clone(&scope)), Rc::clone(&context), typ)?;
         if let Some(value) = value {

compiler/src/lowering/mappers/expressions.rs

@@ -55,8 +55,6 @@ pub fn expr(context: &Context, exp: Node<fe::Expr>) -> Node<fe::Expr> {
         },
         fe::Expr::List { .. } => unimplemented!(),
         fe::Expr::ListComp { .. } => unimplemented!(),
-        // We only accept empty tuples for now. We may want to completely eliminate tuple
-        // expressions before the Yul codegen pass, tho.
         fe::Expr::Tuple { .. } => expr_tuple(context, exp),
         fe::Expr::Str(_) => exp.kind,
     };

compiler/src/lowering/mappers/functions.rs

@@ -49,11 +49,14 @@ fn func_stmt(context: &Context, stmt: Node<fe::FuncStmt>) -> Vec<Node<fe::FuncSt
         fe::FuncStmt::Return { value } => vec![fe::FuncStmt::Return {
             value: expressions::optional_expr(context, value),
         }],
-        fe::FuncStmt::VarDecl { target, typ, value } => vec![fe::FuncStmt::VarDecl {
-            target: expressions::expr(context, target),
-            typ: types::type_desc(context, typ),
-            value: expressions::optional_expr(context, value),
-        }],
+        fe::FuncStmt::VarDecl { target, typ, value } => match target.kind {
+            fe::VarDeclTarget::Name(_) => vec![fe::FuncStmt::VarDecl {
+                target,
+                typ: types::type_desc(context, typ),
+                value: expressions::optional_expr(context, value),
+            }],
+            fe::VarDeclTarget::Tuple(_) => todo!("tuple var decl lowering"),
+        },
         fe::FuncStmt::Assign { targets, value } => vec![fe::FuncStmt::Assign {
             targets: expressions::multiple_exprs(context, targets),
             value: expressions::expr(context, value),

compiler/src/yul/mappers/declarations.rs

@@ -11,7 +11,7 @@ pub fn var_decl(context: &Context, stmt: &Node<fe::FuncStmt>) -> yul::Statement
     let decl_type = context.get_declaration(stmt).expect("missing attributes");
 
     if let fe::FuncStmt::VarDecl { target, value, .. } = &stmt.kind {
-        let target = names::var_name(&expressions::expr_name_string(&target));
+        let target = names::var_name(var_decl_name(&target.kind));
 
         return if let Some(value) = value {
             let value = expressions::expr(context, &value);
@@ -30,6 +30,14 @@ pub fn var_decl(context: &Context, stmt: &Node<fe::FuncStmt>) -> yul::Statement
     unreachable!()
 }
 
+fn var_decl_name(target: &fe::VarDeclTarget) -> &str {
+    if let fe::VarDeclTarget::Name(name) = target {
+        name
+    } else {
+        panic!("complex VarDeclTargets should be lowered to VarDeclTarget::Name")
+    }
+}
+
 #[cfg(test)]
 #[cfg(feature = "fix-context-harness")]
 mod tests {

compiler/src/yul/mappers/expressions.rs

@@ -309,10 +309,10 @@ pub fn slice_index(context: &Context, slice: &Node<fe::Slice>) -> yul::Expressio
 
 fn expr_tuple(exp: &Node<fe::Expr>) -> yul::Expression {
     if let fe::Expr::Tuple { elts } = &exp.kind {
-        if !elts.is_empty() {
-            todo!("Non empty Tuples aren't yet supported")
-        } else {
+        if elts.is_empty() {
             return literal_expression! {0x0};
+        } else {
+            panic!("Non-empty Tuples should be lowered to structs")
         }
     }
 

parser/src/ast.rs

@@ -129,15 +129,13 @@ pub struct FuncDefArg {
     pub typ: Node<TypeDesc>,
 }
 
-// TODO: `Node`s are very large. VarDecl is 328 bytes
-#[allow(clippy::large_enum_variant)]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
 pub enum FuncStmt {
     Return {
         value: Option<Node<Expr>>,
     },
     VarDecl {
-        target: Node<Expr>,
+        target: Node<VarDeclTarget>,
         typ: Node<TypeDesc>,
         value: Option<Node<Expr>>,
     },
@@ -183,6 +181,12 @@ pub enum FuncStmt {
     Revert,
 }
 
+#[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
+pub enum VarDeclTarget {
+    Name(String),
+    Tuple(Vec<Node<VarDeclTarget>>),
+}
+
 #[derive(Serialize, Deserialize, Debug, PartialEq, Clone)]
 pub enum Expr {
     Ternary {

parser/src/grammar/functions.rs

@@ -1,7 +1,9 @@
 use super::expressions::{parse_call_args, parse_expr};
 use super::types::parse_type_desc;
 
-use crate::ast::{BinOperator, ContractStmt, FuncDefArg, FuncStmt, PubQualifier};
+use crate::ast::{
+    BinOperator, ContractStmt, Expr, FuncDefArg, FuncStmt, PubQualifier, VarDeclTarget,
+};
 use crate::lexer::TokenKind;
 use crate::node::Node;
 use crate::{Label, ParseFailed, ParseResult, Parser};
@@ -224,23 +226,17 @@ fn parse_expr_stmt(par: &mut Parser) -> ParseResult<Node<FuncStmt>> {
         }
         Some(Colon) => {
             par.next()?;
+
+            let target = expr_to_vardecl_target(par, expr)?;
             let typ = parse_type_desc(par)?;
             let value = if par.peek() == Some(Eq) {
                 par.next()?;
                 Some(parse_expr(par)?)
             } else {
                 None
             };
-            let span = expr.span + typ.span + value.as_ref();
-            // TODO: restrict VarDecl target type?
-            Node::new(
-                FuncStmt::VarDecl {
-                    target: expr,
-                    typ,
-                    value,
-                },
-                span,
-            )
+            let span = target.span + typ.span + value.as_ref();
+            Node::new(FuncStmt::VarDecl { target, typ, value }, span)
         }
         Some(Eq) => {
             par.next()?;
@@ -279,6 +275,34 @@ fn parse_expr_stmt(par: &mut Parser) -> ParseResult<Node<FuncStmt>> {
     Ok(node)
 }
 
+fn expr_to_vardecl_target(par: &mut Parser, expr: Node<Expr>) -> ParseResult<Node<VarDeclTarget>> {
+    match expr.kind {
+        Expr::Name(name) => Ok(Node::new(VarDeclTarget::Name(name), expr.span)),
+        Expr::Tuple { elts } if !elts.is_empty() => Ok(Node::new(
+            VarDeclTarget::Tuple(
+                elts.into_iter()
+                    .map(|elt| expr_to_vardecl_target(par, elt))
+                    .collect::<ParseResult<Vec<_>>>()?,
+            ),
+            expr.span,
+        )),
+        _ => {
+            par.fancy_error(
+                "failed to parse variable declaration",
+                vec![Label::primary(
+                    expr.span,
+                    "invalid variable declaration target".into(),
+                )],
+                vec![
+                    "The left side of a variable declaration can be either a name\nor a non-empty tuple."
+                        .into(),
+                ],
+            );
+            Err(ParseFailed)
+        }
+    }
+}
+
 /// Parse an `if` statement, or an `elif` block.
 ///
 /// # Panics

parser/tests/cases/errors.rs

@@ -25,8 +25,7 @@ macro_rules! test_parse_err {
     ($name:ident, $parse_fn:expr, $should_fail:expr, $src:expr) => {
         #[test]
         fn $name() {
-            let err = err_string(stringify!($name), $parse_fn, $should_fail, $src);
-            assert_snapshot!(err);
+            assert_snapshot!(err_string(stringify!($name), $parse_fn, $should_fail, $src));
         }
     };
 }
@@ -57,5 +56,20 @@ test_parse_err! { if_no_body, functions::parse_stmt, true, "if x:\nelse:\n x" }
 test_parse_err! { import_bad_name, module::parse_simple_import, true, "import x as 123" }
 test_parse_err! { module_bad_stmt, module::parse_module, true, "if x:\n y" }
 test_parse_err! { module_nonsense, module::parse_module, true, "))" }
-test_parse_err! { string_invalid_escape, expressions::parse_expr, false, r#""a string \c ""# }
 test_parse_err! { struct_bad_field_name, types::parse_struct_def, true, "struct f:\n pub def" }
+test_parse_err! { stmt_vardecl_attr, functions::parse_stmt, true, "f.s : u" }
+test_parse_err! { stmt_vardecl_tuple, functions::parse_stmt, true, "(a, x+1) : u256" }
+test_parse_err! { stmt_vardecl_tuple_empty, functions::parse_stmt, true, "(a, ()) : u256" }
+test_parse_err! { stmt_vardecl_subscript, functions::parse_stmt, true, "a[1] : u256" }
+
+// assert_snapshot! doesn't like the invalid escape code
+#[test]
+fn string_invalid_escape() {
+    let err = err_string(
+        "string_invalid_escape",
+        expressions::parse_expr,
+        false,
+        r#""a string \c ""#,
+    );
+    assert_snapshot!(err);
+}

parser/tests/cases/parse_ast.rs

@@ -102,6 +102,9 @@ test_parse! { stmt_if2, functions::parse_stmt, "if a:\n b \nelif c:\n d \nelif e
 test_parse! { stmt_while, functions::parse_stmt, "while a > 5:\n a -= 1" }
 test_parse! { stmt_for, functions::parse_stmt, "for a in b[0]:\n pass" }
 test_parse! { stmt_for_else, functions::parse_stmt, "for a in b:\n c\nelse:\n d" }
+test_parse! { stmt_var_decl_name, functions::parse_stmt, "foo: u256" }
+test_parse! { stmt_var_decl_tuple, functions::parse_stmt, "(foo, bar): (u256, u256) = (10, 10)" }
+test_parse! { stmt_var_decl_tuples, functions::parse_stmt, "(a, (b, (c, d))): x" }
 
 test_parse! { type_def, types::parse_type_def, "type X = map<address, u256>" }
 test_parse! { type_name, types::parse_type_desc, "MyType" }

parser/tests/cases/snapshots/cases__errors__contract_bad_name.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_bad_name), contracts::parse_contract_def, true,\n           \"contract 1X:\\n x: u8\")"
 
 ---
 error: failed to parse contract definition

parser/tests/cases/snapshots/cases__errors__contract_const_fn.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_const_fn), contracts::parse_contract_def,\n           false, \"contract C:\\n const def f():\\n  pass\")"
 
 ---
 error: `const` qualifier can't be used with function definitions

parser/tests/cases/snapshots/cases__errors__contract_const_pub.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_const_pub), contracts::parse_contract_def,\n           false, \"contract C:\\n const pub x: u8\")"
 
 ---
 error: `const pub` should be written `pub const`

parser/tests/cases/snapshots/cases__errors__contract_empty_body.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_empty_body), module::parse_module, true,\n           \"contract X:\\n \\n \\ncontract Y:\\n x: u8\")"
 
 ---
 error: failed to parse contract definition body

parser/tests/cases/snapshots/cases__errors__contract_field_after_def.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_field_after_def), module::parse_module, false,\n           r#\"\ncontract C:\n  def f():\n    pass\n  x: u8\n\"#)"
 
 ---
 error: contract field definitions must come before any function or event definitions

parser/tests/cases/snapshots/cases__errors__contract_pub_event.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(contract_pub_event), contracts::parse_contract_def,\n           false, \"contract C:\\n pub event E:\\n  x: u8\")"
 
 ---
 error: `pub` qualifier can't be used with event definitions

parser/tests/cases/snapshots/cases__errors__emit_bad_call.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(emit_bad_call), functions::parse_stmt, true,\n           \"emit MyEvent(1)()\")"
 
 ---
 error: unexpected token while parsing emit statement

parser/tests/cases/snapshots/cases__errors__emit_expr.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(emit_expr), functions::parse_stmt, true, \"emit x + 1\")"
 
 ---
 error: failed to parse event invocation parameter list

parser/tests/cases/snapshots/cases__errors__emit_no_args.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(emit_no_args), functions::parse_stmt, true, \"emit x\")"
 
 ---
 error: unexpected end of file

parser/tests/cases/snapshots/cases__errors__expr_bad_prefix.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(expr_bad_prefix), expressions::parse_expr, true,\n           \"*x + 1\")"
 
 ---
 error: Unexpected token while parsing expression: `*`

parser/tests/cases/snapshots/cases__errors__fn_no_args.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(fn_no_args), |par| functions::parse_fn_def(par, None),\n           false, \"def f:\\n  return 5\")"
 
 ---
 error: function definition requires a list of parameters

parser/tests/cases/snapshots/cases__errors__for_no_in.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(for_no_in), functions::parse_stmt, true,\n           \"for x:\\n pass\")"
 
 ---
 error: failed to parse `for` statement

parser/tests/cases/snapshots/cases__errors__if_no_body.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(if_no_body), functions::parse_stmt, true,\n           \"if x:\\nelse:\\n x\")"
 
 ---
 error: failed to parse `if` statement body

parser/tests/cases/snapshots/cases__errors__import_bad_name.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(import_bad_name), module::parse_simple_import, true,\n           \"import x as 123\")"
 
 ---
 error: failed to parse import statement

parser/tests/cases/snapshots/cases__errors__module_bad_stmt.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(module_bad_stmt), module::parse_module, true,\n           \"if x:\\n y\")"
 
 ---
 error: failed to parse module

parser/tests/cases/snapshots/cases__errors__module_nonsense.snap

@@ -1,6 +1,6 @@
 ---
 source: parser/tests/cases/errors.rs
-expression: err
+expression: "err_string(stringify!(module_nonsense), module::parse_module, true, \"))\")"
 
 ---
 error: Unmatched right parenthesis

parser/tests/cases/snapshots/cases__errors__stmt_vardecl_attr.snap

@@ -0,0 +1,15 @@
+---
+source: parser/tests/cases/errors.rs
+expression: "err_string(stringify!(stmt_vardecl_attr), functions::parse_stmt, true,\n           \"f.s : u\")"
+
+---
+error: failed to parse variable declaration
+  ┌─ stmt_vardecl_attr:1:1
+  │
+1 │ f.s : u
+  │ ^^^ invalid variable declaration target
+  │
+  = The left side of a variable declaration can be either a name
+    or a non-empty tuple.
+
+

parser/tests/cases/snapshots/cases__errors__stmt_vardecl_subscript.snap

@@ -0,0 +1,15 @@
+---
+source: parser/tests/cases/errors.rs
+expression: "err_string(stringify!(stmt_vardecl_subscript), functions::parse_stmt, true,\n           \"a[1] : u256\")"
+
+---
+error: failed to parse variable declaration
+  ┌─ stmt_vardecl_subscript:1:1
+  │
+1 │ a[1] : u256
+  │ ^^^^ invalid variable declaration target
+  │
+  = The left side of a variable declaration can be either a name
+    or a non-empty tuple.
+
+