Fix a gadt typing bug introduced in the merge#32
Fix a gadt typing bug introduced in the merge#32mshinwell merged 1 commit intomshinwell:5.2-runtime-wip-mainfrom
Conversation
|
@ccasin Github has noticed that I merged this manually, but please still review. |
ncik-roberts
left a comment
There was a problem hiding this comment.
This looks right to me. (I can't literally click "Approve", maybe because this is in a fork.) I might have suggested not using env at all in this function, replacing all use sites with get_env uenv to avoid the possibility of such bugs, but this has downsides itself (repetitive, drifts from upstream).
Yes, I was on the fence about doing that but decided not to for merge conflict reasons. |
… in the merge) (oxcaml#2975) Fix layout equation bug (cherry picked from commit 61559e5) Co-authored-by: Chris Casinghino <ccasinghino@janestreet.com>
This is a sad bug.
ctypehas always used a ref to old the environment, but now that's obscured a bit behind the "unification environment" such that we missed we were holding on to a stale value after the environment had been updated inadd_gadt_equation.I haven't added a test because I'm not sure of the current state of the test suite. Hopefully there is already a test for this that we accidentally promoted. If not we can add one.