invenia / Intervals.jl

Non-iterable ranges
MIT License
35 stars 18 forks source link

stackoverflow from recursive call in `find_intersections` #200

Open ericphanson opened 2 years ago

ericphanson commented 2 years ago

It is very easy to cause a stackoverflow from:

https://github.com/invenia/Intervals.jl/blob/a785b56ce5e0091928fbb152bd2aa4834ada00d4/src/interval_sets.jl#L480

In my case I had TimeSpans because I forgot to convert them to intervals first.

I believe the intention of this code is to collect any iterators so we get vectors to hit the next method, https://github.com/invenia/Intervals.jl/blob/a785b56ce5e0091928fbb152bd2aa4834ada00d4/src/interval_sets.jl#L481. (I'm not sure why 1-arg vcat would be used over collect but I don't think that's the problem here).

However:

This is extra annoying on MacOS because of the longstanding Julia issue where stackoverflows don't error properly on MacOS.

The easiest fix for the stackoverflow would be to remove the first method, however that would break any code that relies on find_intersections doing this collecting for them. I think a fix that preserves this behavior could look like:

find_intersections(x, y) = _find_intersections(collect(x), collect(y))
find_intersections(x::AbstractVector{<:AbstractInterval}, y::AbstractVector{<:AbstractInterval})  =  _find_intersections(x, y)

and then defining

function _find_intersections(x,y)
   ...
end

with what is currently in the method at https://github.com/invenia/Intervals.jl/blob/a785b56ce5e0091928fbb152bd2aa4834ada00d4/src/interval_sets.jl#L481.