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

Mixing Number and Bigint #49

Closed Jack-Works closed 1 year ago

Jack-Works commented 3 years ago

As for the BIgInt Math proposal, I think it's reasonable to reconsider how we treat different number types.

Currently, any type mismatch will cause a TypeError, but I think some of the cases can be tolerated.

Case 1: BigInt input on Number.range

It should return a number range. (Implicit convert bigint to number is safe (maybe?))

Case 2: Number input on BigInt.range

Convert them if they do not have the decimal part.

BigInt.range(0, 1n) should works.

BigInt.range(0.5, 1n) should throw TypeError. (Or we round them?)

Any idea?

bakkot commented 3 years ago

Implicit convert bigint to number is safe (maybe?)

It is definitely not safe. BigInt.range(9007199254740990n, 9007199254740995n) should give me five different values. If I accidentally write Number.range(9007199254740990n, 9007199254740995n), I'll only get four different values.

BigInt.range(0, 1n)

Why would we want to allow this? Is there a place it's likely to come up?

Jack-Works commented 3 years ago

Why would we want to allow this? Is there a place it's likely to come up?

I don't know, maybe for the same reason as Math.min(0, 1n)

Jack-Works commented 3 years ago

It is definitely not safe. BigInt.range(9007199254740990n, 9007199254740995n) should give me five different values. If I accidentally write Number.range(9007199254740990n, 9007199254740995n), I'll only get four different values.

Maybe we can throw TypeError when the convert is bigger than MAX_SAFE_INT

bakkot commented 3 years ago

I don't know, maybe for the same reason as Math.min(0, 1n)

We should default to disallowing implicit conversions and only them if there is a strong reason to do so.

The only reason to allow Math.min(0, 1n) is because you can already write 0 < 1n (which is well-defined). But range is not like that; it performs arithmetic (not just comparisons) to produce values. So I don't think that the analogy to Math.min is a good enough reason.

Maybe we can throw TypeError when the convert is bigger than MAX_SAFE_INT

I think that would be very bad. People will try it with small Integers and get no indication that it will fail for larger ones. I'd expect that to be a major source of bugs.

ljharb commented 3 years ago

If range is on Number, it should only take Numbers, and if on BigInt, it should only take BigInts.

Mixing them would potentially make sense for methods on other objects, like Math - but I assume you're not suggesting Math.range instead of having range on Number/BigInt?

js-choi commented 3 years ago

@Jack-Works: The BigInt Math proposal is currently focusing on type-overloading just a few Math methods, before possibly adding more overloaded methods like a Math.popcnt. There would be no type mixing except in min and max. For example, Math.pow(5, 2n) would throw a TypeError, just like how 5 ** 2n would throw a TypeError. The proposal is trying to match the precedent set by the math syntax operators.

So my question is—is there a particular reason why you might not want to take a similar approach with range?

In other words, have you considered, instead of Number.range, having a Math.range with type-overloading, so that Math.range(0, 5) and Math.range(0n, 5n) work but Math.range(0, 5n) throws a TypeError?

(I could re-raise this in a new issue here, if you would prefer.)

Jack-Works commented 3 years ago

For example, Math.pow(5, 2n) would throw a TypeError

Oh, I didn't notice that.

Math.range with type-overloading, so that Math.range(0, 5) and Math.range(0n, 5n) work but Math.range(0, 5n) throws a TypeError?

Yes, I think that will be convenient. How do others think?

js-choi commented 3 years ago

Yes, I plan to update the BigInt Math proposal with a vision section. The vision is to start with type-overloading a few select existing methods in Math (sign, abs, pow, min, and max) and form a precedent with which other new Math proposals like popcnt or modPow could follow in type overloading.

This would be one of those proposals, if you feel that that would fit.

Jack-Works commented 3 years ago

This would be one of those proposals, if you feel that that would fit.

I think BigInt Math will reach stage 4 much faster than range so I'll take care of Math.range 😂

Andrew-Cottrell commented 3 years ago

I think of range as a factory function rather than a mathematical operation, so it makes less semantic sense to me that Math.range would exist.

Additionally, I use type checked JavaScript (via JSDoc annotations), so I would much prefer Number.range and BigInt.range rather than Math.range as a function with an overloaded return type reduces the ability of the type checker to detect errors.

ljharb commented 3 years ago

We don’t design javascript based on the current limits of a type checker.

js-choi commented 3 years ago

To add to that—I am sympathetic about type-checking JavaScript, which I do a lot myself. 😄

But there are already many type-overloaded operations in JavaScript. This is not too different than how ** is type overloaded to both Numbers and BigInts—or how Array.from accepts both iterables and non-iterable array-like objects. Any JavaScript type checker already needs to be able to deal with those basic existing cases. Hopefully an overloaded range would eventually be able to fit in your workflow too.

Andrew-Cottrell commented 3 years ago

Well... I did indicate the type checking aspect led only to a preference. I agree JavaScript should not be constrained by tooling and workflows, but I think it's reasonable to mention it as a factor to be considered.

My primary point was regarding the semantic aspect of adding a factory function to the Math namespace-object, which I have understood to be for mathematical operations (and mathematical constants). I'm happy to be corrected if I have the wrong understanding.

ljharb commented 3 years ago

Producing a range is performing repeated addition, so it’s definitely a mathematical operation.

js-choi commented 3 years ago

It is true that this might well hypothetically live in the iterator-helpers proposal’s Iterator object. But I think Math would be fine, though.

Jack-Works commented 1 year ago

Decide not to do this.