diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2da6765c3..67a55168d3b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ #### :bug: Bug fix - Fix integer range patterns, including nested record and exception patterns, incorrectly matching values below the range. https://github.com/rescript-lang/rescript/pull/8714 +- Fix integer range patterns at int32 boundaries and wrapped nondecimal patterns matching incorrect values. https://github.com/rescript-lang/rescript/pull/8717 https://github.com/rescript-lang/rescript/pull/8858 +- Integer patterns that alias the same int32 value (e.g. `0xFFFF_FFFF` and `-1`) may now produce unused-case warnings. https://github.com/rescript-lang/rescript/pull/8717 https://github.com/rescript-lang/rescript/pull/8858 #### :nail_care: Polish diff --git a/compiler/ml/matching.ml b/compiler/ml/matching.ml index b84339aed94..78bc6ecefbb 100644 --- a/compiler/ml/matching.ml +++ b/compiler/ml/matching.ml @@ -25,6 +25,9 @@ open Printf let dbg = false +let int32_low = Int32.to_int Int32.min_int +let int32_high = Int32.to_int Int32.max_int + (* See Peyton-Jones, ``The Implementation of functional programming languages'', chapter 5. *) (* @@ -2032,7 +2035,10 @@ let combine_constant names loc arg cst partial ctx def | _ -> assert false) const_lambda_list in - call_switcher loc fail arg min_int max_int int_lambda_list names + (* Match intervals must use ReScript's int32 bounds, not the host's. + Otherwise the upper fail interval starts at 2147483648, which wraps + when converted to a Lambda integer constant. *) + call_switcher loc fail arg int32_low int32_high int_lambda_list names | Const_char _ -> let int_lambda_list = List.map @@ -2212,13 +2218,13 @@ let combine_constructor sw_names loc arg ex_pat cstr partial ctx def let make_test_sequence_variant_constant fail arg int_lambda_list = let _, (cases, actions) = - as_interval fail min_int max_int + as_interval fail int32_low int32_high (List.map (fun (a, (_, c)) -> (a, c)) int_lambda_list) in Switcher.test_sequence arg cases actions let call_switcher_variant_constant loc fail arg int_lambda_list names = - call_switcher loc fail arg min_int max_int + call_switcher loc fail arg int32_low int32_high (List.map (fun (a, (_, c)) -> (a, c)) int_lambda_list) names @@ -2229,7 +2235,7 @@ let call_switcher_variant_constr loc fail arg int_lambda_list names = Pgenval, v, Lprim (Pfield (0, Fld_poly_var_tag), [arg], loc), - call_switcher loc fail (Lvar v) min_int max_int + call_switcher loc fail (Lvar v) int32_low int32_high (List.map (fun (a, (_, c)) -> (a, c)) int_lambda_list) names ) diff --git a/compiler/ml/typecore.ml b/compiler/ml/typecore.ml index 25d46af3c88..763864f4e2e 100644 --- a/compiler/ml/typecore.ml +++ b/compiler/ml/typecore.ml @@ -252,7 +252,12 @@ let type_constant = function let constant : Parsetree.constant -> (Asttypes.constant, error) result = function | Pconst_integer (i, None) -> ( - try Ok (Const_int (Misc.Int_literal_converter.int i)) + try + (* After the existing host-dependent literal parsing, pattern grouping + and intervals must see Lambda's int32 value, so -0xFFFF_FFFF and 1 + are the same key. This does not change overflow diagnostics. *) + let value = Misc.Int_literal_converter.int i in + Ok (Const_int (Int32.to_int (Int32.of_int value))) with Failure _ -> Error (Literal_overflow "int")) | Pconst_integer (i, Some 'n') -> let sign, i = Bigint_utils.parse_bigint i in diff --git a/tests/build_tests/super_errors/expected/intoverflow.res.expected b/tests/build_tests/super_errors/expected/intoverflow.res.expected index a0ef2bff856..f0533f23b9c 100644 --- a/tests/build_tests/super_errors/expected/intoverflow.res.expected +++ b/tests/build_tests/super_errors/expected/intoverflow.res.expected @@ -53,5 +53,42 @@ 11 │ let v9: int = 0x1_0000_0000 // overflow 12 │ let v10: int = -0x1_0000_0000 // underflow 13 │ + 14 │ // Wrapped aliases are redundant after int32 normalization. - Integer literal exceeds the range of representable integers of type int \ No newline at end of file + Integer literal exceeds the range of representable integers of type int + + + Warning number 107 + /.../fixtures/intoverflow.res:17:5-14 + + 15 │ let wrappedAlias = value => + 16 │ switch value { + 17 │ | 4294967295 => 1 + 18 │ | -1 => 2 + 19 │ | _ => 0 + + Integer literal exceeds the range of representable integers of type int + + + Warning number 11 + /.../fixtures/intoverflow.res:18:5-6 + + 16 ┆ switch value { + 17 ┆ | 4294967295 => 1 + 18 ┆ | -1 => 2 + 19 ┆ | _ => 0 + 20 ┆ } + + this match case is unused. + + + Warning number 11 + /.../fixtures/intoverflow.res:25:5 + + 23 ┆ switch value { + 24 ┆ | -0xFFFF_FFFF => 1 + 25 ┆ | 1 => 2 + 26 ┆ | _ => 0 + 27 ┆ } + + this match case is unused. \ No newline at end of file diff --git a/tests/build_tests/super_errors/fixtures/intoverflow.res b/tests/build_tests/super_errors/fixtures/intoverflow.res index 4bdee11d6af..2b27aaa106b 100644 --- a/tests/build_tests/super_errors/fixtures/intoverflow.res +++ b/tests/build_tests/super_errors/fixtures/intoverflow.res @@ -10,3 +10,18 @@ let v7: int = 0xFFFF_FFFF // -1 let v8: int = -0xFFFF_FFFF // 1 let v9: int = 0x1_0000_0000 // overflow let v10: int = -0x1_0000_0000 // underflow + +// Wrapped aliases are redundant after int32 normalization. +let wrappedAlias = value => + switch value { + | 4294967295 => 1 + | -1 => 2 + | _ => 0 + } + +let nondecimalAlias = value => + switch value { + | -0xFFFF_FFFF => 1 + | 1 => 2 + | _ => 0 + } diff --git a/tests/ounit_tests/ounit_integer_constant_tests.ml b/tests/ounit_tests/ounit_integer_constant_tests.ml new file mode 100644 index 00000000000..8849408c508 --- /dev/null +++ b/tests/ounit_tests/ounit_integer_constant_tests.ml @@ -0,0 +1,35 @@ +open OUnit + +let suites = + __FILE__ + >::: [ + ( "integer pattern constants agree with Lambda" >:: fun _ -> + List.iter + (fun (literal, expected) -> + match Typecore.constant (Pconst_integer (literal, None)) with + | Ok (Asttypes.Const_int value as typed) -> + assert_equal expected value; + assert_equal + (Lam_constant.Const_int + {i = Int32.of_int expected; comment = None}) + (Lam_constant_convert.convert_constant + (Lambda.Const_base typed)) + | _ -> assert_failure ("integer literal was rejected: " ^ literal)) + [ + ("-0xFFFF_FFFF", 1); + ("0xFFFF_FFFF", -1); + ("2147483647", 2147483647); + ("-2147483648", -2147483648); + ] ); + ( "overflow parsing keeps its existing host limits" >:: fun _ -> + let result = + Typecore.constant (Pconst_integer ("2147483649", None)) + in + assert_equal + (if Sys.int_size > 32 then Ok (Asttypes.Const_int (-2147483647)) + else Error (Typecore.Literal_overflow "int")) + result; + assert_equal (Error (Typecore.Literal_overflow "int")) + (Typecore.constant + (Pconst_integer ("999999999999999999999999999", None))) ); + ] diff --git a/tests/ounit_tests/ounit_tests_main.ml b/tests/ounit_tests/ounit_tests_main.ml index f5a676e5fa8..f61d7efb1d3 100644 --- a/tests/ounit_tests/ounit_tests_main.ml +++ b/tests/ounit_tests/ounit_tests_main.ml @@ -21,6 +21,7 @@ let suites = Ounit_bsb_regex_tests.suites; Ounit_bsb_pkg_tests.suites; Ounit_util_tests.suites; + Ounit_integer_constant_tests.suites; Ounit_jsx_loc_tests.suites; Ounit_analysis_references_tests.suites; ] diff --git a/tests/tests/src/int_range_boundary_test.mjs b/tests/tests/src/int_range_boundary_test.mjs new file mode 100644 index 00000000000..1d3279a91b7 --- /dev/null +++ b/tests/tests/src/int_range_boundary_test.mjs @@ -0,0 +1,360 @@ +// Generated by ReScript, PLEASE EDIT WITH CARE + +import * as Mocha from "mocha"; +import * as Test_utils from "./test_utils.mjs"; + +function upperRange(value) { + return value >= 2147483645; +} + +function lowerRange(value) { + return value < -2147483645; +} + +function recordRange(value) { + return value.status >= 2147483645; +} + +function tupleRange(value) { + if (value[0] >= 2147483645) { + return value[1]; + } else { + return false; + } +} + +function variantRange(value) { + return value._0 >= 2147483645; +} + +function arrayRange(value) { + if (value.length !== 1) { + return false; + } + let match = value[0]; + return match >= 2147483645; +} + +function callRange(value) { + let match = value(); + return match >= 2147483645; +} + +function guardedRange(value, enabled) { + if (value >= 2147483645) { + return enabled; + } else { + return false; + } +} + +function bothRanges(value) { + if (value >= -2147483645) { + if (value >= 2147483645) { + return 1; + } else { + return 0; + } + } else { + return -1; + } +} + +function separateCases(value) { + if (value >= -2147483645) { + if (value < 2147483645) { + return 0; + } + switch (value) { + case 2147483645 : + return 4; + case 2147483646 : + return 5; + case 2147483647 : + return 6; + } + } else { + switch (value) { + case -2147483648 : + return 1; + case -2147483647 : + return 2; + case -2147483646 : + return 3; + } + } +} + +function wrappedHex(value) { + return value === 1; +} + +function wrappedOctal(value) { + return value === 1; +} + +function wrappedBinary(value) { + return value === 1; +} + +function wrappedRange(value) { + return !(value > 3 || value < 1); +} + +function wrappedRecordRange(value) { + let match = value.status; + if (match > 0) { + return match < 4; + } else { + return match === -1; + } +} + +function wrappedSeparateCases(value) { + if (value >= 3) { + if (value >= 2147483647) { + return 1; + } else { + return 0; + } + } + if (value < -1) { + return 0; + } + switch (value) { + case -1 : + return 4; + case 0 : + return 5; + case 1 : + return 2; + case 2 : + return 3; + } +} + +function wrappedAliasGuard(value, enabled) { + if (value !== 1) { + return 0; + } else if (enabled) { + return 1; + } else { + return 2; + } +} + +function decimalAliasGuard(value, enabled) { + if (value !== 1) { + return 0; + } else if (enabled) { + return 1; + } else { + return 2; + } +} + +let wrappedSamples = [ + -2147483648, + -2, + -1, + 0, + 1, + 2, + 3, + 4, + 2147483646, + 2147483647 +]; + +let samples = [ + [ + -2147483648, + true, + false, + 1 + ], + [ + -2147483647, + true, + false, + 2 + ], + [ + -2147483646, + true, + false, + 3 + ], + [ + -2147483645, + false, + false, + 0 + ], + [ + -1, + false, + false, + 0 + ], + [ + 0, + false, + false, + 0 + ], + [ + 1, + false, + false, + 0 + ], + [ + 2147483644, + false, + false, + 0 + ], + [ + 2147483645, + false, + true, + 4 + ], + [ + 2147483646, + false, + true, + 5 + ], + [ + 2147483647, + false, + true, + 6 + ] +]; + +Mocha.describe("Int_range_boundary_test", () => { + Mocha.test("integer ranges at both 32-bit limits", () => { + samples.forEach(param => { + let upper = param[2]; + let lower = param[1]; + let value = param[0]; + Test_utils.eq("File \"int_range_boundary_test.res\", line 147, characters 11-18", value >= 2147483645, upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 148, characters 11-18", value < -2147483645, lower); + Test_utils.eq("File \"int_range_boundary_test.res\", line 149, characters 11-18", bothRanges(value), lower ? -1 : ( + upper ? 1 : 0 + )); + }); + }); + Mocha.test("upper-limit ranges in nested patterns and function results", () => { + samples.forEach(param => { + let upper = param[2]; + let value = param[0]; + Test_utils.eq("File \"int_range_boundary_test.res\", line 157, characters 11-18", recordRange({ + status: value + }), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 158, characters 11-18", tupleRange([ + value, + true + ]), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 159, characters 11-18", tupleRange([ + value, + false + ]), false); + Test_utils.eq("File \"int_range_boundary_test.res\", line 160, characters 11-18", variantRange({ + TAG: "Status", + _0: value + }), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 161, characters 11-18", arrayRange([value]), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 162, characters 11-18", arrayRange([ + value, + value + ]), false); + Test_utils.eq("File \"int_range_boundary_test.res\", line 163, characters 11-18", arrayRange([]), false); + Test_utils.eq("File \"int_range_boundary_test.res\", line 164, characters 11-18", callRange(() => value), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 165, characters 11-18", guardedRange(value, true), upper); + Test_utils.eq("File \"int_range_boundary_test.res\", line 166, characters 11-18", guardedRange(value, false), false); + }); + }); + Mocha.test("distinct actions at both 32-bit limits", () => { + samples.forEach(param => Test_utils.eq("File \"int_range_boundary_test.res\", line 174, characters 11-18", separateCases(param[0]), param[3])); + }); + Mocha.test("wrapped nondecimal patterns match only their int32 value", () => { + wrappedSamples.forEach(value => { + Test_utils.eq("File \"int_range_boundary_test.res\", line 182, characters 11-18", value === 1, value === 1); + Test_utils.eq("File \"int_range_boundary_test.res\", line 183, characters 11-18", value === 1, value === 1); + Test_utils.eq("File \"int_range_boundary_test.res\", line 184, characters 11-18", value === 1, value === 1); + }); + }); + Mocha.test("wrapped ranges are sorted in the int32 domain", () => { + wrappedSamples.forEach(value => { + let inRange = value >= 1 && value <= 3; + Test_utils.eq("File \"int_range_boundary_test.res\", line 193, characters 11-18", !(value > 3 || value < 1), inRange); + Test_utils.eq("File \"int_range_boundary_test.res\", line 194, characters 11-18", wrappedRecordRange({ + status: value + }), value === -1 || inRange); + let expected; + if (value >= 3) { + expected = value >= 2147483647 ? 1 : 0; + } else if (value >= -1) { + switch (value) { + case -1 : + expected = 4; + break; + case 0 : + expected = 5; + break; + case 1 : + expected = 2; + break; + case 2 : + expected = 3; + break; + } + } else { + expected = 0; + } + Test_utils.eq("File \"int_range_boundary_test.res\", line 203, characters 11-18", wrappedSeparateCases(value), expected); + }); + }); + Mocha.test("guards fall through between wrapped and decimal aliases", () => { + wrappedSamples.forEach(value => { + [ + false, + true + ].forEach(enabled => { + let expected = value === 1 ? ( + enabled ? 1 : 2 + ) : 0; + Test_utils.eq("File \"int_range_boundary_test.res\", line 214, characters 15-22", wrappedAliasGuard(value, enabled), expected); + Test_utils.eq("File \"int_range_boundary_test.res\", line 215, characters 15-22", decimalAliasGuard(value, enabled), expected); + }); + }); + }); +}); + +export { + upperRange, + lowerRange, + recordRange, + tupleRange, + variantRange, + arrayRange, + callRange, + guardedRange, + bothRanges, + separateCases, + wrappedHex, + wrappedOctal, + wrappedBinary, + wrappedRange, + wrappedRecordRange, + wrappedSeparateCases, + wrappedAliasGuard, + decimalAliasGuard, + wrappedSamples, + samples, +} +/* Not a pure module */ diff --git a/tests/tests/src/int_range_boundary_test.res b/tests/tests/src/int_range_boundary_test.res new file mode 100644 index 00000000000..848cc1ad7ff --- /dev/null +++ b/tests/tests/src/int_range_boundary_test.res @@ -0,0 +1,221 @@ +open Mocha +open Test_utils + +type response = {status: int} +type status = Status(int) + +let upperRange = value => + switch value { + | 2147483645 | 2147483646 | 2147483647 => true + | _ => false + } + +let lowerRange = value => + switch value { + | -2147483648 | -2147483647 | -2147483646 => true + | _ => false + } + +let recordRange = value => + switch value { + | {status: 2147483645 | 2147483646 | 2147483647} => true + | _ => false + } + +let tupleRange = value => + switch value { + | (2147483645 | 2147483646 | 2147483647, true) => true + | _ => false + } + +let variantRange = value => + switch value { + | Status(2147483645 | 2147483646 | 2147483647) => true + | _ => false + } + +let arrayRange = value => + switch value { + | [2147483645 | 2147483646 | 2147483647] => true + | _ => false + } + +let callRange = value => + switch value() { + | 2147483645 | 2147483646 | 2147483647 => true + | _ => false + } + +let guardedRange = (value, enabled) => + switch value { + | 2147483645 | 2147483646 | 2147483647 if enabled => true + | _ => false + } + +let bothRanges = value => + switch value { + | -2147483648 | -2147483647 | -2147483646 => -1 + | 2147483645 | 2147483646 | 2147483647 => 1 + | _ => 0 + } + +let separateCases = value => + switch value { + | -2147483648 => 1 + | -2147483647 => 2 + | -2147483646 => 3 + | 2147483645 => 4 + | 2147483646 => 5 + | 2147483647 => 6 + | _ => 0 + } + +let wrappedHex = value => + switch value { + | -0xFFFF_FFFF => true + | _ => false + } + +let wrappedOctal = value => + switch value { + | -0o37777777777 => true + | _ => false + } + +let wrappedBinary = value => + switch value { + | -0b1111_1111_1111_1111_1111_1111_1111_1111 => true + | _ => false + } + +let wrappedRange = value => + switch value { + | -0xFFFF_FFFF | -0xFFFF_FFFE | -0xFFFF_FFFD => true + | _ => false + } + +let wrappedRecordRange = value => + switch value { + | {status: 0xFFFF_FFFF | -0xFFFF_FFFF | -0xFFFF_FFFE | -0xFFFF_FFFD} => true + | _ => false + } + +let wrappedSeparateCases = value => + switch value { + | -0x8000_0001 => 1 + | -0xFFFF_FFFF => 2 + | -0xFFFF_FFFE => 3 + | 0xFFFF_FFFF => 4 + | 0 => 5 + | _ => 0 + } + +let wrappedAliasGuard = (value, enabled) => + switch value { + | -0xFFFF_FFFF if enabled => 1 + | 1 => 2 + | _ => 0 + } + +let decimalAliasGuard = (value, enabled) => + switch value { + | 1 if enabled => 1 + | -0xFFFF_FFFF => 2 + | _ => 0 + } + +let wrappedSamples = [-2147483648, -2, -1, 0, 1, 2, 3, 4, 2147483646, 2147483647] + +let samples = [ + (-2147483648, true, false, 1), + (-2147483647, true, false, 2), + (-2147483646, true, false, 3), + (-2147483645, false, false, 0), + (-1, false, false, 0), + (0, false, false, 0), + (1, false, false, 0), + (2147483644, false, false, 0), + (2147483645, false, true, 4), + (2147483646, false, true, 5), + (2147483647, false, true, 6), +] + +describe(__MODULE__, () => { + test("integer ranges at both 32-bit limits", () => { + samples->Array.forEach( + ((value, lower, upper, _)) => { + eq(__LOC__, upperRange(value), upper) + eq(__LOC__, lowerRange(value), lower) + eq(__LOC__, bothRanges(value), lower ? -1 : upper ? 1 : 0) + }, + ) + }) + + test("upper-limit ranges in nested patterns and function results", () => { + samples->Array.forEach( + ((value, _, upper, _)) => { + eq(__LOC__, recordRange({status: value}), upper) + eq(__LOC__, tupleRange((value, true)), upper) + eq(__LOC__, tupleRange((value, false)), false) + eq(__LOC__, variantRange(Status(value)), upper) + eq(__LOC__, arrayRange([value]), upper) + eq(__LOC__, arrayRange([value, value]), false) + eq(__LOC__, arrayRange([]), false) + eq(__LOC__, callRange(() => value), upper) + eq(__LOC__, guardedRange(value, true), upper) + eq(__LOC__, guardedRange(value, false), false) + }, + ) + }) + + test("distinct actions at both 32-bit limits", () => { + samples->Array.forEach( + ((value, _, _, expected)) => { + eq(__LOC__, separateCases(value), expected) + }, + ) + }) + + test("wrapped nondecimal patterns match only their int32 value", () => { + wrappedSamples->Array.forEach( + value => { + eq(__LOC__, wrappedHex(value), value == 1) + eq(__LOC__, wrappedOctal(value), value == 1) + eq(__LOC__, wrappedBinary(value), value == 1) + }, + ) + }) + + test("wrapped ranges are sorted in the int32 domain", () => { + wrappedSamples->Array.forEach( + value => { + let inRange = value >= 1 && value <= 3 + eq(__LOC__, wrappedRange(value), inRange) + eq(__LOC__, wrappedRecordRange({status: value}), value == -1 || inRange) + let expected = switch value { + | 2147483647 => 1 + | 1 => 2 + | 2 => 3 + | -1 => 4 + | 0 => 5 + | _ => 0 + } + eq(__LOC__, wrappedSeparateCases(value), expected) + }, + ) + }) + + test("guards fall through between wrapped and decimal aliases", () => { + wrappedSamples->Array.forEach( + value => { + [false, true]->Array.forEach( + enabled => { + let expected = value == 1 ? enabled ? 1 : 2 : 0 + eq(__LOC__, wrappedAliasGuard(value, enabled), expected) + eq(__LOC__, decimalAliasGuard(value, enabled), expected) + }, + ) + }, + ) + }) +})