Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: add negative integer literals #3690

Merged
merged 8 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 4 additions & 3 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"),
Expand Down
8 changes: 1 addition & 7 deletions compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
22 changes: 17 additions & 5 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -100,7 +106,7 @@ impl ExpressionKind {
};

match literal {
Literal::Integer(integer) => Some(*integer),
Literal::Integer(integer, _) => Some(*integer),
_ => None,
}
}
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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 =
Expand Down
11 changes: 7 additions & 4 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1202,7 +1202,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),
Expand Down Expand Up @@ -1691,7 +1691,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 })),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 17 additions & 4 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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!(),
}
}
Expand Down
6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_3394/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_3394"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

6 changes: 6 additions & 0 deletions test_programs/execution_success/regression_3394/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
use dep::std;

fn main() {
let x : i8 = -128;
std::println(x);
}
2 changes: 1 addition & 1 deletion tooling/nargo_fmt/src/rewrite/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(..)
Expand Down
Loading