Fix infinite loop when enumerating empty Date.Range

[sabiwara]

Close #14746

The second commit skips calling fun altogether for empty enumerables, which is what is being done for other slicing functions like drop / slice (and which is why the Enum.slice tests were passing).

To be backported.

[josevalim]

The docs say this:

The `start` position is a number `>= 0` and guaranteed to
exist in the `enumerable`. The length is a number `>= 1`
in a way that `start + length * step <= count`, where
`count` is the maximum amount of elements in the enumerable.

So I think this fix is not necessary. Instead we should indeed avoid calling it.

[sabiwara]

I'm fine reverting it, but just mentioning that I personally prefer the new version of the code even if not to fix a bug:

• consistency with the Range impl
• the recursion feels simpler stopping on the empty list clause, less redundancy (double date_from_iso_days(current, calendar))

But it's really a nit, your call 😄

[josevalim]

As you prefer! Ending on 1 can help us catch future bugs though. :)

[sabiwara]

If we want catching, perhaps a guard on > 1 on the other clause could lead to a nicer way to fail than an infinite loop? But that might have an impact on perf, and I'm too lazy to bench it 😄

I was also wondering if the type system could have caught such a bug in a defimpl, but since integers don't have a subset I don't think there's an easy way to catch it at compile-time, right?

[josevalim]

Yeah, can’t catch at compile time. I think performance cost is negligible. Pick whatever approach you prefer!

[sabiwara]

OK, went with making both Range and Date.Range consistent and added a defensive guard: aef73cb.

[josevalim]

I believe we only need the second commit. Ship it!

[sabiwara]

Also, question: after I merge, should I backport this to 1.18 as well as 1.19?

[sabiwara]

Backported to v1.19 ✅