From b4544b83bd4ec2b5317ddcedc9ca74ac94a18cc1 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 9 Oct 2023 15:45:55 -0500 Subject: [PATCH 1/2] Parse parenthesized lvalues --- .../noirc_frontend/src/hir/type_check/expr.rs | 2 +- .../noirc_frontend/src/hir/type_check/stmt.rs | 7 +++-- compiler/noirc_frontend/src/parser/parser.rs | 29 +++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 28264970620..a2205bcbbce 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -409,7 +409,7 @@ impl<'interner> TypeChecker<'interner> { }); let lhs_type = self.check_expression(&index_expr.collection); - match lhs_type { + match lhs_type.follow_bindings() { // XXX: We can check the array bounds here also, but it may be better to constant fold first // and have ConstId instead of ExprId for constants Type::Array(_, base_type) => *base_type, diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index 6993476e249..a3cb49c6d2e 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -227,16 +227,16 @@ impl<'interner> TypeChecker<'interner> { }, ); - let (result, array) = self.check_lvalue(*array, assign_span); + let (array_type, array) = self.check_lvalue(*array, assign_span); let array = Box::new(array); - let typ = match result { + let typ = match array_type.follow_bindings() { Type::Array(_, elem_type) => *elem_type, Type::Error => Type::Error, other => { // TODO: Need a better span here self.errors.push(TypeCheckError::TypeMismatch { - expected_typ: "an array".to_string(), + expected_typ: "array".to_string(), expr_typ: other.to_string(), expr_span: assign_span, }); @@ -252,6 +252,7 @@ impl<'interner> TypeChecker<'interner> { let element_type = Type::type_variable(self.interner.next_type_variable_id()); let expected_type = Type::MutableReference(Box::new(element_type.clone())); + self.unify(&reference_type, &expected_type, || TypeCheckError::TypeMismatch { expected_typ: expected_type.to_string(), expr_typ: reference_type.to_string(), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index 7216dd9a924..d7ab76e9faf 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -948,29 +948,34 @@ enum LValueRhs { Index(Expression), } -fn lvalue<'a, P>(expr_parser: P) -> impl NoirParser +fn lvalue<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - let l_ident = ident().map(LValue::Ident); + recursive(|lvalue| { + let l_ident = ident().map(LValue::Ident); - let l_member_rhs = just(Token::Dot).ignore_then(field_name()).map(LValueRhs::MemberAccess); + let dereferences = just(Token::Star) + .ignore_then(lvalue.clone()) + .map(|lvalue| LValue::Dereference(Box::new(lvalue))); - let l_index = expr_parser - .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) - .map(LValueRhs::Index); + let parenthesized = lvalue.delimited_by(just(Token::LeftParen), just(Token::RightParen)); + + let term = choice((parenthesized, dereferences, l_ident)); - let dereferences = just(Token::Star).repeated(); + let l_member_rhs = just(Token::Dot).ignore_then(field_name()).map(LValueRhs::MemberAccess); - let lvalues = - l_ident.then(l_member_rhs.or(l_index).repeated()).foldl(|lvalue, rhs| match rhs { + let l_index = expr_parser + .delimited_by(just(Token::LeftBracket), just(Token::RightBracket)) + .map(LValueRhs::Index); + + term.then(l_member_rhs.or(l_index).repeated()).foldl(|lvalue, rhs| match rhs { LValueRhs::MemberAccess(field_name) => { LValue::MemberAccess { object: Box::new(lvalue), field_name } } LValueRhs::Index(index) => LValue::Index { array: Box::new(lvalue), index }, - }); - - dereferences.then(lvalues).foldr(|_, lvalue| LValue::Dereference(Box::new(lvalue))) + }) + }) } fn parse_type<'a>() -> impl NoirParser + 'a { From 6d1bfa09e6edec2bb17d82a90f09e7ad3975b331 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Mon, 9 Oct 2023 15:53:32 -0500 Subject: [PATCH 2/2] Add regression --- .../tests/execution_success/assign_ex/src/main.nr | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tooling/nargo_cli/tests/execution_success/assign_ex/src/main.nr b/tooling/nargo_cli/tests/execution_success/assign_ex/src/main.nr index b0626d63c8e..75cd841a301 100644 --- a/tooling/nargo_cli/tests/execution_success/assign_ex/src/main.nr +++ b/tooling/nargo_cli/tests/execution_success/assign_ex/src/main.nr @@ -3,4 +3,13 @@ fn main(x: Field, y: Field) { assert(z == 3); z = x * y; assert(z == 2); + + regression_3057(); +} + +// Ensure parsing parenthesized lvalues works +fn regression_3057() { + let mut array = [[0, 1], [2, 3]]; + (array[0])[1] = 2; + assert(array[0][1] == 2); }