From c0b65f16f4ad46c4935839a8923e468b39ec54e4 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 25 Apr 2023 16:20:56 +0000 Subject: [PATCH 1/9] feat(keyword): added assert keyword --- crates/noirc_frontend/src/lexer/lexer.rs | 8 ++++++++ crates/noirc_frontend/src/lexer/token.rs | 3 +++ crates/noirc_frontend/src/parser/parser.rs | 14 +++++++++++--- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs index c1ff328a3ed..f772f8c4d04 100644 --- a/crates/noirc_frontend/src/lexer/lexer.rs +++ b/crates/noirc_frontend/src/lexer/lexer.rs @@ -560,6 +560,7 @@ fn test_basic_language_syntax() { x * y; }; constrain mul(five, ten) == 50; + assert ten + five == 15; "; let expected = vec![ @@ -601,6 +602,13 @@ fn test_basic_language_syntax() { Token::Equal, Token::Int(50_i128.into()), Token::Semicolon, + Token::Keyword(Keyword::Assert), + Token::Ident("ten".to_string()), + Token::Plus, + Token::Ident("five".to_string()), + Token::Equal, + Token::Int(15_i128.into()), + Token::Semicolon, Token::EOF, ]; let mut lexer = Lexer::new(input); diff --git a/crates/noirc_frontend/src/lexer/token.rs b/crates/noirc_frontend/src/lexer/token.rs index 0df1fc39938..ab762fe9008 100644 --- a/crates/noirc_frontend/src/lexer/token.rs +++ b/crates/noirc_frontend/src/lexer/token.rs @@ -414,6 +414,7 @@ impl AsRef for Attribute { #[cfg_attr(test, derive(strum_macros::EnumIter))] pub enum Keyword { As, + Assert, Bool, Char, CompTime, @@ -447,6 +448,7 @@ impl fmt::Display for Keyword { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { match *self { Keyword::As => write!(f, "as"), + Keyword::Assert => write!(f, "assert"), Keyword::Bool => write!(f, "bool"), Keyword::Char => write!(f, "char"), Keyword::CompTime => write!(f, "comptime"), @@ -483,6 +485,7 @@ impl Keyword { pub(crate) fn lookup_keyword(word: &str) -> Option { let keyword = match word { "as" => Keyword::As, + "assert" => Keyword::Assert, "bool" => Keyword::Bool, "char" => Keyword::Char, "comptime" => Keyword::CompTime, diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index f4793d06368..6926560e155 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -438,7 +438,7 @@ fn constrain<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - ignore_then_commit(keyword(Keyword::Constrain).labelled("statement"), expr_parser) + ignore_then_commit(one_of([Token::Keyword(Keyword::Constrain), Token::Keyword(Keyword::Assert)]).labelled("statement"), expr_parser) .map(|expr| Statement::Constrain(ConstrainStatement(expr))) } @@ -1172,10 +1172,11 @@ mod test { ); } - /// This is the standard way to declare a constrain statement + /// This is the standard way to declare a assert/constrain statement #[test] - fn parse_constrain() { + fn parse_assert_constrain() { parse_with(constrain(expression()), "constrain x == y").unwrap(); + parse_with(constrain(expression()), "assert x == y").unwrap(); // Currently we disallow constrain statements where the outer infix operator // produces a value. This would require an implicit `==` which @@ -1210,6 +1211,11 @@ mod test { "constrain (x ^ y) == y", "constrain (x ^ y) == (y + m)", "constrain x + x ^ x == y | m", + "assert ((x + y) == k) + z == y", + "assert (x + !y) == y", + "assert (x ^ y) == y", + "assert (x ^ y) == (y + m)", + "assert x + x ^ x == y | m", ], ); } @@ -1468,7 +1474,9 @@ mod test { ("let", 3, "let $error: unspecified = Error"), ("foo = one two three", 1, "foo = plain::one"), ("constrain", 1, "constrain Error"), + ("assert", 1, "assert Error"), ("constrain x ==", 1, "constrain (plain::x == Error)"), + ("assert x ==", 1, "assert (plain::x == Error)"), ]; let show_errors = |v| vecmap(v, ToString::to_string).join("\n"); From e98e7ea4b81893c1789b48c2c803316dba072f99 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 25 Apr 2023 16:24:49 +0000 Subject: [PATCH 2/9] test(keyword): added integration test --- crates/nargo_cli/tests/test_data/assert/Nargo.toml | 5 +++++ crates/nargo_cli/tests/test_data/assert/Prover.toml | 1 + crates/nargo_cli/tests/test_data/assert/src/main.nr | 3 +++ 3 files changed, 9 insertions(+) create mode 100644 crates/nargo_cli/tests/test_data/assert/Nargo.toml create mode 100644 crates/nargo_cli/tests/test_data/assert/Prover.toml create mode 100644 crates/nargo_cli/tests/test_data/assert/src/main.nr diff --git a/crates/nargo_cli/tests/test_data/assert/Nargo.toml b/crates/nargo_cli/tests/test_data/assert/Nargo.toml new file mode 100644 index 00000000000..e0b467ce5da --- /dev/null +++ b/crates/nargo_cli/tests/test_data/assert/Nargo.toml @@ -0,0 +1,5 @@ +[package] +authors = [""] +compiler_version = "0.1" + +[dependencies] \ No newline at end of file diff --git a/crates/nargo_cli/tests/test_data/assert/Prover.toml b/crates/nargo_cli/tests/test_data/assert/Prover.toml new file mode 100644 index 00000000000..4dd6b405159 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/assert/Prover.toml @@ -0,0 +1 @@ +x = "1" diff --git a/crates/nargo_cli/tests/test_data/assert/src/main.nr b/crates/nargo_cli/tests/test_data/assert/src/main.nr new file mode 100644 index 00000000000..ec12ef03f79 --- /dev/null +++ b/crates/nargo_cli/tests/test_data/assert/src/main.nr @@ -0,0 +1,3 @@ +fn main(x: Field) { + assert x == 1; +} From c2e878a7160b0fafc382e3bc05c79ba38a3d9334 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 07:45:53 +0000 Subject: [PATCH 3/9] fix(parser): separate parser for assertion --- crates/noirc_frontend/src/parser/parser.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 6926560e155..958a4e2af30 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -428,17 +428,27 @@ where { choice(( constrain(expr_parser.clone()), + assertion(expr_parser.clone()), declaration(expr_parser.clone()), assignment(expr_parser.clone()), expr_parser.map(Statement::Expression), )) } +fn assertion<'a, P>(expr_parser: P) -> impl NoirParser + 'a +where + P: ExprParser + 'a, +{ + ignore_then_commit(keyword(Keyword::Assert).labelled("statement"), expr_parser) + .map(|expr| Statement::Constrain(ConstrainStatement(expr))) +} + + fn constrain<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - ignore_then_commit(one_of([Token::Keyword(Keyword::Constrain), Token::Keyword(Keyword::Assert)]).labelled("statement"), expr_parser) + ignore_then_commit(keyword(Keyword::Constrain).labelled("statement"), expr_parser) .map(|expr| Statement::Constrain(ConstrainStatement(expr))) } From c081b5665884f3b195f708533a2057b1ba95431c Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 08:16:55 +0000 Subject: [PATCH 4/9] test(parser): fix test --- crates/noirc_frontend/src/parser/parser.rs | 46 +++++++++++++++++++--- 1 file changed, 41 insertions(+), 5 deletions(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 958a4e2af30..cf3f7bf0cb7 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -1182,11 +1182,10 @@ mod test { ); } - /// This is the standard way to declare a assert/constrain statement + /// This is the standard way to declare a constrain statement #[test] - fn parse_assert_constrain() { + fn parse_constrain() { parse_with(constrain(expression()), "constrain x == y").unwrap(); - parse_with(constrain(expression()), "assert x == y").unwrap(); // Currently we disallow constrain statements where the outer infix operator // produces a value. This would require an implicit `==` which @@ -1221,6 +1220,43 @@ mod test { "constrain (x ^ y) == y", "constrain (x ^ y) == (y + m)", "constrain x + x ^ x == y | m", + ], + ); + } + + /// The assert statement is syntax sugar for constrain + #[test] + fn parse_assert() { + parse_with(assertion(expression()), "assert x == y").unwrap(); + + // Currently we disallow assert statements where the outer infix operator + // produces a value. This would require an implicit `==` which + // may not be intuitive to the user. + // + // If this is deemed useful, one would either apply a transformation + // or interpret it with an `==` in the evaluator + let disallowed_operators = vec![ + BinaryOpKind::And, + BinaryOpKind::Subtract, + BinaryOpKind::Divide, + BinaryOpKind::Multiply, + BinaryOpKind::Or, + ]; + + for operator in disallowed_operators { + let src = format!("assert x {} y;", operator.as_string()); + parse_with(assertion(expression()), &src).unwrap_err(); + } + + // These are general cases which should always work. + // + // The first case is the most noteworthy. It contains two `==` + // The first (inner) `==` is a predicate which returns 0/1 + // The outer layer is an infix `==` which is + // associated with the Constrain statement + parse_all( + assertion(expression()), + vec![ "assert ((x + y) == k) + z == y", "assert (x + !y) == y", "assert (x ^ y) == y", @@ -1484,9 +1520,9 @@ mod test { ("let", 3, "let $error: unspecified = Error"), ("foo = one two three", 1, "foo = plain::one"), ("constrain", 1, "constrain Error"), - ("assert", 1, "assert Error"), + ("assert", 1, "constrain Error"), ("constrain x ==", 1, "constrain (plain::x == Error)"), - ("assert x ==", 1, "assert (plain::x == Error)"), + ("assert x ==", 1, "constrain (plain::x == Error)"), ]; let show_errors = |v| vecmap(v, ToString::to_string).join("\n"); From cb8ad8860387fb5ebac160b4d4a0db24cabbc0b6 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 08:18:43 +0000 Subject: [PATCH 5/9] style(parser): fix whitespaces --- crates/noirc_frontend/src/parser/parser.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index cf3f7bf0cb7..3b694ff032b 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -443,7 +443,6 @@ where .map(|expr| Statement::Constrain(ConstrainStatement(expr))) } - fn constrain<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, From 31461b3cd29b1feb35824b22e8d2c7d878639e1e Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 10:44:41 +0000 Subject: [PATCH 6/9] refactor(keyword): assert use constrain parser --- crates/noirc_frontend/src/parser/parser.rs | 55 ++-------------------- 1 file changed, 3 insertions(+), 52 deletions(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 3b694ff032b..f967b90e15d 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -428,26 +428,17 @@ where { choice(( constrain(expr_parser.clone()), - assertion(expr_parser.clone()), declaration(expr_parser.clone()), assignment(expr_parser.clone()), expr_parser.map(Statement::Expression), )) } -fn assertion<'a, P>(expr_parser: P) -> impl NoirParser + 'a -where - P: ExprParser + 'a, -{ - ignore_then_commit(keyword(Keyword::Assert).labelled("statement"), expr_parser) - .map(|expr| Statement::Constrain(ConstrainStatement(expr))) -} - fn constrain<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - ignore_then_commit(keyword(Keyword::Constrain).labelled("statement"), expr_parser) + ignore_then_commit(choice((keyword(Keyword::Assert), keyword(Keyword::Constrain))).labelled("statement"), expr_parser) .map(|expr| Statement::Constrain(ConstrainStatement(expr))) } @@ -1185,6 +1176,8 @@ mod test { #[test] fn parse_constrain() { parse_with(constrain(expression()), "constrain x == y").unwrap(); + // Assert is syntax sugar for constrain + parse_with(constrain(expression()), "assert x == y").unwrap(); // Currently we disallow constrain statements where the outer infix operator // produces a value. This would require an implicit `==` which @@ -1223,48 +1216,6 @@ mod test { ); } - /// The assert statement is syntax sugar for constrain - #[test] - fn parse_assert() { - parse_with(assertion(expression()), "assert x == y").unwrap(); - - // Currently we disallow assert statements where the outer infix operator - // produces a value. This would require an implicit `==` which - // may not be intuitive to the user. - // - // If this is deemed useful, one would either apply a transformation - // or interpret it with an `==` in the evaluator - let disallowed_operators = vec![ - BinaryOpKind::And, - BinaryOpKind::Subtract, - BinaryOpKind::Divide, - BinaryOpKind::Multiply, - BinaryOpKind::Or, - ]; - - for operator in disallowed_operators { - let src = format!("assert x {} y;", operator.as_string()); - parse_with(assertion(expression()), &src).unwrap_err(); - } - - // These are general cases which should always work. - // - // The first case is the most noteworthy. It contains two `==` - // The first (inner) `==` is a predicate which returns 0/1 - // The outer layer is an infix `==` which is - // associated with the Constrain statement - parse_all( - assertion(expression()), - vec![ - "assert ((x + y) == k) + z == y", - "assert (x + !y) == y", - "assert (x ^ y) == y", - "assert (x ^ y) == (y + m)", - "assert x + x ^ x == y | m", - ], - ); - } - #[test] fn parse_let() { // Why is it valid to specify a let declaration as having type u8? From 910740e005b380ece3712a6f7fed7d74d4c96ab5 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 16:00:40 +0000 Subject: [PATCH 7/9] feat(parser): give assertions function form --- .../tests/test_data/assert/src/main.nr | 2 +- crates/noirc_frontend/src/parser/parser.rs | 56 +++++++++++++++++-- 2 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/nargo_cli/tests/test_data/assert/src/main.nr b/crates/nargo_cli/tests/test_data/assert/src/main.nr index ec12ef03f79..00e94414c0b 100644 --- a/crates/nargo_cli/tests/test_data/assert/src/main.nr +++ b/crates/nargo_cli/tests/test_data/assert/src/main.nr @@ -1,3 +1,3 @@ fn main(x: Field) { - assert x == 1; + assert(x == 1); } diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index f967b90e15d..440f4ff7e8e 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -428,6 +428,7 @@ where { choice(( constrain(expr_parser.clone()), + assertion(expr_parser.clone()), declaration(expr_parser.clone()), assignment(expr_parser.clone()), expr_parser.map(Statement::Expression), @@ -438,7 +439,15 @@ fn constrain<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - ignore_then_commit(choice((keyword(Keyword::Assert), keyword(Keyword::Constrain))).labelled("statement"), expr_parser) + ignore_then_commit(keyword(Keyword::Constrain).labelled("statement"), expr_parser) + .map(|expr| Statement::Constrain(ConstrainStatement(expr))) +} + +fn assertion<'a, P>(expr_parser: P) -> impl NoirParser + 'a +where + P: ExprParser + 'a, +{ + ignore_then_commit(keyword(Keyword::Assert).labelled("statement"), parenthesized(expr_parser)) .map(|expr| Statement::Constrain(ConstrainStatement(expr))) } @@ -1176,8 +1185,6 @@ mod test { #[test] fn parse_constrain() { parse_with(constrain(expression()), "constrain x == y").unwrap(); - // Assert is syntax sugar for constrain - parse_with(constrain(expression()), "assert x == y").unwrap(); // Currently we disallow constrain statements where the outer infix operator // produces a value. This would require an implicit `==` which @@ -1216,6 +1223,47 @@ mod test { ); } + #[test] + fn parse_assert() { + parse_with(assertion(expression()), "assert(x == y)").unwrap(); + + // Currently we disallow constrain statements where the outer infix operator + // produces a value. This would require an implicit `==` which + // may not be intuitive to the user. + // + // If this is deemed useful, one would either apply a transformation + // or interpret it with an `==` in the evaluator + let disallowed_operators = vec![ + BinaryOpKind::And, + BinaryOpKind::Subtract, + BinaryOpKind::Divide, + BinaryOpKind::Multiply, + BinaryOpKind::Or, + ]; + + for operator in disallowed_operators { + let src = format!("assert(x {} y);", operator.as_string()); + parse_with(assertion(expression()), &src).unwrap_err(); + } + + // These are general cases which should always work. + // + // The first case is the most noteworthy. It contains two `==` + // The first (inner) `==` is a predicate which returns 0/1 + // The outer layer is an infix `==` which is + // associated with the Constrain statement + parse_all( + assertion(expression()), + vec![ + "assert(((x + y) == k) + z == y)", + "assert((x + !y) == y)", + "assert((x ^ y) == y)", + "assert((x ^ y) == (y + m))", + "assert(x + x ^ x == y | m)", + ], + ); + } + #[test] fn parse_let() { // Why is it valid to specify a let declaration as having type u8? @@ -1472,7 +1520,7 @@ mod test { ("constrain", 1, "constrain Error"), ("assert", 1, "constrain Error"), ("constrain x ==", 1, "constrain (plain::x == Error)"), - ("assert x ==", 1, "constrain (plain::x == Error)"), + ("assert(x ==)", 1, "constrain (plain::x == Error)"), ]; let show_errors = |v| vecmap(v, ToString::to_string).join("\n"); From f0e294391b69e796874e0b4572a4cfd587b6862b Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 16:02:35 +0000 Subject: [PATCH 8/9] fix(lexer): update the basic test --- crates/noirc_frontend/src/lexer/lexer.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/lexer/lexer.rs b/crates/noirc_frontend/src/lexer/lexer.rs index f772f8c4d04..5e0d99cfed9 100644 --- a/crates/noirc_frontend/src/lexer/lexer.rs +++ b/crates/noirc_frontend/src/lexer/lexer.rs @@ -560,7 +560,7 @@ fn test_basic_language_syntax() { x * y; }; constrain mul(five, ten) == 50; - assert ten + five == 15; + assert(ten + five == 15); "; let expected = vec![ @@ -603,11 +603,13 @@ fn test_basic_language_syntax() { Token::Int(50_i128.into()), Token::Semicolon, Token::Keyword(Keyword::Assert), + Token::LeftParen, Token::Ident("ten".to_string()), Token::Plus, Token::Ident("five".to_string()), Token::Equal, Token::Int(15_i128.into()), + Token::RightParen, Token::Semicolon, Token::EOF, ]; From 567ff45641d9dce1b7a76f9146dc7dc2fc8a6c50 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 26 Apr 2023 16:09:27 +0000 Subject: [PATCH 9/9] style: label the whole assertion statement --- crates/noirc_frontend/src/parser/parser.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index 440f4ff7e8e..369985b4aec 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -447,7 +447,8 @@ fn assertion<'a, P>(expr_parser: P) -> impl NoirParser + 'a where P: ExprParser + 'a, { - ignore_then_commit(keyword(Keyword::Assert).labelled("statement"), parenthesized(expr_parser)) + ignore_then_commit(keyword(Keyword::Assert), parenthesized(expr_parser)) + .labelled("statement") .map(|expr| Statement::Constrain(ConstrainStatement(expr))) }