Fix eta-expansion in evaluator #2782
Conversation
|
I can confirm that this also fixes the original non-minimized version of the problem. @christiaanb do you still want to keep this as a draft PR? |
|
I'll add the test case and make it ready for review. |
tests/shouldwork/Issues/T2781.hs
Outdated
| fullMeshSwCcTest sysClk = spiDone | ||
| where | ||
| (spiDone | ||
| , ugnsStable -- :: Vec 1 (Signal System Bool) -- this signature causes "Evaluator.instantiate: Not a tylambda: Lambda" |
There was a problem hiding this comment.
Better remove this comment to avoid confusion.
Originally the bug was triggered by the addition of this type signature, hence the comment.
But this curious property was lost somewhere during minimization of the test case, in the current form it always triggered the bug.
There was a problem hiding this comment.
I'll remove the comment.
|
|
||
| go _ supply (Left tv) = (supply, Right tv) | ||
| go names supply (Right ty) = | ||
| let ((supply1, _), n) = mkUniqSystemId (supply, names) ("x", ty) |
There was a problem hiding this comment.
Is it safe to just ignore the updated InscopeSet here?
There was a problem hiding this comment.
No, it's not. The new binders should not shadow each other. I'll fix this.
For some eta-reduced 'e', we used to bogusly eta-expand to: \x.(\y. e y) x We now correctly expand to: \x.\y.(e x) y Fixes #2781
For some eta-reduced 'e', we used to bogusly eta-expand to:
We now correctly expand to:
Fixes #2781
Still TODO: