Skip to content

Conversation

@Bisht13
Copy link
Collaborator

@Bisht13 Bisht13 commented Jul 24, 2025

No description provided.

@Bisht13 Bisht13 requested review from ashpect and Copilot July 24, 2025 15:14
Copy link

Copilot AI left a 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 PrintableValue and related APIs from generic F: AcirField to concrete BigInt types
  • Updating all printing, debugging, and formatting functions to work with BigInt representations
  • Converting field elements to BigInt at 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 =
Copy link
Collaborator

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 {
Copy link
Collaborator

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() {
Copy link
Collaborator

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

@shreyas-londhe shreyas-londhe changed the base branch from px/ssa-exec to px/bigint-ssa July 28, 2025 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants