Skip to content

Commit 97cf8ae

Browse files
committed
Remove resolvedDead mutation - use reactive liveness check
- Add is_pos_live function to ReactiveSolver that uses reactive live collection - Store decls and live in ReactiveSolver.t for direct reactive lookup - Update Reanalyze.ml to use is_pos_live instead of Decl.isLive - Remove resolvedDead mutations from collect_issues Optional args now use reactive liveness check instead of mutable field. All 380 issues still match between reactive and non-reactive modes.
1 parent 96a122d commit 97cf8ae

File tree

3 files changed

+24
-15
lines changed

3 files changed

+24
-15
lines changed

analysis/reanalyze/src/ReactiveSolver.ml

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,22 +5,26 @@
55
Current status:
66
- dead_decls, live_decls, dead_modules are reactive (zero recomputation on cache hit)
77
- dead_modules = modules with dead decls but no live decls (reactive anti-join)
8-
- collect_issues still iterates dead_decls + live_decls to set resolvedDead
8+
- is_pos_live uses reactive live collection (no resolvedDead mutation needed)
9+
- collect_issues still iterates dead_decls + live_decls for annotations + sorting
910
- Uses DeadCommon.reportDeclaration for isInsideReportedValue and hasRefBelow
1011
1112
TODO for fully reactive issues:
1213
- isInsideReportedValue: needs reactive tracking of reported positions
1314
(currently relies on sequential iteration order via ReportingContext)
1415
- hasRefBelow: uses O(total_refs) linear scan of refs_from per dead decl;
1516
could use reactive refs_to index for O(1) lookup per decl
16-
- resolvedDead: still mutated on every call; could be computed on-demand
17+
- report field: still mutated to suppress annotated decls; could check in reportDeclaration
18+
- Sorting: O(n log n) for isInsideReportedValue ordering; fundamentally sequential
1719
1820
All issues now match between reactive and non-reactive modes (380 on deadcode test):
1921
- Dead code issues: 362 (Exception:2, Module:31, Type:87, Value:233, ValueWithSideEffects:8)
2022
- Incorrect @dead: 1
2123
- Optional args: 18 (Redundant:6, Unused:12) *)
2224

2325
type t = {
26+
decls: (Lexing.position, Decl.t) Reactive.t;
27+
live: (Lexing.position, unit) Reactive.t;
2428
dead_decls: (Lexing.position, Decl.t) Reactive.t;
2529
live_decls: (Lexing.position, Decl.t) Reactive.t;
2630
annotations: (Lexing.position, FileAnnotations.annotated_as) Reactive.t;
@@ -89,7 +93,7 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
8993
()
9094
in
9195

92-
{dead_decls; live_decls; annotations; value_refs_from; dead_modules}
96+
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules}
9397

9498
(** Check if a module is dead using reactive collection. Returns issue if dead.
9599
Uses reported_modules set to avoid duplicate reports. *)
@@ -121,11 +125,10 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
121125
(* Track reported modules to avoid duplicates *)
122126
let reported_modules = Hashtbl.create 64 in
123127

124-
(* Mark dead declarations and collect them (no DeadModules.markDead) *)
128+
(* Collect dead declarations - NO resolvedDead mutation *)
125129
let dead_list = ref [] in
126130
Reactive.iter
127131
(fun _pos (decl : Decl.t) ->
128-
decl.resolvedDead <- Some true;
129132
(* Check annotation to decide if we report.
130133
Don't report if @live, @genType, or @dead (user knows it's dead) *)
131134
let should_report =
@@ -141,11 +144,10 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
141144
t.dead_decls;
142145
let t1 = Unix.gettimeofday () in
143146

144-
(* Mark live declarations (no DeadModules.markLive) *)
147+
(* Check live declarations for incorrect @dead - NO resolvedDead mutation *)
145148
let incorrect_dead_issues = ref [] in
146149
Reactive.iter
147150
(fun _pos (decl : Decl.t) ->
148-
decl.resolvedDead <- Some false;
149151
(* Check for incorrect @dead annotation on live decl *)
150152
if AnnotationStore.is_annotated_dead ann_store decl.pos then (
151153
let issue =
@@ -204,6 +206,13 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
204206
let iter_live_decls ~(t : t) (f : Decl.t -> unit) : unit =
205207
Reactive.iter (fun _pos decl -> f decl) t.live_decls
206208

209+
(** Check if a position is live using the reactive collection.
210+
Returns true if pos is not a declaration (matches non-reactive behavior). *)
211+
let is_pos_live ~(t : t) (pos : Lexing.position) : bool =
212+
match Reactive.get t.decls pos with
213+
| None -> true (* not a declaration, assume live *)
214+
| Some _ -> Reactive.get t.live pos <> None
215+
207216
(** Stats *)
208217
let stats ~(t : t) : int * int =
209218
(Reactive.length t.dead_decls, Reactive.length t.live_decls)

analysis/reanalyze/src/ReactiveSolver.mli

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,9 @@ val collect_issues :
2222
val iter_live_decls : t:t -> (Decl.t -> unit) -> unit
2323
(** Iterate over live declarations *)
2424

25+
val is_pos_live : t:t -> Lexing.position -> bool
26+
(** Check if a position is live using the reactive collection.
27+
Returns true if pos is not a declaration (matches non-reactive behavior). *)
28+
2529
val stats : t:t -> int * int
2630
(** (dead, live) counts *)

analysis/reanalyze/src/Reanalyze.ml

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -383,14 +383,10 @@ let runAnalysis ~dce_config ~cmtRoot ~reactive_collection ~reactive_merge
383383
CrossFileItemsStore.of_reactive
384384
merged.ReactiveMerge.cross_file_items
385385
in
386-
(* Compute optional args state using declaration liveness
387-
Note: is_live returns true if pos is not a declaration (matches non-reactive)
388-
Uses Decl.isLive which checks resolvedDead set by collect_issues *)
389-
let is_live pos =
390-
match Reactive.get merged.ReactiveMerge.decls pos with
391-
| None -> true (* not a declaration, assume live *)
392-
| Some decl -> Decl.isLive decl
393-
in
386+
(* Compute optional args state using reactive liveness check.
387+
Uses ReactiveSolver.is_pos_live which checks the reactive live collection
388+
instead of mutable resolvedDead field. *)
389+
let is_live pos = ReactiveSolver.is_pos_live ~t:solver pos in
394390
let find_decl pos =
395391
Reactive.get merged.ReactiveMerge.decls pos
396392
in

0 commit comments

Comments
 (0)