Skip to content

Commit 6671778

Browse files
richard-vineylpil
authored andcommitted
Fix incorrect warning about unsafe ints on JS when there's an @external
1 parent d008c13 commit 6671778

6 files changed

+105
-8
lines changed

CHANGELOG.md

+4
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88
custom types on the JavaScript target.
99
([yoshi](https://github.com/joshi-monster))
1010

11+
- Fixed a bug where a warning about unsafe integers on the JavaScript target was
12+
emitted when the enclosing function has an external JavaScript implementation.
13+
([Richard Viney](https://github.com/richard-viney))
14+
1115
## v1.6.0-rc1 - 2024-11-10
1216

1317
### Build tool

compiler-core/src/type_/expression.rs

+15-5
Original file line numberDiff line numberDiff line change
@@ -315,7 +315,9 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
315315
int_value,
316316
..
317317
} => {
318-
if self.environment.target == Target::JavaScript {
318+
if self.environment.target == Target::JavaScript
319+
&& !self.implementations.uses_javascript_externals
320+
{
319321
check_javascript_int_safety(&int_value, location, self.problems);
320322
}
321323

@@ -1309,8 +1311,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
13091311

13101312
// Ensure the pattern matches the type of the value
13111313
let pattern_location = pattern.location();
1312-
let mut pattern_typer =
1313-
pattern::PatternTyper::new(self.environment, &self.hydrator, self.problems);
1314+
let mut pattern_typer = pattern::PatternTyper::new(
1315+
self.environment,
1316+
&self.implementations,
1317+
&self.hydrator,
1318+
self.problems,
1319+
);
13141320
let unify_result = pattern_typer.unify(pattern, value_typ.clone(), None);
13151321

13161322
let minimum_required_version = pattern_typer.minimum_required_version;
@@ -1533,8 +1539,12 @@ impl<'a, 'b> ExprTyper<'a, 'b> {
15331539
subjects: &[TypedExpr],
15341540
location: &SrcSpan,
15351541
) -> Result<(TypedMultiPattern, Vec<TypedMultiPattern>), Error> {
1536-
let mut pattern_typer =
1537-
pattern::PatternTyper::new(self.environment, &self.hydrator, self.problems);
1542+
let mut pattern_typer = pattern::PatternTyper::new(
1543+
self.environment,
1544+
&self.implementations,
1545+
&self.hydrator,
1546+
self.problems,
1547+
);
15381548
let typed_pattern = pattern_typer.infer_multi_pattern(pattern, subjects, location)?;
15391549

15401550
// Each case clause has one or more patterns that may match the

compiler-core/src/type_/pattern.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use std::sync::Arc;
1616

1717
pub struct PatternTyper<'a, 'b> {
1818
environment: &'a mut Environment<'b>,
19+
implementations: &'a Implementations,
1920
hydrator: &'a Hydrator,
2021
mode: PatternMode,
2122
initial_pattern_vars: HashSet<EcoString>,
@@ -36,11 +37,13 @@ enum PatternMode {
3637
impl<'a, 'b> PatternTyper<'a, 'b> {
3738
pub fn new(
3839
environment: &'a mut Environment<'b>,
40+
implementations: &'a Implementations,
3941
hydrator: &'a Hydrator,
4042
problems: &'a mut Problems,
4143
) -> Self {
4244
Self {
4345
environment,
46+
implementations,
4447
hydrator,
4548
mode: PatternMode::Initial,
4649
initial_pattern_vars: HashSet::new(),
@@ -435,7 +438,9 @@ impl<'a, 'b> PatternTyper<'a, 'b> {
435438
} => {
436439
unify(type_, int()).map_err(|e| convert_unify_error(e, location))?;
437440

438-
if self.environment.target == Target::JavaScript {
441+
if self.environment.target == Target::JavaScript
442+
&& !self.implementations.uses_javascript_externals
443+
{
439444
check_javascript_int_safety(&int_value, location, self.problems);
440445
}
441446

compiler-core/src/type_/tests.rs

+13
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,19 @@ macro_rules! assert_js_warning {
318318
};
319319
}
320320

321+
#[macro_export]
322+
macro_rules! assert_js_no_warnings {
323+
($src:expr) => {
324+
let warning = $crate::type_::tests::get_printed_warnings(
325+
$src,
326+
vec![],
327+
crate::build::Target::JavaScript,
328+
None,
329+
);
330+
assert!(warning.is_empty());
331+
};
332+
}
333+
321334
#[macro_export]
322335
macro_rules! assert_warnings_with_gleam_version {
323336
($gleam_version:expr, $src:expr$(,)?) => {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
---
2+
source: compiler-core/src/type_/tests/warnings.rs
3+
expression: "\npub fn main() {\n 9_007_199_254_740_992 + helper()\n}\n\n@external(javascript, \"a\", \"b\")\nfn helper() -> Int\n"
4+
---
5+
----- SOURCE CODE
6+
7+
pub fn main() {
8+
9_007_199_254_740_992 + helper()
9+
}
10+
11+
@external(javascript, "a", "b")
12+
fn helper() -> Int
13+
14+
15+
----- WARNING
16+
warning: Int is outside JavaScript's safe integer range
17+
┌─ /src/warning/wrn.gleam:3:3
18+
19+
3 │ 9_007_199_254_740_992 + helper()
20+
^^^^^^^^^^^^^^^^^^^^^ This is not a safe integer value on JavaScript
21+
22+
This integer value is too large to be represented accurately by
23+
JavaScript's number type. To avoid this warning integer values must be in
24+
the range -(2^53 - 1) - (2^53 - 1).
25+
26+
See JavaScript's Number.MAX_SAFE_INTEGER and Number.MIN_SAFE_INTEGER
27+
properties for more information.

compiler-core/src/type_/tests/warnings.rs

+40-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use super::*;
22
use crate::{
3-
assert_js_warning, assert_no_warnings, assert_warning, assert_warnings_with_gleam_version,
4-
assert_warnings_with_imports,
3+
assert_js_no_warnings, assert_js_warning, assert_no_warnings, assert_warning,
4+
assert_warnings_with_gleam_version, assert_warnings_with_imports,
55
};
66

77
#[test]
@@ -2671,3 +2671,41 @@ pub fn go() {
26712671
"#
26722672
);
26732673
}
2674+
2675+
#[test]
2676+
fn javascript_unsafe_int_with_external_implementation() {
2677+
assert_js_no_warnings!(
2678+
r#"
2679+
@external(javascript, "./test.mjs", "go")
2680+
pub fn go() -> Int {
2681+
9_007_199_254_740_992
2682+
}
2683+
"#
2684+
);
2685+
}
2686+
2687+
#[test]
2688+
fn javascript_unsafe_int_segment_in_pattern_with_external_implementation() {
2689+
assert_js_no_warnings!(
2690+
r#"
2691+
@external(javascript, "./test.mjs", "go")
2692+
pub fn go(b: BitArray) -> BitArray {
2693+
let assert <<0xFFF0000000000000:64>> = b
2694+
}
2695+
"#
2696+
);
2697+
}
2698+
2699+
#[test]
2700+
fn javascript_unsafe_int_with_external_function_call() {
2701+
assert_js_warning!(
2702+
r#"
2703+
pub fn main() {
2704+
9_007_199_254_740_992 + helper()
2705+
}
2706+
2707+
@external(javascript, "a", "b")
2708+
fn helper() -> Int
2709+
"#
2710+
);
2711+
}

0 commit comments

Comments
 (0)