fix(codegen): look at struct args (generics) when generating structs#9079
fix(codegen): look at struct args (generics) when generating structs#9079
Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Compilation Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 02ef0bd | Previous: de2a1bd | Ratio |
|---|---|---|---|
rollup-root |
1.7 s |
1.37 s |
1.24 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Test Suite Duration'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 3052df2 | Previous: de2a1bd | Ratio |
|---|---|---|---|
test_report_zkpassport_noir_rsa_ |
2 s |
1 s |
2 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
|
Even though this seems correct it ends up generating duplicated structs in some cases. I'm not going to work on this for now so I'll close this. |
Description
Problem
Resolves #9044
Summary
The fix in the previous PR was wrong. Nested structs were generated. It's just that at one point generic struct field types get replaced with bindings, and the bindings (possibly structs) end up in an
argsfield. So what was missing is taking a look atargsto find more structs to generate.Additional Context
I added
rm -rf ./test/test_lib/exportto the test script because otherwise when you made changed to the Noir code some old code might remain there and error, and it can be a bit misleading.Documentation
Check one:
PR Checklist
cargo fmton default settings.