-
Notifications
You must be signed in to change notification settings - Fork 0
feat: print bigint instead of felts #9
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
base: px/bigint-ssa
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the printing system to use BigInt instead of field elements (FieldElement) across the codebase. The primary purpose is to standardize the internal representation of numeric values during the printing/debugging process, allowing for better handling of large numeric values and improved consistency.
Key changes include:
- Transitioning
PrintableValueand related APIs from genericF: AcirFieldto concreteBigInttypes - Updating all printing, debugging, and formatting functions to work with
BigIntrepresentations - Converting field elements to
BigIntat conversion boundaries while maintaining API compatibility
Reviewed Changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| tooling/ssa_fuzzer/src/builder.rs | Updates FuzzerBuilder to accept BigInt instead of FieldElement for constants |
| tooling/ssa_fuzzer/fuzzer/src/fuzz_lib/base_context.rs | Converts field elements to BigInt using NumericValue conversion utilities |
| tooling/profiler/src/opcode_formatter.rs | Adds handling for phantom BrilligOpcode variant |
| tooling/profiler/src/cli/execution_flamegraph_cmd.rs | Switches from Bn254BlackBoxSolver to M31BlackBoxSolver |
| tooling/noirc_artifacts/src/debug_vars.rs | Removes generic type parameter and converts to BigInt-based implementation |
| tooling/noirc_abi/src/lib.rs | Updates ABI decoding functions to convert FieldElement to BigInt before processing |
| tooling/noir_executor/src/main.rs | Updates test code with new SSA example and printing functions |
| tooling/nargo/src/foreign_calls/print.rs | Converts foreign call parameters from FieldElement to BigInt for printing |
| tooling/nargo/src/foreign_calls/mocker.rs | Updates string parsing to use BigInt conversion |
| tooling/nargo/src/errors.rs | Updates error message formatting to work with new BigInt-based display functions |
| tooling/debugger/src/foreign_calls.rs | Removes generic type parameters and converts field values to BigInt for debugging |
| tooling/debugger/src/context.rs | Updates debug context and stack frame handling to use BigInt values |
| compiler/noirc_printable_type/src/lib.rs | Core refactoring to replace generic F: AcirField with concrete BigInt usage |
| compiler/noirc_evaluator/src/ssa/opt/assert_constant.rs | Updates static assertion evaluation to use BigInt conversion |
| compiler/noirc_evaluator/src/ssa/interpreter/value.rs | Makes field-to-BigInt conversion public and adds utility functions |
| compiler/noirc_evaluator/src/ssa/interpreter/intrinsics.rs | Adds value_to_bigints function for BigInt-based printing |
| acvm-repo/brillig/src/foreign_call.rs | Updates ForeignCallParam to work with Clone trait instead of AcirField |
| @@ -691,7 +691,7 @@ impl<W: Write> Interpreter<'_, W> { | |||
| // We'll let each parser take as many fields as they need. | |||
| let meta_idx = args.len() - 1 - num_values; | |||
| let input_as_fields = | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
better naming of variable,
| } | ||
| "; | ||
| let ssa = r#" | ||
| acir(inline) fn main f0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
irrelevant spaces
| use num_bigint::BigInt; | ||
|
|
||
| #[test] | ||
| fn test_resolve_foreign_calls_stepping_into_brillig() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment out the test, as brilling vm isn't handled now
No description provided.