Conversation

cometkim

There is no infinity or nan for bigint.

No need to check int semantic IMHO, bigint works totally different with number (int & float) anyway

@cometkimcometkim requested review from cknitt and mununki November 9, 2024 00:36
@mununki

I don't think I fully understand the intention of this PR. Why do we remove the check for RangeError (Division by zero) that could occur at runtime?

@cometkim

I don't think I fully understand the intention of this PR. Why do we remove the check for RangeError (Division by zero) that could occur at runtime?

We threw a similar but own custom error Division_by_zero, which is used by integer operations

@cometkim

I think it should at least be distinguishable from regular integer errors.

@cometkim

Or I think it would be better to have per-module definitions of exceptions like Primitive_int.Divide_by_zero and Primitive_bigint.Divide_by_zero

Any thoughts on this suggestion? @cknitt @cristianoc

@cristianoc

Or I think it would be better to have per-module definitions of exceptions like Primitive_int.Divide_by_zero and Primitive_bigint.Divide_by_zero

Any thoughts on this suggestion? @cknitt @cristianoc

Without sub typing it seems strange to have to know where the exception is coming from.

@cometkim

sub typing? int and bigint have nothing to do with each other.

@cristianoc

sub typing? int and bigint have nothing to do with each other.

Exactly: there's no way to catch all division by zero other than listing them all. This imposes some refactoring overhead and possible foot guns when toggling between using int and bigint. That's the downsides. I'm not sure about upsides.
Pushed to the extreme, one could have one different exception for each different unsafe operation in core.

@cometkim

I was thinking a little more in terms of JavaScript. Dividing a regular number by 0 returns Infinity.

But bigint is more explicit about everything. All operations on bigint are incompatible with numbers, and there are even no implicit conversions between them.

You mentioned listing of errors, but that's actually the case. Since the two operations never mix, there are two ways to track its cause originally in Js: checking Number.isFinite vs catching RangeError. Mixing it all up into one error case feels like a loss of information to me.

@cometkim

Another problem is that RangeError is the native means expected by JavaScript applications for bigint operations. As we defining different behavior, we have to explicitly state that bigint in ReScript behaves differently than in JavaScript.

@cristianoc

It is true that if one wants to publish a library for consumption by JS then one better not change the semantics.
What's an example of how to catch RangeError from ReScript?

The question remains of what to do with int as (1/0)|0 is 0 and I doubt we would want to ignore division by zero in that way.

@cometkim

What's an example of how to catch RangeError from ReScript?

That's another feature we need to support natively, for all of JS' built-in Error classes

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.