fix: add negative integer literals (#3690)

# Description ## Problem\* Resolves #3394 ## Summary\* Add negative integer literals so that -128 becomes now a literal instead of a unary expression. ## Additional Context ## Documentation\* Check one: - [X] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [X] I have tested the changes locally. - [X] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
noir-lang · Dec 5, 2023 · 8b3a68f · 8b3a68f
1 parent 7f1d796
commit 8b3a68f
Show file tree

Hide file tree

Showing 14 changed files with 66 additions and 30 deletions.
diff --git 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>) -> 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"),

diff --git 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
@@ -50,7 +50,13 @@ 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, sign)), .. },
+            ) => ExpressionKind::Literal(Literal::Integer(*field, !sign)),
+            _ => ExpressionKind::Prefix(Box::new(PrefixExpression { operator, rhs })),
+        }
     }
 
     pub fn array(contents: Vec<Expression>) -> ExpressionKind {
@@ -65,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 {
@@ -100,7 +106,7 @@ impl ExpressionKind {
         };
 
         match literal {
-            Literal::Integer(integer) => Some(*integer),
+            Literal::Integer(integer, _) => Some(*integer),
             _ => None,
         }
     }
@@ -314,7 +320,7 @@ impl UnaryOp {
 pub enum Literal {
     Array(ArrayLiteral),
     Bool(bool),
-    Integer(FieldElement),
+    Integer(FieldElement, /*sign*/ bool), // false for positive integer and true for negative
     Str(String),
     RawStr(String, u8),
     FmtStr(String),
@@ -510,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 =

diff --git 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<UnresolvedTypeExpression, Expression> {
         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
@@ -1199,7 +1199,7 @@ impl<'a> Resolver<'a> {
 
                     HirLiteral::Array(HirArrayLiteral::Repeated { repeated_element, length })
                 }
-                Literal::Integer(integer) => HirLiteral::Integer(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),
@@ -1688,7 +1688,7 @@ impl<'a> Resolver<'a> {
         span: Span,
     ) -> Result<u128, Option<ResolverError>> {
         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
@@ -114,7 +114,7 @@ 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(_, _) => 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
@@ -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
@@ -78,7 +78,7 @@ impl HirBinaryOp {
 pub enum HirLiteral {
     Array(HirArrayLiteral),
     Bool(bool),
-    Integer(FieldElement),
+    Integer(FieldElement, bool), //true for negative integer and false for positive
     Str(String),
     FmtStr(String, Vec<ExprId>),
     Unit,

diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs
@@ -316,10 +316,23 @@ 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::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"),
+                    }
+                } 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 {
                 HirArrayLiteral::Standard(array) => self.standard_array(expr, array),

diff --git 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!(),
         }
     }

diff --git a/test_programs/execution_success/regression_3394/Nargo.toml 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
@@ -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
@@ -0,0 +1,6 @@
+use dep::std;
+
+fn main() {
+    let x : i8 = -128;
+    std::println(x);
+}
diff --git a/tooling/nargo_fmt/src/rewrite/expr.rs b/tooling/nargo_fmt/src/rewrite/expr.rs
@@ -110,7 +110,7 @@ pub(crate) fn rewrite(
             NewlineMode::Normal,
         ),
         ExpressionKind::Literal(literal) => match literal {
-            Literal::Integer(_)
+            Literal::Integer(_, _)
             | Literal::Bool(_)
             | Literal::Str(_)
             | Literal::RawStr(..)