diff --git a/acvm-repo/acir_field/src/field_element.rs b/acvm-repo/acir_field/src/field_element.rs index 0bba70ec2fc..57542248ec5 100644 --- a/acvm-repo/acir_field/src/field_element.rs +++ b/acvm-repo/acir_field/src/field_element.rs @@ -143,6 +143,29 @@ impl FieldElement { let fr = F::from_str(input).ok()?; Some(FieldElement(fr)) } + + /// Assume this field element holds a signed integer of the given `bit_size` and format + /// it as a string. The range of valid values for this field element is `0..2^bit_size` + /// with `0..2^(bit_size - 1)` representing positive values and `2^(bit_size - 1)..2^bit_size` + /// representing negative values (as is commonly done for signed integers). + /// `2^(bit_size - 1)` is the lowest negative value, so for example if bit_size is 8 then + /// `0..127` map to `0..127`, `128` maps to `-128`, `129` maps to `-127` and `255` maps to `-1`. + /// If `self` falls outside of the valid range it's formatted as-is. + pub fn to_string_as_signed_integer(self, bit_size: u32) -> String { + assert!(bit_size <= 128); + if self.num_bits() > bit_size { + return self.to_string(); + } + + // Compute the maximum value that is considered a positive value + let max = if bit_size == 128 { i128::MAX as u128 } else { (1 << (bit_size - 1)) - 1 }; + if self.to_u128() > max { + let f = FieldElement::from(2u32).pow(&bit_size.into()) - self; + format!("-{f}") + } else { + self.to_string() + } + } } impl AcirField for FieldElement { @@ -255,6 +278,21 @@ impl AcirField for FieldElement { bytes.reverse(); hex::encode(bytes) } + + fn to_short_hex(self) -> String { + if self.is_zero() { + return "0x00".to_owned(); + } + let mut hex = "0x".to_string(); + let self_to_hex = self.to_hex(); + let trimmed_field = self_to_hex.trim_start_matches('0'); + if trimmed_field.len() % 2 != 0 { + hex.push('0'); + } + hex.push_str(trimmed_field); + hex + } + fn from_hex(hex_str: &str) -> Option> { let value = hex_str.strip_prefix("0x").unwrap_or(hex_str); // Values of odd length require an additional "0" prefix @@ -527,4 +565,48 @@ mod tests { prop_assert_eq!(fe_1, fe_2, "equivalent hex strings with opposite parity deserialized to different values"); } } + + #[test] + fn test_to_hex() { + type F = FieldElement; + assert_eq!( + F::zero().to_hex(), + "0000000000000000000000000000000000000000000000000000000000000000" + ); + assert_eq!( + F::one().to_hex(), + "0000000000000000000000000000000000000000000000000000000000000001" + ); + assert_eq!( + F::from(0x123_u128).to_hex(), + "0000000000000000000000000000000000000000000000000000000000000123" + ); + assert_eq!( + F::from(0x1234_u128).to_hex(), + "0000000000000000000000000000000000000000000000000000000000001234" + ); + } + + #[test] + fn test_to_short_hex() { + type F = FieldElement; + assert_eq!(F::zero().to_short_hex(), "0x00"); + assert_eq!(F::one().to_short_hex(), "0x01"); + assert_eq!(F::from(0x123_u128).to_short_hex(), "0x0123"); + assert_eq!(F::from(0x1234_u128).to_short_hex(), "0x1234"); + } + + #[test] + fn to_string_as_signed_integer() { + type F = FieldElement; + assert_eq!(F::zero().to_string_as_signed_integer(8), "0"); + assert_eq!(F::one().to_string_as_signed_integer(8), "1"); + assert_eq!(F::from(127_u128).to_string_as_signed_integer(8), "127"); + assert_eq!(F::from(128_u128).to_string_as_signed_integer(8), "-128"); + assert_eq!(F::from(129_u128).to_string_as_signed_integer(8), "-127"); + assert_eq!(F::from(255_u128).to_string_as_signed_integer(8), "-1"); + assert_eq!(F::from(32767_u128).to_string_as_signed_integer(16), "32767"); + assert_eq!(F::from(32768_u128).to_string_as_signed_integer(16), "-32768"); + assert_eq!(F::from(65535_u128).to_string_as_signed_integer(16), "-1"); + } } diff --git a/acvm-repo/acir_field/src/generic_ark.rs b/acvm-repo/acir_field/src/generic_ark.rs index 1473a37a23b..4e2aaec0263 100644 --- a/acvm-repo/acir_field/src/generic_ark.rs +++ b/acvm-repo/acir_field/src/generic_ark.rs @@ -68,8 +68,16 @@ pub trait AcirField: /// Before using this FieldElement, please ensure that this behavior is necessary fn inverse(&self) -> Self; + /// Returns the vale of this field as a hex string without the `0x` prefix. + /// The returned string will have a length equal to the maximum number of hex + /// digits needed to represent the maximum value of this field. fn to_hex(self) -> String; + /// Returns the value of this field as a hex string with leading zeroes removed, + /// prepended with `0x`. + /// A singular '0' will be prepended as well if the trimmed string has an odd length. + fn to_short_hex(self) -> String; + fn from_hex(hex_str: &str) -> Option; fn to_be_bytes(self) -> Vec; @@ -184,6 +192,10 @@ macro_rules! field_wrapper { self.0.to_hex() } + fn to_short_hex(self) -> String { + self.0.to_short_hex() + } + fn from_hex(hex_str: &str) -> Option { $field::from_hex(hex_str).map(Self) } diff --git a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/cast.rs b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/cast.rs index deb6250a21e..426aaea77dc 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/cast.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/dfg/simplify/cast.rs @@ -101,7 +101,7 @@ mod tests { assert_ssa_snapshot!(ssa, @r" brillig(inline) predicate_pure fn main f0 { b0(): - return i8 135 + return i8 -121 } "); } diff --git a/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 5573a0e576d..706cc1c3aa2 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -1,7 +1,7 @@ //! This file is for pretty-printing the SSA IR in a human-readable form for debugging. use std::fmt::{Display, Formatter, Result}; -use acvm::acir::AcirField; +use acvm::{FieldElement, acir::AcirField}; use fm::codespan_files; use im::Vector; use iter_extended::vecmap; @@ -48,7 +48,7 @@ impl Display for Printer<'_> { for (id, global_value) in globals_dfg.values_iter() { match global_value { Value::NumericConstant { constant, typ } => { - writeln!(f, "g{} = {typ} {constant}", id.to_u32())?; + writeln!(f, "g{} = {typ} {}", id.to_u32(), number(*constant, typ))?; } Value::Instruction { instruction, .. } => { display_instruction(&globals_dfg, *instruction, true, self.fm, f)?; @@ -122,7 +122,7 @@ fn display_block( fn value(dfg: &DataFlowGraph, id: ValueId) -> String { match &dfg[id] { Value::NumericConstant { constant, typ } => { - format!("{typ} {constant}") + format!("{typ} {}", number(*constant, typ)) } Value::Function(id) => id.to_string(), Value::Intrinsic(intrinsic) => intrinsic.to_string(), @@ -140,6 +140,18 @@ fn value(dfg: &DataFlowGraph, id: ValueId) -> String { } } +/// Formats the given number assuming it has the given type. +/// Unsigned types and field element types will be formatter as-is, +/// while signed types will be formatted as positive or negative numbers +/// depending on where the number falls in the range given by the type's bit size. +fn number(number: FieldElement, typ: &NumericType) -> String { + if let NumericType::Signed { bit_size } = typ { + number.to_string_as_signed_integer(*bit_size) + } else { + number.to_string() + } +} + /// Display each value along with its type. E.g. `v0: Field, v1: u64, v2: u1` fn value_list_with_types(dfg: &DataFlowGraph, values: &[ValueId]) -> String { vecmap(values, |id| { diff --git a/compiler/noirc_evaluator/src/ssa/parser/tests.rs b/compiler/noirc_evaluator/src/ssa/parser/tests.rs index f6ee84a9189..31070be4bd6 100644 --- a/compiler/noirc_evaluator/src/ssa/parser/tests.rs +++ b/compiler/noirc_evaluator/src/ssa/parser/tests.rs @@ -53,6 +53,17 @@ fn test_return_integer() { } } +#[test] +fn test_return_negative_integer() { + let src = " + acir(inline) fn main f0 { + b0(): + return i8 -53 + } + "; + assert_ssa_roundtrip(src); +} + #[test] fn test_make_array() { let src = " diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 8d746d773ae..76142a0c22a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -1,8 +1,8 @@ use std::fmt::Display; +use acvm::AcirField; use iter_extended::vecmap; use noirc_errors::Location; -use noirc_printable_type::format_field_string; use crate::{ Type, @@ -385,7 +385,7 @@ impl Display for ValuePrinter<'_, '_> { } Value::Field(value) => { // write!(f, "{value}") // This would display the Field as a number, but it doesn't match the runtime. - write!(f, "{}", format_field_string(value.to_field_element())) + write!(f, "{}", value.to_field_element().to_short_hex()) } Value::I8(value) => write!(f, "{value}"), Value::I16(value) => write!(f, "{value}"), diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 93f1c42c904..1788b85f886 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -99,7 +99,7 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op let mut output = String::new(); match (value, typ) { (PrintableValue::Field(f), PrintableType::Field) => { - output.push_str(&format_field_string(*f)); + output.push_str(&f.to_short_hex()); } (PrintableValue::Field(f), PrintableType::UnsignedInteger { width }) => { // Retain the lower 'width' bits @@ -277,21 +277,6 @@ fn write_template_replacing_interpolations( write!(fmt, "{}", &template[last_index..]) } -/// This trims any leading zeroes. -/// A singular '0' will be prepended as well if the trimmed string has an odd length. -/// A hex string's length needs to be even to decode into bytes, as two digits correspond to -/// one byte. -pub fn format_field_string(field: F) -> String { - if field.is_zero() { - return "0x00".to_owned(); - } - let mut trimmed_field = field.to_hex().trim_start_matches('0').to_owned(); - if trimmed_field.len() % 2 != 0 { - trimmed_field = "0".to_owned() + &trimmed_field; - } - "0x".to_owned() + &trimmed_field -} - /// Assumes that `field_iterator` contains enough field elements in order to decode the [PrintableType]. pub fn decode_printable_value( field_iterator: &mut impl Iterator, diff --git a/tooling/artifact_cli/src/execution.rs b/tooling/artifact_cli/src/execution.rs index 5d0361b09d7..8967dcb2ba2 100644 --- a/tooling/artifact_cli/src/execution.rs +++ b/tooling/artifact_cli/src/execution.rs @@ -6,7 +6,6 @@ use nargo::{NargoError, foreign_calls::ForeignCallExecutor}; use noirc_abi::{AbiType, Sign, input_parser::InputValue}; use noirc_artifacts::debug::DebugArtifact; use noirc_driver::CompiledProgram; -use noirc_printable_type::format_field_string; use crate::{ errors::CliError, @@ -161,7 +160,7 @@ pub fn input_value_to_string(input_value: &InputValue, abi_type: &AbiType) -> St fn append_input_value_to_string(input_value: &InputValue, abi_type: &AbiType, string: &mut String) { match (abi_type, input_value) { (AbiType::Field, InputValue::Field(field_element)) => { - string.push_str(&format_field_string(*field_element)); + string.push_str(&field_element.to_short_hex()); } (AbiType::Array { length: _, typ }, InputValue::Vec(input_values)) => { string.push('['); @@ -178,16 +177,7 @@ fn append_input_value_to_string(input_value: &InputValue, abi_type: &AbiType, st string.push_str(&f.to_string()); } Sign::Signed => { - let bit_size = *bit_size; - let max = - if bit_size == 128 { i128::MAX as u128 } else { (1 << (bit_size - 1)) - 1 }; - if f.num_bits() > 128 || f.to_u128() > max { - string.push('-'); - let f = FieldElement::from(2u32).pow(&bit_size.into()) - *f; - string.push_str(&f.to_string()); - } else { - string.push_str(&f.to_string()); - } + string.push_str(&f.to_string_as_signed_integer(*bit_size)); } }, (AbiType::Boolean, InputValue::Field(field_element)) => { diff --git a/tooling/ast_fuzzer/src/program/tests.rs b/tooling/ast_fuzzer/src/program/tests.rs index bfe00213bd0..5ea15a529ee 100644 --- a/tooling/ast_fuzzer/src/program/tests.rs +++ b/tooling/ast_fuzzer/src/program/tests.rs @@ -86,9 +86,9 @@ fn test_modulo_of_negative_literals_in_range() { assert_ssa_snapshot!(ssa, @r" acir(inline) fn main f0 { b0(): - jmp b1(i64 18446744073709551612) + jmp b1(i64 -4) b1(v0: i64): - v3 = lt v0, i64 18446744073709551615 + v3 = lt v0, i64 -1 jmpif v3 then: b2, else: b3 b2(): jmp b3() diff --git a/tooling/nargo_cli/src/cli/interpret_cmd.rs b/tooling/nargo_cli/src/cli/interpret_cmd.rs index 0fafcea5c97..75ac67cf8ca 100644 --- a/tooling/nargo_cli/src/cli/interpret_cmd.rs +++ b/tooling/nargo_cli/src/cli/interpret_cmd.rs @@ -129,6 +129,8 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl }); let interpreter_options = InterpreterOptions { trace: args.trace }; + let file_manager = + if args.compile_options.no_ssa_locations { None } else { Some(&file_manager) }; print_and_interpret_ssa( ssa_options, @@ -138,7 +140,7 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl &ssa_args, &ssa_return, interpreter_options, - &file_manager, + file_manager, )?; // Run SSA passes in the pipeline and interpret the ones we are interested in. @@ -161,7 +163,7 @@ pub(crate) fn run(args: InterpretCommand, workspace: Workspace) -> Result<(), Cl &ssa_args, &ssa_return, interpreter_options, - &file_manager, + file_manager, )?; } } @@ -223,7 +225,7 @@ fn msg_matches(patterns: &[String], msg: &str) -> bool { patterns.iter().any(|p| msg.contains(&p.to_lowercase())) } -fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: &FileManager) { +fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: Option<&FileManager>) { let print = match options.ssa_logging { SsaLogging::All => true, SsaLogging::None => false, @@ -231,7 +233,7 @@ fn print_ssa(options: &SsaEvaluatorOptions, ssa: &mut Ssa, msg: &str, fm: &FileM }; if print { ssa.normalize_ids(); - println!("After {msg}:\n{}", ssa.print_with(Some(fm))); + println!("After {msg}:\n{}", ssa.print_with(fm)); } } @@ -280,7 +282,7 @@ fn print_and_interpret_ssa( args: &[Value], return_value: &Option>, interpreter_options: InterpreterOptions, - fm: &FileManager, + fm: Option<&FileManager>, ) -> Result<(), CliError> { print_ssa(options, ssa, msg, fm); interpret_ssa(passes_to_interpret, ssa, msg, args, return_value, interpreter_options)