Skip to content

Commit 5ef231e

Browse files
committed
Make dead_decls_by_file reactive and remove report mutation
- Add reactive dead_decls_by_file collection (flatMap with merge) - Add shouldReport callback to reportDeclaration (replaces report field mutation) - Issue generation now uses reactive per-file grouping - No more decl.report mutation in reactive path All 380/19000 issues still match between reactive and non-reactive modes.
1 parent 05c83f9 commit 5ef231e

File tree

2 files changed

+52
-55
lines changed

2 files changed

+52
-55
lines changed

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -172,11 +172,17 @@ let make_hasRefBelow ~transitive ~iter_value_refs_from =
172172
(** Report a dead declaration. Returns list of issues (dead module first, then dead value).
173173
[hasRefBelow] checks if there are references from "below" the declaration.
174174
Only used when [config.run.transitive] is false.
175-
[?checkModuleDead] optional callback for checking dead modules. Defaults to DeadModules.checkModuleDead. *)
176-
let reportDeclaration ~config ~hasRefBelow ?checkModuleDead
175+
[?checkModuleDead] optional callback for checking dead modules. Defaults to DeadModules.checkModuleDead.
176+
[?shouldReport] optional callback to check if a decl should be reported. Defaults to checking decl.report. *)
177+
let reportDeclaration ~config ~hasRefBelow ?checkModuleDead ?shouldReport
177178
(ctx : ReportingContext.t) decl : Issue.t list =
178179
let insideReportedValue = decl |> isInsideReportedValue ctx in
179-
if not decl.report then []
180+
let should_report =
181+
match shouldReport with
182+
| Some f -> f decl
183+
| None -> decl.report
184+
in
185+
if not should_report then []
180186
else
181187
let deadWarning, message =
182188
match decl.declKind with

analysis/reanalyze/src/ReactiveSolver.ml

Lines changed: 43 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
11
(** Reactive dead code solver.
22
3-
Reactive pipeline: decls + live → dead_decls, live_decls, dead_modules
3+
Reactive pipeline: decls + live → dead_decls, live_decls, dead_modules, dead_decls_by_file
44
55
Current status:
6-
- dead_decls, live_decls, dead_modules are reactive (zero recomputation on cache hit)
6+
- dead_decls, live_decls, dead_modules, dead_decls_by_file are all reactive
77
- dead_modules = modules with dead decls but no live decls (reactive anti-join)
8-
- is_pos_live uses reactive live collection (no resolvedDead mutation needed)
9-
- Per-file issue generation: group by file, sort within file, fresh ReportingContext per file
8+
- dead_decls_by_file = dead decls grouped by file (reactive flatMap with merge)
9+
- is_pos_live uses reactive live collection (no resolvedDead mutation)
10+
- shouldReport callback replaces report field mutation (no mutation needed)
11+
- Per-file issue generation using reactive dead_decls_by_file
1012
- isInsideReportedValue is per-file only, so files are independent
1113
1214
TODO for fully reactive issues:
13-
- Make dead_decls_by_file a reactive collection (per-file grouping)
14-
- Make per-file issues reactive (only recompute changed files)
15+
- Make per-file issues a reactive collection (issues_by_file)
16+
Currently we iterate dead_decls_by_file to generate issues
1517
- hasRefBelow: uses O(total_refs) linear scan of refs_from per dead decl;
1618
could use reactive refs_to index for O(1) lookup per decl
17-
- report field: still mutated to suppress annotated decls; could check in reportDeclaration
1819
1920
All issues now match between reactive and non-reactive modes (380 on deadcode test):
2021
- Dead code issues: 362 (Exception:2, Module:31, Type:87, Value:233, ValueWithSideEffects:8)
@@ -30,6 +31,8 @@ type t = {
3031
value_refs_from: (Lexing.position, PosSet.t) Reactive.t option;
3132
dead_modules: (Name.t, Location.t) Reactive.t;
3233
(** Modules where all declarations are dead. Reactive anti-join. *)
34+
dead_decls_by_file: (string, Decl.t list) Reactive.t;
35+
(** Dead declarations grouped by file. Reactive per-file grouping. *)
3336
}
3437

3538
(** Extract module name from a declaration *)
@@ -92,7 +95,15 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
9295
()
9396
in
9497

95-
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules}
98+
(* Reactive per-file grouping of dead declarations *)
99+
let dead_decls_by_file =
100+
Reactive.flatMap dead_decls
101+
~f:(fun _pos decl -> [(decl.pos.Lexing.pos_fname, [decl])])
102+
~merge:(fun decls1 decls2 -> decls1 @ decls2)
103+
()
104+
in
105+
106+
{decls; live; dead_decls; live_decls; annotations; value_refs_from; dead_modules; dead_decls_by_file}
96107

97108
(** Check if a module is dead using reactive collection. Returns issue if dead.
98109
Uses reported_modules set to avoid duplicate reports. *)
@@ -124,26 +135,7 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
124135
(* Track reported modules to avoid duplicates *)
125136
let reported_modules = Hashtbl.create 64 in
126137

127-
(* Collect dead declarations - NO resolvedDead mutation *)
128-
let dead_list = ref [] in
129-
Reactive.iter
130-
(fun _pos (decl : Decl.t) ->
131-
(* Check annotation to decide if we report.
132-
Don't report if @live, @genType, or @dead (user knows it's dead) *)
133-
let should_report =
134-
match Reactive.get t.annotations decl.pos with
135-
| Some FileAnnotations.Live -> false
136-
| Some FileAnnotations.GenType -> false
137-
| Some FileAnnotations.Dead ->
138-
false (* @dead = user knows, don't warn *)
139-
| None -> true
140-
in
141-
if not should_report then decl.report <- false;
142-
dead_list := decl :: !dead_list)
143-
t.dead_decls;
144-
let t1 = Unix.gettimeofday () in
145-
146-
(* Check live declarations for incorrect @dead - NO resolvedDead mutation *)
138+
(* Check live declarations for incorrect @dead *)
147139
let incorrect_dead_issues = ref [] in
148140
Reactive.iter
149141
(fun _pos (decl : Decl.t) ->
@@ -161,19 +153,9 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
161153
incorrect_dead_issues := mod_issue :: !incorrect_dead_issues);
162154
incorrect_dead_issues := issue :: !incorrect_dead_issues))
163155
t.live_decls;
164-
let t2 = Unix.gettimeofday () in
156+
let t1 = Unix.gettimeofday () in
165157

166-
(* Group dead declarations by file *)
167-
let by_file : (string, Decl.t list) Hashtbl.t = Hashtbl.create 64 in
168-
List.iter
169-
(fun (decl : Decl.t) ->
170-
let file = decl.pos.pos_fname in
171-
let existing = Hashtbl.find_opt by_file file |> Option.value ~default:[] in
172-
Hashtbl.replace by_file file (decl :: existing))
173-
!dead_list;
174-
let t3 = Unix.gettimeofday () in
175-
176-
(* Generate issues per-file with independent ReportingContext.
158+
(* Generate issues per-file using reactive dead_decls_by_file.
177159
isInsideReportedValue only checks within same file, so files are independent. *)
178160
let transitive = config.DceConfig.run.transitive in
179161
let hasRefBelow =
@@ -188,33 +170,42 @@ let collect_issues ~(t : t) ~(config : DceConfig.t)
188170
check_module_dead ~dead_modules:t.dead_modules ~reported_modules ~fileName
189171
moduleName
190172
in
173+
(* Callback to check if we should report a dead decl (no mutation) *)
174+
let shouldReport (decl : Decl.t) =
175+
match Reactive.get t.annotations decl.pos with
176+
| Some FileAnnotations.Live -> false
177+
| Some FileAnnotations.GenType -> false
178+
| Some FileAnnotations.Dead -> false (* @dead = user knows, don't warn *)
179+
| None -> true
180+
in
181+
let num_files = ref 0 in
191182
let dead_issues =
192-
Hashtbl.fold
193-
(fun _file decls acc ->
194-
(* Sort within file for isInsideReportedValue *)
183+
let issues = ref [] in
184+
Reactive.iter
185+
(fun _file decls ->
186+
incr num_files;
187+
(* Sort within file for isInsideReportedValue - pass ALL decls for correct context *)
195188
let sorted = decls |> List.fast_sort Decl.compareForReporting in
196189
(* Fresh ReportingContext per file *)
197190
let reporting_ctx = DeadCommon.ReportingContext.create () in
198191
let file_issues =
199192
sorted
200193
|> List.concat_map (fun decl ->
201194
DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead
202-
reporting_ctx decl)
195+
~shouldReport reporting_ctx decl)
203196
in
204-
file_issues @ acc)
205-
by_file []
197+
issues := file_issues @ !issues)
198+
t.dead_decls_by_file;
199+
!issues
206200
in
207-
let t4 = Unix.gettimeofday () in
201+
let t2 = Unix.gettimeofday () in
208202

209203
if !Cli.timing then
210204
Printf.eprintf
211-
" collect_issues: iter_dead=%.2fms iter_live=%.2fms group=%.2fms \
212-
report=%.2fms (%d files)\n"
205+
" collect_issues: iter_live=%.2fms per_file=%.2fms (%d files)\n"
213206
((t1 -. t0) *. 1000.0)
214207
((t2 -. t1) *. 1000.0)
215-
((t3 -. t2) *. 1000.0)
216-
((t4 -. t3) *. 1000.0)
217-
(Hashtbl.length by_file);
208+
!num_files;
218209

219210
List.rev !incorrect_dead_issues @ dead_issues
220211

0 commit comments

Comments
 (0)