Erroneous code transformation at partial applications (MPR#7531)#1162
Erroneous code transformation at partial applications (MPR#7531)#1162mshinwell merged 2 commits intoocaml:trunkfrom
Conversation
|
Addendum: the output of the test case without this patch is: |
|
This looks correct to me |
|
Looks good! (For a few minutes I'd convinced myself that the subsequent code for handling over-applications had the same issue, but I think I was mistaken and it's correct) |
|
I also had such a misapprehension, but after looking through the code with @lpw25 I also think the overapplication code is now correct (I fixed that a while back, IIRC). |
|
Yes, it looks like the issue I was worrying about was fixed by #967. |
|
This patch actually had a slightly obscure bug, which has been fixed by the commit I just pushed ( |
|
@xavierleroy Does this look correct to you? |
|
@mshinwell how important is it to have it in 4.05 ? |
|
This seems important to me. FWIW, we're running with this patch (on 4.04) at Jane Street now, and I don't believe there have been any problems. |
|
I've reviewed and it seems low-risk enough, let's merge and cherry-pick to 4.05. |
|
Merged and cherry-picked to 4.05. |
5 participants
Effects may be incorrectly pushed under lambdas that are generated at partial application sites when using the Closure path of the native code compiler. The test case here is a slightly more elaborate version of MPR#7531; it checks for non-duplication of an effect as well as ensuring that the function component of an application gets evaluated after the argument.
This should probably go on 4.05 as well.