Skip to content

Commit 58d4736

Browse files
cknittcknitt
committed
fixes
1 parent 4389e9c commit 58d4736

7 files changed

Lines changed: 60 additions & 24 deletions

File tree

‎analysis/reanalyze/src/SideEffects.ml‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,7 @@ let rec exprNoSideEffects (expr : Typedtree.expression) =
6262
| Texp_for (_id, _pat, e1, e2, _dir, e3) ->
6363
e1 |> exprNoSideEffects && e2 |> exprNoSideEffects
6464
&& e3 |> exprNoSideEffects
65-
| Texp_for_of (_id, _pat, e1, e2) ->
66-
e1 |> exprNoSideEffects && e2 |> exprNoSideEffects
67-
| Texp_for_await_of (_id, _pat, e1, e2) ->
68-
e1 |> exprNoSideEffects && e2 |> exprNoSideEffects
65+
| Texp_for_of _ | Texp_for_await_of _ -> false
6966
| Texp_send _ -> false
7067
| Texp_letexception (_ec, e) -> e |> exprNoSideEffects
7168
| Texp_pack _ -> false

‎compiler/core/js_analyzer.ml‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,9 @@ let no_side_effect_obj =
135135
| Throw _ | Debugger | Break _ | Variable _ | Continue _ ->
136136
raise_notrace Not_found
137137
| Exp e -> self.expression self e
138-
| Int_switch _ | String_switch _ | ForRange _ | ForOf _ | ForAwaitOf _
139-
| If _ | While _ | Block _ | Return _ | Try _ ->
138+
| ForOf _ | ForAwaitOf _ -> raise_notrace Not_found
139+
| Int_switch _ | String_switch _ | ForRange _ | If _ | While _ | Block _
140+
| Return _ | Try _ ->
140141
super.statement self s);
141142
expression =
142143
(fun _ s ->

‎tests/analysis_tests/tests-reanalyze/deadcode/expected/deadcode.txt‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -352,17 +352,16 @@
352352
addValueDeclaration +r FirstClassModulesInterface.resi:7:0 path:FirstClassModulesInterface
353353
Scanning ForAwaitOf.cmt Source:ForAwaitOf.res
354354
addValueDeclaration +keep ForAwaitOf.res:2:4 path:+ForAwaitOf
355-
addValueDeclaration +sideEffects ForAwaitOf.res:8:4 path:+ForAwaitOf
355+
addValueDeclaration +sideEffects ForAwaitOf.res:10:4 path:+ForAwaitOf
356356
addValueReference ForAwaitOf.res:2:4 --> ForAwaitOf.res:3:2
357357
addValueReference ForAwaitOf.res:2:4 --> ForAwaitOf.res:2:18
358-
addValueDeclaration +asyncIterable ForAwaitOf.res:9:6 path:+ForAwaitOf
359-
addValueReference ForAwaitOf.res:9:6 --> ForAwaitOf.res:9:44
360-
addValueReference ForAwaitOf.res:8:4 --> ForAwaitOf.res:13:2
361-
addValueReference ForAwaitOf.res:8:4 --> ForAwaitOf.res:9:6
358+
addValueReference ForAwaitOf.res:10:4 --> ForAwaitOf.res:8:0
362359
Scanning ForOf.cmt Source:ForOf.res
363360
addValueDeclaration +keep ForOf.res:2:4 path:+ForOf
361+
addValueDeclaration +sideEffects ForOf.res:10:4 path:+ForOf
364362
addValueReference ForOf.res:2:4 --> ForOf.res:3:2
365363
addValueReference ForOf.res:2:4 --> ForOf.res:2:11
364+
addValueReference ForOf.res:10:4 --> ForOf.res:8:0
366365
Scanning Hooks.cmt Source:Hooks.res
367366
addValueDeclaration +make Hooks.res:4:4 path:+Hooks
368367
addValueDeclaration +default Hooks.res:25:4 path:+Hooks
@@ -1947,7 +1946,7 @@ Forward Liveness Analysis
19471946

19481947
decls: 700
19491948
roots(external targets): 134
1950-
decl-deps: decls_with_out=411 edges_to_decls=289
1949+
decl-deps: decls_with_out=411 edges_to_decls=288
19511950

19521951
Root (annotated): Value +Hooks.+default
19531952
Root (external ref): Value +FirstClassModules.M.InnerModule2.+k
@@ -2753,12 +2752,8 @@ Forward Liveness Analysis
27532752
-> +FirstClassModulesInterface.+r
27542753
Live (annotated) Value +ForAwaitOf.+keep
27552754
Dead Value +ForAwaitOf.+sideEffects
2756-
deps: in=0 (live=0 dead=0) out=1
2757-
-> +ForAwaitOf.+asyncIterable
2758-
Dead Value +ForAwaitOf.+asyncIterable
2759-
deps: in=1 (live=0 dead=1) out=0
2760-
<- +ForAwaitOf.+sideEffects (dead)
27612755
Live (annotated) Value +ForOf.+keep
2756+
Dead Value +ForOf.+sideEffects
27622757
Live (external ref) RecordLabel +Hooks.vehicle.name
27632758
deps: in=4 (live=4 dead=0) out=0
27642759
<- +Hooks.+make (live)
@@ -4249,7 +4244,11 @@ Forward Liveness Analysis
42494244
r is never used
42504245

42514246
Warning Dead Value With Side Effects
4252-
ForAwaitOf.res:8:1-174
4247+
ForAwaitOf.res:10:1-69
4248+
sideEffects is never used and could have side effects
4249+
4250+
Warning Dead Value With Side Effects
4251+
ForOf.res:10:1-54
42534252
sideEffects is never used and could have side effects
42544253

42554254
Warning Dead Type
@@ -5256,4 +5255,4 @@ Forward Liveness Analysis
52565255
OptArg.res:26:1-70
52575256
optional argument c of function wrapfourArgs is always supplied (2 calls)
52585257

5259-
Analysis reported 319 issues (Incorrect Dead Annotation:1, Warning Dead Exception:2, Warning Dead Module:22, Warning Dead Type:93, Warning Dead Value:179, Warning Dead Value With Side Effects:4, Warning Redundant Optional Argument:6, Warning Unused Argument:12)
5258+
Analysis reported 320 issues (Incorrect Dead Annotation:1, Warning Dead Exception:2, Warning Dead Module:22, Warning Dead Type:93, Warning Dead Value:179, Warning Dead Value With Side Effects:5, Warning Redundant Optional Argument:6, Warning Unused Argument:12)

‎tests/analysis_tests/tests-reanalyze/deadcode/src/ForAwaitOf.res‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,11 @@ let keep = async (xs: AsyncIterable.t<int>) => {
55
}
66
}
77

8-
let sideEffects = {
9-
let asyncIterable: AsyncIterable.t<int> = %raw(`(async function* () {
10-
yield 1
11-
})()`)
8+
@val external asyncIterable: AsyncIterable.t<int> = "asyncIterable"
129

10+
let sideEffects = {
11+
// Keep the body pure so the loop's effect comes only from iteration itself.
1312
for await value of asyncIterable {
14-
Js.log(value)
13+
()
1514
}
1615
}

‎tests/analysis_tests/tests-reanalyze/deadcode/src/ForOf.res‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,3 +4,12 @@ let keep = xs => {
44
ignore(x)
55
}
66
}
7+
8+
@val external iterable: Iterable.t<int> = "iterable"
9+
10+
let sideEffects = {
11+
// Keep the body pure so the loop's effect comes only from iteration itself.
12+
for _ of iterable {
13+
()
14+
}
15+
}

‎tests/ounit_tests/ounit_js_analyzer_tests.ml‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
let ( >:: ), ( >::: ) = OUnit.(( >:: ), ( >::: ))
2+
3+
let pure_iterable = Js_exp_make.var (Ident.create "iterable")
4+
let empty_body = []
5+
6+
let for_of_statement =
7+
{
8+
J.statement_desc =
9+
ForOf (None, Ident.create "_for_of", pure_iterable, empty_body);
10+
comment = None;
11+
}
12+
13+
let for_await_of_statement =
14+
{
15+
J.statement_desc =
16+
ForAwaitOf (None, Ident.create "_for_await_of", pure_iterable, empty_body);
17+
comment = None;
18+
}
19+
20+
let suites =
21+
__FILE__
22+
>::: [
23+
( __LOC__ >:: fun _ ->
24+
OUnit.assert_bool __LOC__
25+
(not (Js_analyzer.no_side_effect_statement for_of_statement)) );
26+
( __LOC__ >:: fun _ ->
27+
OUnit.assert_bool __LOC__
28+
(not (Js_analyzer.no_side_effect_statement for_await_of_statement))
29+
);
30+
]

‎tests/ounit_tests/ounit_tests_main.ml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ let suites =
1818
Ounit_utf8_test.suites;
1919
Ounit_unicode_tests.suites;
2020
Ounit_util_tests.suites;
21+
Ounit_js_analyzer_tests.suites;
2122
Ounit_jsx_loc_tests.suites;
2223
]
2324

0 commit comments

Comments
 (0)