chore: add compile-time assertions on generic arguments of stdlib functions #6981
Conversation
…tatic_assert locations for field/mod.nr
michaeljklein
changed the title
Compilation Report
|
Execution Report
|
Compilation Memory Report
|
Execution Memory Report
|
…cks for numeric generics in field/mod.nr, fixed printing in test_transform_program_is_idempotent
…c_assert, ensure dynamic vars rejected by static_assert
…raint(input_expr_witness, bit_size)?;' for to_radix
Changes to number of Brillig opcodes executed
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
|
@michaeljklein Is this waiting on anything other than formatting? |
|
@TomAFrench Nope, should be good to go |
|
This has still got formatting issues. There's also a regression in u128 which I'm not sure on the cause of but these assertions shouldn't affect the final artifact. |
|
@TomAFrench I was able to recreate the difference, but haven't been able to figure out why it's happening: $ ~/.nargo/bin/nargo info --silence-warnings --profile-execution --json --inliner-aggressiveness 0
{"programs":[{"package_name":"u128","functions":[{"name":"main","opcodes":0}],"unconstrained_functions":[{"name":"main","opcodes":44332}]}]}
$ cargo run info --silence-warnings --profile-execution --json --inliner-aggressiveness 0
{"programs":[{"package_name":"u128","functions":[{"name":"main","opcodes":0}],"unconstrained_functions":[{"name":"main","opcodes":44453}]}]}( When I run $ cargo run compile --show-ssa --inliner-aggressiveness 0
$ ~/.nargo/bin/nargo compile --show-ssa --inliner-aggressiveness 0(Also when compiling with So I'm not sure why there's a ~0.2% difference, but it seems likely to be unrelated changes in master and/or a measurement error because 1) |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 55fcc95 | Previous: 8804f0a | Ratio |
|---|---|---|---|
regression_4709 |
0.002 s |
0.001 s |
2 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
| .unwrap_or_else(|err| { | ||
| let error_string: String = | ||
| err.iter().fold(String::new(), |mut output, diagnostic| { | ||
| let _ = write!( | ||
| output, | ||
| "{}\n---\n", | ||
| diagnostic_to_string(diagnostic, &file_manager) | ||
| ); | ||
| output | ||
| }); | ||
| panic!("Failed to compile:\n\n{}", error_string) | ||
| }); |
There was a problem hiding this comment.
Patch so that test_transform_program_is_idempotent shows failing test_program's in a readable way.
I've simplified it a bit.
|
We took a look into the regression and the result is due to the changes in weights causing things that would otherwise be inlined to not be inlined. This will be resolved by #7148 |
| static_assert( | ||
| N <= modulus_le_bytes().len(), | ||
| "N must be less than or equal to modulus_le_bytes().len()", | ||
| ); |
There was a problem hiding this comment.
Can't comment on it directly but one requirement of to_le_radix is that the radix is a power of 2.
There was a problem hiding this comment.
Also greater than 1 and lte 256
There was a problem hiding this comment.
Ok, I've added those three checks + basic tests that they're triggered. (wip)
…re tests, wip debugging test failures, cleanup diagnostic printing
…e debug 'static_assert's and only check radix when not is_unconstrained, simplify to nothing when invalid radix, check radix range when generating ACIR, add brillig-specific to_radix tests, add handling for unconstrained mode for acir to_radix tests
Changes to Brillig bytecode sizes
🧾 Summary (10% most significant diffs)
Full diff report 👇
|
Description
Problem*
Resolves #6962
Summary*
Additional Context
Documentation*
Check one:
PR Checklist*
cargo fmton default settings.