diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs index 253c7dc6769..7869f59d4cf 100644 --- a/compiler/noirc_driver/src/abi_gen.rs +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -226,9 +226,10 @@ pub(super) fn value_from_hir_expression(context: &Context, expression: HirExpres }, HirLiteral::Bool(value) => AbiValue::Boolean { value }, HirLiteral::Str(value) => AbiValue::String { value }, - HirLiteral::Integer(value) => { - AbiValue::Integer { value: value.field.to_hex(), sign: value.is_negative } - } + HirLiteral::Integer(value) => AbiValue::Integer { + value: value.absolute_value().to_hex(), + sign: value.is_negative(), + }, _ => unreachable!("Literal cannot be used in the abi"), }, _ => unreachable!("Type cannot be used in the abi {:?}", expression), diff --git a/compiler/noirc_evaluator/src/ssa/ir/types.rs b/compiler/noirc_evaluator/src/ssa/ir/types.rs index 4bc80e8cc82..5384780e5ff 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/types.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/types.rs @@ -62,16 +62,20 @@ impl NumericType { match self { NumericType::Unsigned { bit_size } => { let max = if bit_size == 128 { u128::MAX } else { 2u128.pow(bit_size) - 1 }; - if value.is_negative { + if value.is_negative() { return Some(format!("0..={}", max)); } - if value.field <= max.into() { None } else { Some(format!("0..={}", max)) } + if value.absolute_value() <= max.into() { + None + } else { + Some(format!("0..={}", max)) + } } NumericType::Signed { bit_size } => { let min = 2u128.pow(bit_size - 1); let max = 2u128.pow(bit_size - 1) - 1; - let target_max = if value.is_negative { min } else { max }; - if value.field <= target_max.into() { + let target_max = if value.is_negative() { min } else { max }; + if value.absolute_value() <= target_max.into() { None } else { Some(format!("-{}..={}", min, max)) diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs index 8dcb2d65607..16f18ed2d21 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs @@ -297,17 +297,17 @@ impl<'a> FunctionContext<'a> { }); } - let value = if value.is_negative { + let value = if value.is_negative() { match numeric_type { - NumericType::NativeField => -value.field, + NumericType::NativeField => -value.absolute_value(), NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => { assert!(bit_size < 128); let base = 1_u128 << bit_size; - FieldElement::from(base) - value.field + FieldElement::from(base) - value.absolute_value() } } } else { - value.field + value.absolute_value() }; Ok(self.builder.numeric_constant(value, numeric_type)) diff --git a/compiler/noirc_frontend/src/elaborator/lints.rs b/compiler/noirc_frontend/src/elaborator/lints.rs index fe330f24515..6a8812470f8 100644 --- a/compiler/noirc_frontend/src/elaborator/lints.rs +++ b/compiler/noirc_frontend/src/elaborator/lints.rs @@ -222,7 +222,7 @@ pub(crate) fn overflowing_int( Type::Integer(Signedness::Unsigned, bit_size) => { let bit_size: u32 = (*bit_size).into(); let max = if bit_size == 128 { u128::MAX } else { 2u128.pow(bit_size) - 1 }; - if value.field > max.into() || value.is_negative { + if value.absolute_value() > max.into() || value.is_negative() { errors.push(TypeCheckError::OverflowingAssignment { expr: value, ty: annotated_type.clone(), @@ -235,9 +235,11 @@ pub(crate) fn overflowing_int( let bit_count: u32 = (*bit_count).into(); let min = 2u128.pow(bit_count - 1); let max = 2u128.pow(bit_count - 1) - 1; - if (value.is_negative && value.field > min.into()) - || (!value.is_negative && value.field > max.into()) - { + + let is_negative = value.is_negative(); + let abs = value.absolute_value(); + + if (is_negative && abs > min.into()) || (!is_negative && abs > max.into()) { errors.push(TypeCheckError::OverflowingAssignment { expr: value, ty: annotated_type.clone(), diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 939c20d2827..4bfd270caa0 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -37,7 +37,6 @@ use crate::{ TraitMethodId, }, shared::Signedness, - signed_field::SignedField, token::SecondaryAttributeKind, }; @@ -1141,7 +1140,9 @@ impl Elaborator<'_> { use HirExpression::Literal; let from_value_opt = match self.interner.expression(from_expr_id) { - Literal(HirLiteral::Integer(SignedField { field, is_negative: false })) => Some(field), + Literal(HirLiteral::Integer(field)) if !field.is_negative() => { + Some(field.absolute_value()) + } // TODO(https://github.com/noir-lang/noir/issues/6247): // handle negative literals diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 88e4405baf4..5e488172ad5 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -1954,15 +1954,15 @@ fn expr_as_integer( expr_as(interner, arguments, return_type.clone(), location, |expr| match expr { ExprValue::Expression(ExpressionKind::Literal(Literal::Integer(field))) => { Some(Value::Tuple(vec![ - Value::Field(SignedField::positive(field.field)), - Value::Bool(field.is_negative), + Value::Field(SignedField::positive(field.absolute_value())), + Value::Bool(field.is_negative()), ])) } ExprValue::Expression(ExpressionKind::Resolved(id)) => { if let HirExpression::Literal(HirLiteral::Integer(field)) = interner.expression(&id) { Some(Value::Tuple(vec![ - Value::Field(SignedField::positive(field.field)), - Value::Bool(field.is_negative), + Value::Field(SignedField::positive(field.absolute_value())), + Value::Bool(field.is_negative()), ])) } else { None diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/cast.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/cast.rs index 5fad7daa0d9..81dd03906aa 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/cast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/cast.rs @@ -28,7 +28,7 @@ pub(super) fn evaluate_cast_one_step( }}; } let (lhs, lhs_is_negative) = match evaluated_lhs { - Value::Field(value) => (value.field, value.is_negative), + Value::Field(value) => (value.absolute_value(), value.is_negative()), Value::U1(value) => ((value as u128).into(), false), Value::U8(value) => ((value as u128).into(), false), Value::U16(value) => ((value as u128).into(), false), diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 2246d934346..4ca865b6026 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -551,10 +551,10 @@ impl Value { } } Value::Field(value) => { - if value.is_negative { - vec![Token::Minus, Token::Int(value.field)] + if value.is_negative() { + vec![Token::Minus, Token::Int(value.absolute_value())] } else { - vec![Token::Int(value.field)] + vec![Token::Int(value.absolute_value())] } } other => vec![Token::UnquoteMarker(other.into_hir_expression(interner, location)?)], @@ -590,10 +590,10 @@ impl Value { pub(crate) fn to_field_element(&self) -> Option { match self { Self::Field(value) => { - if value.is_negative { + if value.is_negative() { None } else { - Some(value.field) + Some(value.absolute_value()) } } diff --git a/compiler/noirc_frontend/src/monomorphization/debug.rs b/compiler/noirc_frontend/src/monomorphization/debug.rs index 680e7882811..0c05807fc40 100644 --- a/compiler/noirc_frontend/src/monomorphization/debug.rs +++ b/compiler/noirc_frontend/src/monomorphization/debug.rs @@ -76,7 +76,7 @@ impl Monomorphizer<'_> { // instantiate tracked variable for the value type and associate it with // the ID used by the injected instrumentation code let var_type = self.interner.id_type(call.arguments[DEBUG_VALUE_ARG_SLOT]); - let source_var_id = source_var_id.field.to_u128().into(); + let source_var_id = source_var_id.absolute_value().to_u128().into(); // then update the ID used for tracking at runtime let var_id = self.debug_type_tracker.insert_var(source_var_id, &var_type); let interned_var_id = self.intern_var_id(var_id, &call.location); @@ -98,7 +98,7 @@ impl Monomorphizer<'_> { unreachable!("Missing source_var_id in __debug_var_drop call"); }; // update variable ID for tracked drops (ie. when the var goes out of scope) - let source_var_id = source_var_id.field.to_u128().into(); + let source_var_id = source_var_id.absolute_value().to_u128().into(); let var_id = self .debug_type_tracker .get_var_id(source_var_id) @@ -126,7 +126,7 @@ impl Monomorphizer<'_> { unreachable!("Missing source_var_id in __debug_member_assign call"); }; // update variable member assignments - let source_var_id = source_var_id.field.to_u128().into(); + let source_var_id = source_var_id.absolute_value().to_u128().into(); let var_type = self .debug_type_tracker @@ -138,8 +138,8 @@ impl Monomorphizer<'_> { if let Some(HirExpression::Literal(HirLiteral::Integer(fe_i))) = hir_arguments.get(DEBUG_MEMBER_FIELD_INDEX_ARG_SLOT + i) { - let index = fe_i.field.to_i128().unsigned_abs(); - if fe_i.is_negative { + let index = fe_i.absolute_value().to_i128().unsigned_abs(); + if fe_i.is_negative() { // We use negative indices at instrumentation time to indicate // and reference member accesses by name which cannot be // resolved until we have a type. This strategy is also used diff --git a/compiler/noirc_frontend/src/parser/parser/global.rs b/compiler/noirc_frontend/src/parser/parser/global.rs index fe3fe172c45..429f7a9a0e9 100644 --- a/compiler/noirc_frontend/src/parser/parser/global.rs +++ b/compiler/noirc_frontend/src/parser/parser/global.rs @@ -183,7 +183,7 @@ mod tests { panic!("Expected integer literal expression, got {:?}", let_statement.expression.kind); }; - assert!(value.is_negative); - assert_eq!(value.field, FieldElement::from(17u128)); + assert!(value.is_negative()); + assert_eq!(value.absolute_value(), FieldElement::from(17u128)); } } diff --git a/compiler/noirc_frontend/src/signed_field.rs b/compiler/noirc_frontend/src/signed_field.rs index 16814339aad..6018c906479 100644 --- a/compiler/noirc_frontend/src/signed_field.rs +++ b/compiler/noirc_frontend/src/signed_field.rs @@ -2,12 +2,15 @@ use acvm::{AcirField, FieldElement}; #[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)] pub struct SignedField { - pub field: FieldElement, - pub is_negative: bool, + field: FieldElement, + is_negative: bool, } impl SignedField { - pub fn new(field: FieldElement, is_negative: bool) -> Self { + pub fn new(field: FieldElement, mut is_negative: bool) -> Self { + if field.is_zero() { + is_negative = false; + } Self { field, is_negative } } @@ -16,7 +19,7 @@ impl SignedField { } pub fn negative(field: impl Into) -> Self { - Self { field: field.into(), is_negative: true } + Self::new(field.into(), true) } pub fn zero() -> SignedField { @@ -27,6 +30,15 @@ impl SignedField { Self { field: FieldElement::one(), is_negative: false } } + /// Returns the inner FieldElement which will always be positive + pub fn absolute_value(&self) -> FieldElement { + self.field + } + + pub fn is_negative(&self) -> bool { + self.is_negative + } + /// Convert a signed integer to a SignedField, carefully handling /// INT_MIN in the process. Note that to convert an unsigned integer /// you can call `SignedField::positive`. @@ -147,9 +159,8 @@ impl std::ops::Div for SignedField { impl std::ops::Neg for SignedField { type Output = Self; - fn neg(mut self) -> Self::Output { - self.is_negative = !self.is_negative; - self + fn neg(self) -> Self::Output { + Self::new(self.field, !self.is_negative) } } diff --git a/test_programs/compile_success_empty/comptime_negative_zero/Nargo.toml b/test_programs/compile_success_empty/comptime_negative_zero/Nargo.toml new file mode 100644 index 00000000000..648596bcff7 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_negative_zero/Nargo.toml @@ -0,0 +1,6 @@ +[package] +name = "comptime_negative_zero" +type = "bin" +authors = [""] + +[dependencies] diff --git a/test_programs/compile_success_empty/comptime_negative_zero/src/main.nr b/test_programs/compile_success_empty/comptime_negative_zero/src/main.nr new file mode 100644 index 00000000000..e5f98791f35 --- /dev/null +++ b/test_programs/compile_success_empty/comptime_negative_zero/src/main.nr @@ -0,0 +1,9 @@ +fn main() { + comptime { + assert_eq(-0, 0); + assert_eq(-1 + 1, 0); + assert_eq(-1 - -1, 0); + assert_eq(0 * -1, 0); + assert_eq(0 / -1, 0); + } +} diff --git a/tooling/ast_fuzzer/src/program/expr.rs b/tooling/ast_fuzzer/src/program/expr.rs index 12a298d38f1..c5150d631b1 100644 --- a/tooling/ast_fuzzer/src/program/expr.rs +++ b/tooling/ast_fuzzer/src/program/expr.rs @@ -24,10 +24,7 @@ pub fn gen_literal(u: &mut Unstructured, typ: &Type) -> arbitrary::Result Expression::Literal(Literal::Unit), Type::Bool => Expression::Literal(Literal::Bool(bool::arbitrary(u)?)), Type::Field => { - let field = SignedField { - field: Field::from(u128::arbitrary(u)?), - is_negative: bool::arbitrary(u)?, - }; + let field = SignedField::new(Field::from(u128::arbitrary(u)?), bool::arbitrary(u)?); Expression::Literal(Literal::Integer(field, Type::Field, Location::dummy())) } Type::Integer(signedness, integer_bit_size) => { @@ -64,7 +61,7 @@ pub fn gen_literal(u: &mut Unstructured, typ: &Type) -> arbitrary::Result, { Expression::Literal(Literal::Integer( - SignedField { field: FieldElement::from(value), is_negative }, + SignedField::new(value.into(), is_negative), typ, Location::dummy(), )) diff --git a/tooling/lsp/src/requests/hover/from_visitor.rs b/tooling/lsp/src/requests/hover/from_visitor.rs index 7d3f0ccbff7..39d5dba2f94 100644 --- a/tooling/lsp/src/requests/hover/from_visitor.rs +++ b/tooling/lsp/src/requests/hover/from_visitor.rs @@ -71,12 +71,12 @@ impl Visitor for HoverFinder<'_> { } fn format_integer(typ: &Type, value: SignedField) -> String { - let value_base_10 = value.field.to_string(); + let value_base_10 = value.absolute_value().to_string(); // For simplicity we parse the value as a BigInt to convert it to hex // because `FieldElement::to_hex` will include many leading zeros. let value_big_int = BigInt::from_str(&value_base_10).unwrap(); - let negative = if value.is_negative { "-" } else { "" }; + let negative = if value.is_negative() { "-" } else { "" }; format!( " {typ}\n---\nvalue of literal: `{negative}{value_base_10} ({negative}0x{value_big_int:02x})`" diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap new file mode 100644 index 00000000000..e67697edcbd --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap @@ -0,0 +1,7 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + () +} diff --git a/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__force_brillig_false_inliner_0.snap b/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__force_brillig_false_inliner_0.snap new file mode 100644 index 00000000000..7de940827c8 --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/compile_success_empty/comptime_negative_zero/execute__tests__force_brillig_false_inliner_0.snap @@ -0,0 +1,26 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: artifact +--- +{ + "noir_version": "[noir_version]", + "hash": "[hash]", + "abi": { + "parameters": [], + "return_type": null, + "error_types": {} + }, + "bytecode": [ + "func 0", + "current witness index : _0", + "private parameters indices : []", + "public parameters indices : []", + "return value indices : []" + ], + "debug_symbols": "XY5BCsQwCEXv4rqLWfcqw1BsaosgJtikMITefWyYQOlK/3/6tcJCc9km1jXuML4rzMYivE0SA2aO6m49B+hyykbkFty4byU00gyjFpEBDpTShvaE2mpGc/oagHTx6oErC13d+XGBge158UBjnIX+ci0abjR/Uyf942Qx0FKMrqTGPPsH", + "file_map": {}, + "names": [ + "main" + ], + "brillig_names": [] +} diff --git a/tooling/nargo_cli/tests/snapshots/expand/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap b/tooling/nargo_cli/tests/snapshots/expand/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap new file mode 100644 index 00000000000..e67697edcbd --- /dev/null +++ b/tooling/nargo_cli/tests/snapshots/expand/compile_success_empty/comptime_negative_zero/execute__tests__expanded.snap @@ -0,0 +1,7 @@ +--- +source: tooling/nargo_cli/tests/execute.rs +expression: expanded_code +--- +fn main() { + () +}