Warn about unsafe integer values on JavaScript #3764
Conversation
| .warning(Warning::JavaScriptIntUnsafe { location }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Seeing as we want to use the int values in a few places should we do this string-to-int during parsing? To avoid each step of the compiler parsing it over and over
There was a problem hiding this comment.
The most recent commit to this branch attempts this. I'm not 100% sure it's what you were meaning, but it might be!
Token::Intgets anint_value: Option<BigInt>field, which flows through toUntypedExpr::Int,TypedExpr::Int,Pattern::Int, andConstant::Int.- This field is then used for the new warning.
There was a problem hiding this comment.
Very nice! Why is it optional? Shouldn't it always be present?
There was a problem hiding this comment.
Yes you're right, I've made it always present and fixed handling of leading underscores on parsing as bigint.
There are now three .expect("int value to parse as bigint") calls, but I believe these to be safe due to previous validation done during parsing.
| fn javascript_unsafe_int() { | ||
| assert_js_warning!( | ||
| r#" | ||
| pub fn go() { |
There was a problem hiding this comment.
Split these into multiple tests please, each test should only test one case.
There was a problem hiding this comment.
Have made the tests more granular
| @@ -1046,6 +1046,22 @@ See: https://tour.gleam.run/functions/pipelines/", | |||
| }), | |||
| } | |||
| } | |||
|
|
|||
| type_::Warning::JavaScriptIntUnsafe { location } => Diagnostic { | |||
| title: "Int is outside JavaScript's safe integer range".into(), | |||
There was a problem hiding this comment.
It's not clear what this means, could you add more information to the test please 🙏
There was a problem hiding this comment.
Have filled in the text member, the output is now:
warning: Int is outside the safe range on JavaScript
┌─ /Users/richard/Desktop/int_test/src/int_test.gleam:1:15
│
1 │ pub const i = 9_007_199_254_740_992
│ ^^^^^^^^^^^^^^^^^^^^^ This is not a safe integer on JavaScript
This integer value is too large to be represented accurately by
JavaScript's number type. To avoid this warning integer values must be in
the range -(2^53 - 1) - (2^53 - 1).
See JavaScript's Number.MAX_SAFE_INTEGER and Number.MIN_SAFE_INTEGER
properties for more information.
28c2b0a to
dadac1c
Compare
5327f05 to
54ffd42
Compare
|
Rebased to fix merge conflicts |
54ffd42 to
e684b6a
Compare
This PR adds a warning when targeting JavaScript and Int literals or constants outside JavaScript's safe range are encountered.
Fixes #3746.