-
Notifications
You must be signed in to change notification settings - Fork 456
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Casting infinity
to integer makes value 0
#6737
Comments
Maybe we could guide the user by adding |
I guess the compiler uses the bitwise operation to convert float to int e.g. |
Yes for sure. It would be nice to have poly infinity, since not, at least there should be something to avoid pitfalls |
Indeed, since we don't have a polymorphic number type, we have to handle it to avoid pitfalls. |
or can we make it a keyword? then it can act like a polymorphic |
Another pitfalls: let t4 = (22130921800000. -. 0.)->Belt.Float.toInt
let t5 = (22130921800000.)->Belt.Float.toInt generates var t4 = 22130921800000 - 0 | 0; // -1044676288
var t5 = 2147483647; // max value of Int32 |
This issue doesn't seem to be one that can be solved with types. I think we need to rethink the rules for converting Float to Int. |
It looks like another kind of problem 😅
The reason infinity is confusing is that, as mentioned above, it can be used as a kind of flag. |
let t5 = (22130921800000.)->Belt.Float.toInt Shouldn't this also be generated like this, at least in terms of consistency? var t5 = 22130921800000 | 0; |
Side note: Why did the compiler try to optimize numeric operations with Int32 for the int type? Because OCaml's Int64 can handle JS's safe integer (2^53). |
Yes, I expect both examples to be equally |
Or, Another option is to not optimize and not change the value in generated js. |
No, it because JavaScript JIT engines are actually perform better with int32. Inside the JS optimizer, there is a clear distinction between numbers that exceed 32 bits and those that don't, and being able to guarantee 32-bit integers in a specific path has huge optimization benefits. A bit off-topic, JavaScript's subtle number handling causes problems when interoperating with other systems. This often causes interoperability problems with most systems that make a stricter distinction between int32/int64/float32/float64/etc. For example, GraphQL |
It might be "correct" for However, it still poses problems for interoperability with other JS libraries that use |
Okay, I'm still not sure about the rules for converting float values beyond int32 to int actually, but I think the inconsistency between t4 and t5 is another issue. I'll open another issue for it.
I agree that int32 coercion process should not happen for infinity. |
Can you elaborate this? |
we can make it have an independent meaning on the context rather than just the value ident. but never mind, I've changed my mind. That wouldn't be a good idea. Because it's actually a value, if we don't treat it as a value it'll have other problems. |
|
Mergin #6736 to this. I don't think there's a problem with the overall ReScript's semantic. However, it inherits some of the JS ecosystem where all numbers can potentially be converted to floating anytime. When we write a library binding, we simply make an interface float to represent the JS numbers. I could say that is naive, but I think it's pretty conventional today. (even we use that example in the official blog post) Actually, even though I'm pretty familiar with ReScript, at one point I made a bug (I simply made the initial value to infinity, because that's ok in JS) and didn't suspect it was the problem for a while. I think we can at least generate a compiler warning like we do with the untagged variants. Even further, maybe we can create a new number type that is explicitly selected for JS interop purposes. |
I'm not sure what you're asking for here. As far as I'm aware, |
I think that the comment by @glennsl is the good reference for this issue #6038 (comment)
|
No, considering
And in the JS spec So I would not say that is completely wrong for all number values in JS to have to be mapped to Let me drop an example, imagine there is a JS animation library that doesn't support partial repetition. And has a FFI authors have no hesitation in modeling this as an Or if even something more "correct": @unboxed
type repeat =
// isn't supported
| @as(infinity) Infinity
// probably wrong for precision, but it's practical choice
| Count(int)
type options = {
repeat: repeat,
} |
I'm not sure what you mean is contradictory with what I'm saying. As far as I can tell, The underlying difference between I can see why you would want to model it this way from a conceptual perspective, but I fear this would just further obscure what's actually going on and invite more confusion and trouble, for relatively little gain. |
Yes for sure, and I thought that All Number operations defined in the ECMA specification refer to |
The From what I can tell, SMI does not include any representations for |
I mean, there are no additional allocations in |
There isn't for |
I don't think it returns to a different values after the basic operations (except min/max) with +/- |
To clarify, I am not (yet) advocating here to make If our |
Depends how you define problems. I think it might lead to confusion about the underlying fundamentals, which might lead to mistakes that cause poor performance. Operations aren't determined by values directly, but by types inferred from the values used. If you have a box that mostly contains
On a conceptual level I kind of agree, but this goes against JS semantics and will make float to int conversion significantly more expensive. |
What does that mean? Checking if values are And sure adding runtime check is redundant and unnecessarily expensive. But if we keep |
There was too much divergence. Let me summarize the arguments:
But we have an actual problem here, given the following situation (pseudocode):
For JS users, rather than modeling the Users may reach abstract operations in ECMAScript spec are not the JS semantic and are never directly exposed to the JS user. The only number operations that reference When This is not a good thing for ReScript. This is just as dangerous as the non-exhaustive matches we care about. I can suggest a few options that I explored here:
|
generates
both are
0
. so we don't actually have a way to compare int value with the infinity without%raw
. becauseinfinity
in pervasive is floatInfinity
is often used in arithmetic operations as well as a value indicating "no limit" (e.g. pagination)#6038 also mentioned
The text was updated successfully, but these errors were encountered: