tc39 / proposal-iterator.range

A proposal for ECMAScript to add a built-in Iterator.range()
https://tc39.es/proposal-iterator.range/
MIT License
487 stars 13 forks source link

Add inclusive option #32

Closed Jack-Works closed 4 years ago

Jack-Works commented 4 years ago

In #31 we introduce infer step for range(0, -5) to be -1.

Therefore, if developers want to have inclusive range, they can't dummly do range(x, y + 1) cause that will be buggy. The inclusive option mentioned in #26 is necessary now.

Close #26.

tabatkins commented 4 years ago

The spec isn't quite right - right now, if step is less than 1ulp at end, an inclusive loop will output end as the value multiple times, stopping only when the next iteration would finally be a value higher than end.

That is, Number.range(0, 2**53, {inclusive:true}) will end up outputting 2*53 twice* before finally ending.

But an inclusive range is meant to have precisely one more value than an exclusive range - the one value at the end, which the exclusive range excludes.

So instead the spec should look like:

  1. Let _currentCount_ be _iterator_.[[currentCount]]
+ 1. Let _hitEnd_ be _iterator_.[[hitEnd]]
+ 1. If _hitEnd_ is *true*, return CreateIterResultObject(*undefined*, *true*).
  1. Let _currentYieldingValue_ to be _start_ + (_step_ \* _currentCount_)
+ 1. If _currentYieldingValue_ == _end_, set _iterator_.[[hitEnd]] to *true*.
  1. Let _nextCount_ to _currentCount_ + _one_
  <!-- Prevent value overflow -->
+ 1. If _inclusiveEnd_ is *false* and _currentYieldingValue_ == _end_, return CreateIterResultObject(*undefined*, *true*).
  1. If _ifIncrease_ is *true* and _currentYieldingValue_ > _end_, return CreateIterResultObject(*undefined*, *true*).
  1. If _ifIncrease_ is *false* and _end_ > _currentYieldingValue_, return CreateIterResultObject(*undefined*, *true*).
  1. Set _iterator_.[[currentCount]] to _nextCount_
  1. Return CreateIterResultObject(_currentYieldingValue_, *false*).
  <!-- Finish -->
Jack-Works commented 4 years ago

Thanks for pointing that out. I should add some tests to verify the algorithm after this merge

Jack-Works commented 4 years ago
Number.range(0, 5, {inclusive: true}).toArray()
> (6) [0, 1, 2, 3, 4, 5]

Sorry I didn't catch your meaning, in what case it will produce the end value twice?

tabatkins commented 4 years ago

I gave the example in my earlier comment: Number.range(2**53-1, 2**53, {inclusive:true}). It happens when step is below the Number resolution at end, so it has to iterate more than once to exceed end and finally stop per your suggested text.

My suggested edits instead record the first time it hits end, and then do an early "return done" step afterwards.

Jack-Works commented 4 years ago

@tabatkins thanks! I got your meaning now.