From c529ec720d095a5f2270a8074c4399fc65787ef3 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 12:05:16 +0000 Subject: [PATCH 1/6] add negative integer literals --- compiler/noirc_evaluator/src/ssa/ir/types.rs | 8 +------- compiler/noirc_frontend/src/ast/expression.rs | 13 ++++++++++++- .../noirc_frontend/src/hir/resolution/resolver.rs | 1 + compiler/noirc_frontend/src/hir/type_check/expr.rs | 4 +++- compiler/noirc_frontend/src/hir_def/expr.rs | 1 + compiler/noirc_frontend/src/monomorphization/mod.rs | 12 ++++++++++++ .../execution_success/regression_3394/Nargo.toml | 6 ++++++ .../execution_success/regression_3394/Prover.toml | 1 + .../execution_success/regression_3394/src/main.nr | 6 ++++++ tooling/nargo_fmt/src/rewrite/expr.rs | 1 + 10 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 test_programs/execution_success/regression_3394/Nargo.toml create mode 100644 test_programs/execution_success/regression_3394/Prover.toml create mode 100644 test_programs/execution_success/regression_3394/src/main.nr diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index fbc95a16387..bae06a805d0 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -127,13 +127,7 @@ impl NumericType { /// for the current NumericType. pub(crate) fn value_is_within_limits(self, field: FieldElement) -> bool { match self { - NumericType::Signed { bit_size } => { - let min = -(2i128.pow(bit_size - 1)); - let max = 2u128.pow(bit_size - 1) - 1; - // Signed integers are odd since they will overflow the field value - field <= max.into() || field >= min.into() - } - NumericType::Unsigned { bit_size } => { + NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { let max = 2u128.pow(bit_size) - 1; field <= max.into() } diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 41807d7eca7..32de6ec81e9 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -50,7 +50,15 @@ impl ExpressionKind { } pub fn prefix(operator: UnaryOp, rhs: Expression) -> ExpressionKind { - ExpressionKind::Prefix(Box::new(PrefixExpression { operator, rhs })) + match (operator, &rhs) { + ( + UnaryOp::Minus, + Expression { kind: ExpressionKind::Literal(Literal::Integer(field)), .. }, + ) => { + ExpressionKind::Literal(Literal::NegativeInteger(*field)) + } + _ => ExpressionKind::Prefix(Box::new(PrefixExpression { operator, rhs })), + } } pub fn array(contents: Vec) -> ExpressionKind { @@ -65,6 +73,7 @@ impl ExpressionKind { } pub fn integer(contents: FieldElement) -> ExpressionKind { + // todo!(); ExpressionKind::Literal(Literal::Integer(contents)) } @@ -315,6 +324,7 @@ pub enum Literal { Array(ArrayLiteral), Bool(bool), Integer(FieldElement), + NegativeInteger(FieldElement), Str(String), RawStr(String, u8), FmtStr(String), @@ -519,6 +529,7 @@ impl Display for Literal { } Literal::FmtStr(string) => write!(f, "f\"{string}\""), Literal::Unit => write!(f, "()"), + Literal::NegativeInteger(integer) => write!(f, "-{}", integer.to_u128()), } } } diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 4e7ed7e2ea9..511a172508d 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1203,6 +1203,7 @@ impl<'a> Resolver<'a> { HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) } Literal::Integer(integer) => HirLiteral::Integer(integer), + Literal::NegativeInteger(integer) => HirLiteral::NegativeInteger(integer), Literal::Str(str) => HirLiteral::Str(str), Literal::RawStr(str, _) => HirLiteral::Str(str), Literal::FmtStr(str) => self.resolve_fmt_str_literal(str, expr.span), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index 74f076212fa..ef2b2114b4a 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -114,7 +114,9 @@ impl<'interner> TypeChecker<'interner> { Type::Array(Box::new(length), Box::new(elem_type)) } HirLiteral::Bool(_) => Type::Bool, - HirLiteral::Integer(_) => Type::polymorphic_integer(self.interner), + HirLiteral::Integer(_) | HirLiteral::NegativeInteger(_) => { + Type::polymorphic_integer(self.interner) + } HirLiteral::Str(string) => { let len = Type::Constant(string.len() as u64); Type::String(Box::new(len)) diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 755817ba1e6..8cf462a1ea5 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -79,6 +79,7 @@ pub enum HirLiteral { Array(HirArrayLiteral), Bool(bool), Integer(FieldElement), + NegativeInteger(FieldElement), Str(String), FmtStr(String, Vec), Unit, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 4f035e0fc2e..0741909925e 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -321,6 +321,18 @@ impl<'interner> Monomorphizer<'interner> { let location = self.interner.id_location(expr); Literal(Integer(value, typ, location)) } + HirExpression::Literal(HirLiteral::NegativeInteger(value)) => { + let typ = self.convert_type(&self.interner.id_type(expr)); + let location = self.interner.id_location(expr); + match typ { + ast::Type::Field => Literal(Integer(-value, typ, location)), + ast::Type::Integer(_, bit_size) => { + let base = 1_u128 << bit_size; + Literal(Integer(FieldElement::from(base) - value, typ, location)) + } + _ => unreachable!("Integer literal must be numeric"), + } + } HirExpression::Literal(HirLiteral::Array(array)) => match array { HirArrayLiteral::Standard(array) => self.standard_array(expr, array), HirArrayLiteral::Repeated { repeated_element, length } => { diff --git a/test_programs/execution_success/regression_3394/Nargo.toml b/test_programs/execution_success/regression_3394/Nargo.toml new file mode 100644 index 00000000000..4949962b16a --- /dev/null +++ b/test_programs/execution_success/regression_3394/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "regression_3394" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/execution_success/regression_3394/Prover.toml b/test_programs/execution_success/regression_3394/Prover.toml new file mode 100644 index 00000000000..8b137891791 --- /dev/null +++ b/test_programs/execution_success/regression_3394/Prover.toml @@ -0,0 +1 @@ + diff --git a/test_programs/execution_success/regression_3394/src/main.nr b/test_programs/execution_success/regression_3394/src/main.nr new file mode 100644 index 00000000000..cc45487b98b --- /dev/null +++ b/test_programs/execution_success/regression_3394/src/main.nr @@ -0,0 +1,6 @@ +use dep::std; + +fn main() { + let x : i8 = -128; + std::println(x); +} \ No newline at end of file diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 3c46319c1aa..55d280effb4 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -111,6 +111,7 @@ pub(crate) fn rewrite( ), ExpressionKind::Literal(literal) => match literal { Literal::Integer(_) + | Literal::NegativeInteger(_) | Literal::Bool(_) | Literal::Str(_) | Literal::RawStr(..) From 70a7a31bafce71e1ec628f5482d3a4959567b95f Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 12:15:53 +0000 Subject: [PATCH 2/6] format --- compiler/noirc_frontend/src/ast/expression.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 32de6ec81e9..87a987896c9 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -54,9 +54,7 @@ impl ExpressionKind { ( UnaryOp::Minus, Expression { kind: ExpressionKind::Literal(Literal::Integer(field)), .. }, - ) => { - ExpressionKind::Literal(Literal::NegativeInteger(*field)) - } + ) => ExpressionKind::Literal(Literal::NegativeInteger(*field)), _ => ExpressionKind::Prefix(Box::new(PrefixExpression { operator, rhs })), } } From cbd2bcf8a20b4029f8fd69099d3860802d4e8321 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 15:53:36 +0000 Subject: [PATCH 3/6] remove debug comment --- compiler/noirc_frontend/src/ast/expression.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 87a987896c9..abcbf103e9e 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -71,7 +71,6 @@ impl ExpressionKind { } pub fn integer(contents: FieldElement) -> ExpressionKind { - // todo!(); ExpressionKind::Literal(Literal::Integer(contents)) } From 39605c0a7f0e06af7baa5937f79c02fa5bc1dde3 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 16:19:47 +0000 Subject: [PATCH 4/6] use a flag on Integer for the sign --- compiler/noirc_frontend/src/ast/expression.rs | 20 ++++++++----- compiler/noirc_frontend/src/ast/mod.rs | 11 ++++--- .../src/hir/resolution/resolver.rs | 5 ++-- .../noirc_frontend/src/hir/type_check/expr.rs | 4 +-- .../noirc_frontend/src/hir/type_check/stmt.rs | 2 +- compiler/noirc_frontend/src/hir_def/expr.rs | 3 +- .../src/monomorphization/mod.rs | 29 ++++++++++--------- tooling/nargo_fmt/src/rewrite/expr.rs | 3 +- 8 files changed, 40 insertions(+), 37 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index abcbf103e9e..e4929acc708 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -53,8 +53,8 @@ impl ExpressionKind { match (operator, &rhs) { ( UnaryOp::Minus, - Expression { kind: ExpressionKind::Literal(Literal::Integer(field)), .. }, - ) => ExpressionKind::Literal(Literal::NegativeInteger(*field)), + Expression { kind: ExpressionKind::Literal(Literal::Integer(field, sign)), .. }, + ) => ExpressionKind::Literal(Literal::Integer(*field, !sign)), _ => ExpressionKind::Prefix(Box::new(PrefixExpression { operator, rhs })), } } @@ -71,7 +71,7 @@ impl ExpressionKind { } pub fn integer(contents: FieldElement) -> ExpressionKind { - ExpressionKind::Literal(Literal::Integer(contents)) + ExpressionKind::Literal(Literal::Integer(contents, false)) } pub fn boolean(contents: bool) -> ExpressionKind { @@ -106,7 +106,7 @@ impl ExpressionKind { }; match literal { - Literal::Integer(integer) => Some(*integer), + Literal::Integer(integer, _) => Some(*integer), _ => None, } } @@ -320,8 +320,7 @@ impl UnaryOp { pub enum Literal { Array(ArrayLiteral), Bool(bool), - Integer(FieldElement), - NegativeInteger(FieldElement), + Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative Str(String), RawStr(String, u8), FmtStr(String), @@ -517,7 +516,13 @@ impl Display for Literal { write!(f, "[{repeated_element}; {length}]") } Literal::Bool(boolean) => write!(f, "{}", if *boolean { "true" } else { "false" }), - Literal::Integer(integer) => write!(f, "{}", integer.to_u128()), + Literal::Integer(integer, sign) => { + if *sign { + write!(f, "-{}", integer.to_u128()) + } else { + write!(f, "{}", integer.to_u128()) + } + } Literal::Str(string) => write!(f, "\"{string}\""), Literal::RawStr(string, num_hashes) => { let hashes: String = @@ -526,7 +531,6 @@ impl Display for Literal { } Literal::FmtStr(string) => write!(f, "f\"{string}\""), Literal::Unit => write!(f, "()"), - Literal::NegativeInteger(integer) => write!(f, "-{}", integer.to_u128()), } } } diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 2fbe73dafef..5c10d3fe8f0 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -234,10 +234,13 @@ impl UnresolvedTypeExpression { fn from_expr_helper(expr: Expression) -> Result { match expr.kind { - ExpressionKind::Literal(Literal::Integer(int)) => match int.try_to_u64() { - Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)), - None => Err(expr), - }, + ExpressionKind::Literal(Literal::Integer(int, sign)) => { + assert!(!sign, "Negative literal is not allowed here"); + match int.try_to_u64() { + Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)), + None => Err(expr), + } + } ExpressionKind::Variable(path) => Ok(UnresolvedTypeExpression::Variable(path)), ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => { let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span)); diff --git a/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 511a172508d..5c94ec7644a 100644 --- a/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1202,8 +1202,7 @@ impl<'a> Resolver<'a> { HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length }) } - Literal::Integer(integer) => HirLiteral::Integer(integer), - Literal::NegativeInteger(integer) => HirLiteral::NegativeInteger(integer), + Literal::Integer(integer, sign) => HirLiteral::Integer(integer, sign), Literal::Str(str) => HirLiteral::Str(str), Literal::RawStr(str, _) => HirLiteral::Str(str), Literal::FmtStr(str) => self.resolve_fmt_str_literal(str, expr.span), @@ -1692,7 +1691,7 @@ impl<'a> Resolver<'a> { span: Span, ) -> Result> { match self.interner.expression(&rhs) { - HirExpression::Literal(HirLiteral::Integer(int)) => { + HirExpression::Literal(HirLiteral::Integer(int, false)) => { int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) } _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), diff --git a/compiler/noirc_frontend/src/hir/type_check/expr.rs b/compiler/noirc_frontend/src/hir/type_check/expr.rs index ef2b2114b4a..e4d3a0be342 100644 --- a/compiler/noirc_frontend/src/hir/type_check/expr.rs +++ b/compiler/noirc_frontend/src/hir/type_check/expr.rs @@ -114,9 +114,7 @@ impl<'interner> TypeChecker<'interner> { Type::Array(Box::new(length), Box::new(elem_type)) } HirLiteral::Bool(_) => Type::Bool, - HirLiteral::Integer(_) | HirLiteral::NegativeInteger(_) => { - Type::polymorphic_integer(self.interner) - } + HirLiteral::Integer(_, _) => Type::polymorphic_integer(self.interner), HirLiteral::Str(string) => { let len = Type::Constant(string.len() as u64); Type::String(Box::new(len)) diff --git a/compiler/noirc_frontend/src/hir/type_check/stmt.rs b/compiler/noirc_frontend/src/hir/type_check/stmt.rs index e289ae0fc9d..aa2a947c961 100644 --- a/compiler/noirc_frontend/src/hir/type_check/stmt.rs +++ b/compiler/noirc_frontend/src/hir/type_check/stmt.rs @@ -350,7 +350,7 @@ impl<'interner> TypeChecker<'interner> { let expr = self.interner.expression(rhs_expr); let span = self.interner.expr_span(rhs_expr); match expr { - HirExpression::Literal(HirLiteral::Integer(value)) => { + HirExpression::Literal(HirLiteral::Integer(value, false)) => { let v = value.to_u128(); if let Type::Integer(_, bit_count) = annotated_type { let max = 1 << bit_count; diff --git a/compiler/noirc_frontend/src/hir_def/expr.rs b/compiler/noirc_frontend/src/hir_def/expr.rs index 8cf462a1ea5..ef1c3af7ac0 100644 --- a/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/compiler/noirc_frontend/src/hir_def/expr.rs @@ -78,8 +78,7 @@ impl HirBinaryOp { pub enum HirLiteral { Array(HirArrayLiteral), Bool(bool), - Integer(FieldElement), - NegativeInteger(FieldElement), + Integer(FieldElement, bool), //true for negative integer and false for positive Str(String), FmtStr(String, Vec), Unit, diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 0741909925e..5f8c5535ca4 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -316,21 +316,22 @@ impl<'interner> Monomorphizer<'interner> { )) } HirExpression::Literal(HirLiteral::Bool(value)) => Literal(Bool(value)), - HirExpression::Literal(HirLiteral::Integer(value)) => { - let typ = self.convert_type(&self.interner.id_type(expr)); - let location = self.interner.id_location(expr); - Literal(Integer(value, typ, location)) - } - HirExpression::Literal(HirLiteral::NegativeInteger(value)) => { - let typ = self.convert_type(&self.interner.id_type(expr)); - let location = self.interner.id_location(expr); - match typ { - ast::Type::Field => Literal(Integer(-value, typ, location)), - ast::Type::Integer(_, bit_size) => { - let base = 1_u128 << bit_size; - Literal(Integer(FieldElement::from(base) - value, typ, location)) + HirExpression::Literal(HirLiteral::Integer(value, sign)) => { + if sign { + let typ = self.convert_type(&self.interner.id_type(expr)); + let location = self.interner.id_location(expr); + match typ { + ast::Type::Field => Literal(Integer(-value, typ, location)), + ast::Type::Integer(_, bit_size) => { + let base = 1_u128 << bit_size; + Literal(Integer(FieldElement::from(base) - value, typ, location)) + } + _ => unreachable!("Integer literal must be numeric"), } - _ => unreachable!("Integer literal must be numeric"), + } else { + let typ = self.convert_type(&self.interner.id_type(expr)); + let location = self.interner.id_location(expr); + Literal(Integer(value, typ, location)) } } HirExpression::Literal(HirLiteral::Array(array)) => match array { diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs index 55d280effb4..32d104f559b 100644 --- a/tooling/nargo_fmt/src/rewrite/expr.rs +++ b/tooling/nargo_fmt/src/rewrite/expr.rs @@ -110,8 +110,7 @@ pub(crate) fn rewrite( NewlineMode::Normal, ), ExpressionKind::Literal(literal) => match literal { - Literal::Integer(_) - | Literal::NegativeInteger(_) + Literal::Integer(_, _) | Literal::Bool(_) | Literal::Str(_) | Literal::RawStr(..) From 42d0790ff917663351dd95d6beb321d5a018bd56 Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 17:05:45 +0000 Subject: [PATCH 5/6] fix build --- compiler/noirc_frontend/src/ast/expression.rs | 2 +- compiler/noirc_frontend/src/parser/parser.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index e4929acc708..c78deaf6dbb 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -320,7 +320,7 @@ impl UnaryOp { pub enum Literal { Array(ArrayLiteral), Bool(bool), - Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative + Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative Str(String), RawStr(String, u8), FmtStr(String), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index cc85fe88205..088aa7dd4fb 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -2264,7 +2264,7 @@ mod test { let hex = parse_with(literal(), "0x05").unwrap(); match (expr_to_lit(int), expr_to_lit(hex)) { - (Literal::Integer(int), Literal::Integer(hex)) => assert_eq!(int, hex), + (Literal::Integer(int, false), Literal::Integer(hex, false)) => assert_eq!(int, hex), _ => unreachable!(), } } From ae69409e17437eb80866e98dc344e01ad90666de Mon Sep 17 00:00:00 2001 From: guipublic Date: Tue, 5 Dec 2023 17:17:41 +0000 Subject: [PATCH 6/6] fix the build #2 --- aztec_macros/src/lib.rs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/aztec_macros/src/lib.rs b/aztec_macros/src/lib.rs index 6d3aa0d8b01..1b433c33df3 100644 --- a/aztec_macros/src/lib.rs +++ b/aztec_macros/src/lib.rs @@ -914,9 +914,10 @@ fn create_loop_over(var: Expression, loop_body: Vec) -> Statement { // `for i in 0..{ident}.len()` make_statement(StatementKind::For(ForLoopStatement { range: ForRange::Range( - expression(ExpressionKind::Literal(Literal::Integer(FieldElement::from(i128::from( - 0, - ))))), + expression(ExpressionKind::Literal(Literal::Integer( + FieldElement::from(i128::from(0)), + false, + ))), end_range_expression, ), identifier: ident("i"),