@daanx, I understand if you would like to redo this yourself to make it cleaner, but this should illustrate the issue / solution.
Specifically @evv-index does not include an index for duplicate labels. This is fine for most effect expressions, since the effect row is incrementally adjusted. However, for the general open case where you use a vector of indices and there are multiple labels of the same kind, we get invalid offsets. This would happen with or without the code in Simplify.hs which turns them into constant offsets.
In general I think the types for effect rows need to represent duplicate label indices more explicitly (when extracting the ordered effect), so I defaulted to including the count of duplicate labels (pseudo index) in extractOrderedEffect. However, the indices aren't really important till after type checking, so there are a lot of places where I needed to discard the indices. I don't know if you would rather do something to represent this invariant more explicitly in the type, or if you would rather create another function for when you don't care about indices, since there are only a few places that do.
In particular the issue is illustrated by the following short code snippet from the core output. (from the example in #511)
(std/core/hnd/@open1(
(std/core/vector/unvlist(
// Here is the problem both of these indices are the same!!!!
(std/core/types/Cons((std/core/types/@make-ssize_t(0)),
(std/core/types/Cons((std/core/types/@make-ssize_t(0)),
std/core/types/Nil)))))
Fixes #511
@daanx, I understand if you would like to redo this yourself to make it cleaner, but this should illustrate the issue / solution.
Specifically
@evv-indexdoes not include an index for duplicate labels. This is fine for most effect expressions, since the effect row is incrementally adjusted. However, for the generalopencase where you use a vector of indices and there are multiple labels of the same kind, we get invalid offsets. This would happen with or without the code inSimplify.hswhich turns them into constant offsets.In general I think the types for effect rows need to represent duplicate label indices more explicitly (when extracting the ordered effect), so I defaulted to including the count of duplicate labels (pseudo index) in
extractOrderedEffect. However, the indices aren't really important till after type checking, so there are a lot of places where I needed to discard the indices. I don't know if you would rather do something to represent this invariant more explicitly in the type, or if you would rather create another function for when you don't care about indices, since there are only a few places that do.In particular the issue is illustrated by the following short code snippet from the core output. (from the example in #511)