Skip to content

Commit 7069899

Browse files
committed
Fix bug where a ref assignment is moved ouside a conditional
Fixes #7170 Assignment of a ref, or other record with a single mutable field, is compiled to simple variable assignment. This triggers an incorrect optimisation that moves identical assignments in the true and false branches of a conditional above if they are identical and the guard has no side effects. Added a check that if the assigments to be moved are to variable, that variable must not occur free in the guard of the conditional.
1 parent 5ec125a commit 7069899

File tree

5 files changed

+64
-1
lines changed

5 files changed

+64
-1
lines changed

CHANGELOG.md

+3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
1313
# 12.0.0-alpha.6 (Unreleased)
1414

15+
#### :bug: Bug fix
16+
- Fix bug where a ref assignment is moved ouside a conditional.
17+
1518
# 12.0.0-alpha.5
1619

1720
#### :rocket: New Feature

compiler/core/js_analyzer.mli

+1-1
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929

3030
val free_variables_of_statement : J.statement -> Set_ident.t
3131

32-
val free_variables_of_expression : J.finish_ident_expression -> Set_ident.t
32+
val free_variables_of_expression : J.expression -> Set_ident.t
3333

3434
(* val no_side_effect_expression_desc :
3535
J.expression_desc -> bool *)

compiler/core/js_stmt_make.ml

+10
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,16 @@ let if_ ?comment ?declaration ?else_ (e : J.expression) (then_ : J.block) : t =
303303
aux ?comment (E.or_ e (E.not pred1)) ifso ifso1
304304
| ifso1 :: ifso_rest, ifnot1 :: ifnot_rest
305305
when Js_analyzer.eq_statement ifnot1 ifso1
306+
&& (match ifso1.statement_desc with
307+
| Exp
308+
{
309+
expression_desc =
310+
Bin (Eq, {expression_desc = Var (Id v)}, _);
311+
_;
312+
} ->
313+
let guard_vars = Js_analyzer.free_variables_of_expression e in
314+
not (Set_ident.mem guard_vars v)
315+
| _ -> true)
306316
&& Js_analyzer.no_side_effect_expression e ->
307317
(* here we do agressive optimization, because it can help optimization later,
308318
move code outside of branch is generally helpful later
+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
// Generated by ReScript, PLEASE EDIT WITH CARE
2+
3+
4+
let j = 0;
5+
6+
let k = 1;
7+
8+
if (j === 0) {
9+
j = j + 2 | 0;
10+
console.log("j.c");
11+
} else {
12+
j = j + 2 | 0;
13+
}
14+
15+
j = j + 2 | 0;
16+
17+
if (k === 0) {
18+
console.log("k.c");
19+
}
20+
21+
let j$1 = 0;
22+
23+
let k$1 = 0;
24+
25+
export {
26+
j$1 as j,
27+
k$1 as k,
28+
}
29+
/* Not a pure module */
+21
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
type t = {mutable c: int}
2+
3+
let j = {c: 0}
4+
let k = {c: 1}
5+
6+
if j.c == 0 {
7+
j.c = j.c + 2
8+
Console.log("j.c")
9+
} else {
10+
j.c = j.c + 2
11+
}
12+
13+
if k.c == 0 {
14+
j.c = j.c + 2
15+
Console.log("k.c")
16+
} else {
17+
j.c = j.c + 2
18+
}
19+
20+
let j = 0
21+
let k = 0

0 commit comments

Comments
 (0)