fix: unify macro call type with actual type in comptime blocks#11411
fix: unify macro call type with actual type in comptime blocks#11411
Conversation
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: ecabab9 | Previous: 8d91700 | Ratio |
|---|---|---|---|
test_report_zkpassport_noir-ecdsa_ |
3 s |
2 s |
1.50 |
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 'Brillig Execution Time'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: ecabab9 | Previous: 8d91700 | Ratio |
|---|---|---|---|
rollup-block-root |
0.004 s |
0.003 s |
1.33 |
This comment was automatically generated by workflow using github-action-benchmark.
CC: @TomAFrench
jfecher
left a comment
There was a problem hiding this comment.
Somewhat of a complex interaction between comptime & the type system but the old behavior with not unifying was also unexpected and this reigns that in.
Description
Problem
Resolves #11409
Summary
This is a follow-up to #6105
#6105 is good but because we don't unify types, some type variables end up unbound. That's why #11409 was failing.
The solution here is:
This allows a variable to change types across macro loop iterations.
For this I also had to relax the checking of unbound generics when a function context is popped, only in comptime code. The reason is that in the new test here, in
foo.len()we end up withfoowithout a knownN, but it's fine because it's thatNthat we want to infer later on. In this case, ifNends up being used when interpreting the block and it's not bound, we'll get an error anyway.Additional Context
User Documentation
Check one:
PR Checklist
cargo fmton default settings.