Skip to content

Fix apparent nondeterminism in pattern matching compilation. #7557

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

Merged
merged 1 commit into from
Jun 17, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
- Prop punning when types don't match results in I/O error: _none_: No such file or directory. https://github.com/rescript-lang/rescript/pull/7533
- Fix partial application with user-defined function types. https://github.com/rescript-lang/rescript/pull/7548
- Fix doc comment before variant throwing syntax error. https://github.com/rescript-lang/rescript/pull/7535
- Fix apparent non-determinism in generated code for pattern matching. https://github.com/rescript-lang/rescript/pull/7557

#### :nail_care: Polish

Expand Down
5 changes: 4 additions & 1 deletion compiler/ml/matching.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1808,7 +1808,10 @@ let reintroduce_fail sw =
let i_max = ref (-1) and max = ref (-1) in
Hashtbl.iter
(fun i c ->
if c > !max then (
if
c > !max || (c = !max && i > !i_max)
Copy link
Preview

Copilot AI Jun 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tie-break condition claims to choose the smallest index, but the condition 'i > !i_max' actually selects the larger index. Consider revising it to 'i < !i_max' to ensure the intended deterministic behavior.

Suggested change
c > !max || (c = !max && i > !i_max)
c > !max || (c = !max && i < !i_max)

Copilot uses AI. Check for mistakes.

(* tie-break for determinism: choose the smallest index*)
then (
i_max := i;
max := c))
t;
Expand Down
36 changes: 36 additions & 0 deletions tests/tests/src/NondetPatterMatch.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// Generated by ReScript, PLEASE EDIT WITH CARE


function f1(action) {
if (typeof action === "object") {
switch (action.TAG) {
case "WithPayload5" :
case "WithPayload6" :
case "WithPayload7" :
case "WithPayload8" :
console.log("hello");
break;
}
}
return 42;
}

function f2(action) {
if (typeof action === "object") {
switch (action.TAG) {
case "WithPayload5" :
case "WithPayload6" :
case "WithPayload7" :
case "WithPayload8" :
console.log("hello");
break;
}
}
return 42;
}

export {
f1,
f2,
}
/* No side effect */
33 changes: 33 additions & 0 deletions tests/tests/src/NondetPatterMatch.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
type action =
| WithoutPayload1
| WithoutPayload2
| WithPayload2({y: int})
| WithPayload3({y: int})
| WithPayload5({y: int})
| WithPayload6({x: int})
| WithPayload7({y: int})
| WithPayload8({x: int})

let f1 = (action: action) => {
switch action {
| WithPayload5(_)
| WithPayload6(_)
| WithPayload7(_)
| WithPayload8(_) =>
Console.log("hello")
| _ => ()
}
42
}

let f2 = (action: action) => {
switch action {
| WithPayload5(_)
| WithPayload6(_)
| WithPayload7(_)
| WithPayload8(_) =>
Console.log("hello")
| _ => ()
}
42
}
27 changes: 15 additions & 12 deletions tests/tests/src/adt_optimize_test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -134,19 +134,22 @@ function f8(x) {

function f9(x) {
if (typeof x !== "object") {
if (x === "T63") {
return 3;
} else {
return 1;
switch (x) {
case "T60" :
case "T61" :
case "T62" :
return 1;
default:
return 3;
}
} else {
switch (x.TAG) {
case "T64" :
case "T65" :
return 2;
default:
return 3;
}
}
switch (x.TAG) {
case "T64" :
case "T65" :
return 2;
case "T66" :
case "T68" :
return 3;
}
}

Expand Down