You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The basicOverlaps implementation for Data.Vector.Mutable is:
basicOverlaps (MVector i m arr1) (MVector j n arr2)
= sameMutableArray arr1 arr2
&& (between i j (j+n) || between j i (i+m))
where
between x y z = x >= y && x < z
This will return True for two vectors where two vectors share the same mutable array and offset, but one has a zero length and the other has a non-zero length. I cannot find a precise definition of what it means for possibly-empty vectors to overlap, so it's hard to say for sure this is wrong, but it certainly disagrees with my understanding of what overlaps ought to mean.
I suggest this implementation instead:
basicOverlaps (MVector i m arr1) (MVector j n arr2)
= sameMutableArray arr1 arr2 && i < j + n && j < i + m
That will still report an overlap for an empty vector with an offset in the middle of a containing non-empty vector, though. It's even less clear what the right answer is there. So I guess what we really need is a precise specification first, and then we can write an implementation that matches it.
The text was updated successfully, but these errors were encountered:
cdsmith
changed the title
overlaps for Data.Vector.Mutable appears to be incorrect
overlaps for Data.Vector.Mutable behaves oddly for empty vectors
Jul 20, 2022
This will return True for two vectors where two vectors share the same mutable array and offset, but one has a zero length and the other has a non-zero length. I cannot find a precise definition of what it means for possibly-empty vectors to overlap, so it's hard to say for sure this is wrong, but it certainly disagrees with my understanding of what overlaps ought to mean.
I don't see any problem with this. Afaik, the only time where overlapping is relevant (apart from wanting to know if modifying one vector may change the other, which clearly isn't the case if one of them is empty) is when deciding wether to use copy or move (since the vectors may not overlap for copy), but if the source or target vector is empty, that doesn't matter.
Intuitively, I'd expect two vectors to always be non-overlapping when at least one of them is empty. The easiest way to do that is probably to simply check if either of the lengths is 0.
Makes sense. So if the definition of overlapping is that there exists a mutation to one that will also modify the other, then the existing implementation is wrong. So is my proposed replacement, though it's wrong in fewer cases. I agree, adding an explicit check if either vector is empty would be an easy way to fix it.
The
basicOverlaps
implementation for Data.Vector.Mutable is:This will return
True
for two vectors where two vectors share the same mutable array and offset, but one has a zero length and the other has a non-zero length. I cannot find a precise definition of what it means for possibly-empty vectors to overlap, so it's hard to say for sure this is wrong, but it certainly disagrees with my understanding of whatoverlaps
ought to mean.I suggest this implementation instead:
That will still report an overlap for an empty vector with an offset in the middle of a containing non-empty vector, though. It's even less clear what the right answer is there. So I guess what we really need is a precise specification first, and then we can write an implementation that matches it.
The text was updated successfully, but these errors were encountered: