Skip to content

Commit 90726f9

Browse files
committed
DCE: Tasks 5 & 6 - References and CrossFileItems patterns
Applies the map → list → merge pattern to references and cross-file items. ## Task 5: References New module: References.ml/.mli - builder (mutable) for AST processing - t (immutable) for solver - Tracks both value refs and type refs - PosSet for position sets Changes: - Thread ~refs:References.builder through AST processing - addValueReference, addTypeReference use References API - Solver uses References.find_value_refs, References.find_type_refs - Deleted global ValueReferences.table and TypeReferences.table ## Task 6: CrossFileItems (renamed from DelayedItems) New module: CrossFileItems.ml/.mli - builder (mutable) for AST processing - t (immutable) for processing after merge - Three item types: exception_refs, optional_arg_calls, function_refs Changes: - Thread ~cross_file:CrossFileItems.builder through AST processing - DeadException.markAsUsed adds to cross_file builder - DeadOptionalArgs.addReferences, addFunctionReference add to cross_file - Deleted global delayedItems refs from DeadException and DeadOptionalArgs ## Data flow process_cmt_file (per-file) → file_data { annotations; decls; refs; cross_file } Merge phase: FileAnnotations.merge_all → annotations (immutable) Declarations.merge_all → decls (immutable) CrossFileItems.merge_all → cross_file (immutable) References builders merged into refs_builder Process cross-file items: process_exception_refs → writes to refs_builder process_optional_args → reads decls Freeze: refs_builder → refs (immutable) Solver: reportDead ~annotations ~decls ~refs ## Global state deleted - DeadCommon.ValueReferences.table - DeadCommon.TypeReferences.table - DeadException.delayedItems - DeadOptionalArgs.delayedItems - DeadOptionalArgs.functionReferences ## Naming Renamed DelayedItems → CrossFileItems because it better describes the semantic meaning: items that reference across file boundaries.
1 parent ba64e1a commit 90726f9

13 files changed

+431
-148
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,11 @@ you can swap one file's data without affecting others.
8080

8181
**Impact**: Can't analyze a subset of files without reanalyzing everything. Can't clear state between test runs without module reloading.
8282

83-
### P3: Delayed/deferred processing queues
83+
### P3: Cross-file processing queues
8484
**Problem**: Several analyses use global queues that get "flushed" later:
85-
- `DeadOptionalArgs.delayedItems` - deferred optional arg analysis
86-
- `DeadException.delayedItems` - deferred exception checks
87-
- `DeadType.TypeDependencies.delayedItems` - deferred type deps
85+
- `DeadOptionalArgs.delayedItems` - cross-file optional arg analysis → DELETED (now `CrossFileItems`)
86+
- `DeadException.delayedItems` - cross-file exception checks → DELETED (now `CrossFileItems`)
87+
- `DeadType.TypeDependencies.delayedItems` - per-file type deps (already handled per-file)
8888
- `ProcessDeadAnnotations.positionsAnnotated` - annotation tracking
8989

9090
**Additional problem**: `positionsAnnotated` mixes **input** (source annotations from AST) with **output** (positions the solver determines are dead). The solver mutates this during analysis, violating purity.
@@ -346,29 +346,39 @@ val is_annotated_* : t -> ... -> bool
346346
**Pattern**: Same as Task 3/4.
347347

348348
**Changes**:
349-
- [ ] Create `References` module with `builder` and `t` types
350-
- [ ] `process_cmt_file` returns `References.builder` for both value and type refs
351-
- [ ] `References.merge_all : builder list -> t`
352-
- [ ] Delete global `ValueReferences.table` and `TypeReferences.table`
349+
- [x] Create `References` module with `builder` and `t` types
350+
- [x] Thread `~refs:References.builder` through `addValueReference`, `addTypeReference`
351+
- [x] `process_cmt_file` returns `References.builder` in `file_data`
352+
- [x] Merge refs into builder, process delayed items, then freeze
353+
- [x] Solver uses `References.t` via `find_value_refs` and `find_type_refs`
354+
- [x] Delete global `ValueReferences.table` and `TypeReferences.table`
355+
356+
**Status**: Complete ✅
353357

354358
**Test**: Process files in different orders - results should be identical.
355359

356360
**Estimated effort**: Medium (similar to Task 4)
357361

358-
### Task 6: Delayed items use map → list → merge pattern (P3)
362+
### Task 6: Cross-file items use map → list → merge pattern (P3)
359363

360-
**Value**: No global queues. Delayed items are per-file immutable data.
364+
**Value**: No global queues. Cross-file items are per-file immutable data.
361365

362366
**Pattern**: Same as Task 3/4/5.
363367

364368
**Changes**:
365-
- [ ] Create `DelayedItems` module with `builder` and `t` types
366-
- [ ] `process_cmt_file` returns `DelayedItems.builder`
367-
- [ ] `DelayedItems.merge_all : builder list -> t`
368-
- [ ] `forceDelayedItems` is pure function on `DelayedItems.t`
369-
- [ ] Delete global `delayedItems` refs
369+
- [x] Create `CrossFileItems` module with `builder` and `t` types
370+
- [x] Thread `~cross_file:CrossFileItems.builder` through AST processing
371+
- [x] `process_cmt_file` returns `CrossFileItems.builder` in `file_data`
372+
- [x] `CrossFileItems.merge_all : builder list -> t`
373+
- [x] `process_exception_refs` and `process_optional_args` are pure functions on merged `t`
374+
- [x] Delete global `delayedItems` refs from `DeadException` and `DeadOptionalArgs`
375+
376+
**Status**: Complete ✅
370377

371-
**Key insight**: "Delayed" items are just per-file data collected during AST processing.
378+
**Note**: `DeadType.TypeDependencies` was already per-file (processed within `process_cmt_file`),
379+
so it didn't need to be included.
380+
381+
**Key insight**: Cross-file items are references that span file boundaries.
372382
They should follow the same pattern as everything else.
373383

374384
**Test**: Process files in different orders - results should be identical.
@@ -496,6 +506,37 @@ This enables parallelization, caching, and incremental recomputation.
496506

497507
---
498508

509+
## Optional Future Tasks
510+
511+
### Optional Task: Make OptionalArgs tracking immutable
512+
513+
**Value**: Currently `CrossFileItems.process_optional_args` mutates `optionalArgs` inside declarations.
514+
Making this immutable would complete the pure pipeline.
515+
516+
**Current state**:
517+
- `OptionalArgs.t` inside `decl.declKind = Value {optionalArgs}` is mutable
518+
- `OptionalArgs.call` and `OptionalArgs.combine` mutate the record
519+
- This happens after merge but before solver
520+
521+
**Why it's acceptable now**:
522+
- Mutation happens in a well-defined phase (after merge, before solver)
523+
- Solver sees effectively immutable data
524+
- Order independence is maintained (calls accumulate, order doesn't matter)
525+
526+
**Changes needed**:
527+
- [ ] Make `OptionalArgs.t` an immutable data structure
528+
- [ ] Collect call info during AST processing as `OptionalArgCalls.builder`
529+
- [ ] Return calls from `process_cmt_file` in `file_data`
530+
- [ ] Merge all calls after file processing
531+
- [ ] Build final `OptionalArgs` state from merged calls (pure)
532+
- [ ] Store immutable `OptionalArgs` in declarations
533+
534+
**Estimated effort**: Medium-High (touches core data structures)
535+
536+
**Priority**: Low (current design works, just not fully pure)
537+
538+
---
539+
499540
## Success Criteria
500541

501542
After all tasks:
Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
(** Cross-file items collected during AST processing.
2+
3+
These are references that span file boundaries and need to be resolved
4+
after all files are processed. *)
5+
6+
open Common
7+
8+
(** {2 Item types} *)
9+
10+
type exception_ref = {exception_path: Path.t; loc_from: Location.t}
11+
12+
type optional_arg_call = {
13+
pos_to: Lexing.position;
14+
arg_names: string list;
15+
arg_names_maybe: string list;
16+
}
17+
18+
type function_ref = {pos_from: Lexing.position; pos_to: Lexing.position}
19+
20+
(** {2 Types} *)
21+
22+
type t = {
23+
exception_refs: exception_ref list;
24+
optional_arg_calls: optional_arg_call list;
25+
function_refs: function_ref list;
26+
}
27+
28+
type builder = {
29+
mutable exception_refs: exception_ref list;
30+
mutable optional_arg_calls: optional_arg_call list;
31+
mutable function_refs: function_ref list;
32+
}
33+
34+
(** {2 Builder API} *)
35+
36+
let create_builder () : builder =
37+
{exception_refs = []; optional_arg_calls = []; function_refs = []}
38+
39+
let add_exception_ref (b : builder) ~exception_path ~loc_from =
40+
b.exception_refs <- {exception_path; loc_from} :: b.exception_refs
41+
42+
let add_optional_arg_call (b : builder) ~pos_to ~arg_names ~arg_names_maybe =
43+
b.optional_arg_calls <-
44+
{pos_to; arg_names; arg_names_maybe} :: b.optional_arg_calls
45+
46+
let add_function_reference (b : builder) ~pos_from ~pos_to =
47+
b.function_refs <- {pos_from; pos_to} :: b.function_refs
48+
49+
(** {2 Merge API} *)
50+
51+
let merge_all (builders : builder list) : t =
52+
let exception_refs =
53+
builders |> List.concat_map (fun b -> b.exception_refs)
54+
in
55+
let optional_arg_calls =
56+
builders |> List.concat_map (fun b -> b.optional_arg_calls)
57+
in
58+
let function_refs = builders |> List.concat_map (fun b -> b.function_refs) in
59+
{exception_refs; optional_arg_calls; function_refs}
60+
61+
(** {2 Processing API} *)
62+
63+
let process_exception_refs (t : t) ~refs ~find_exception ~config =
64+
t.exception_refs
65+
|> List.iter (fun {exception_path; loc_from} ->
66+
match find_exception exception_path with
67+
| None -> ()
68+
| Some loc_to ->
69+
DeadCommon.addValueReference ~config ~refs ~binding:Location.none
70+
~addFileReference:true ~locFrom:loc_from ~locTo:loc_to)
71+
72+
let process_optional_args (t : t) ~decls =
73+
(* Process optional arg calls *)
74+
t.optional_arg_calls
75+
|> List.iter (fun {pos_to; arg_names; arg_names_maybe} ->
76+
match Declarations.find_opt decls pos_to with
77+
| Some {declKind = Value r} ->
78+
r.optionalArgs
79+
|> OptionalArgs.call ~argNames:arg_names
80+
~argNamesMaybe:arg_names_maybe
81+
| _ -> ());
82+
(* Process function references *)
83+
t.function_refs
84+
|> List.iter (fun {pos_from; pos_to} ->
85+
match
86+
( Declarations.find_opt decls pos_from,
87+
Declarations.find_opt decls pos_to )
88+
with
89+
| Some {declKind = Value rFrom}, Some {declKind = Value rTo}
90+
when not (OptionalArgs.isEmpty rTo.optionalArgs) ->
91+
OptionalArgs.combine rFrom.optionalArgs rTo.optionalArgs
92+
| _ -> ())
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
(** Cross-file items collected during AST processing.
2+
3+
These are references that span file boundaries and need to be resolved
4+
after all files are processed. Two types are provided:
5+
- [builder] - mutable, for AST processing
6+
- [t] - immutable, for processing after merge *)
7+
8+
(** {2 Types} *)
9+
10+
type t
11+
(** Immutable cross-file items - for processing after merge *)
12+
13+
type builder
14+
(** Mutable builder - for AST processing *)
15+
16+
(** {2 Builder API - for AST processing} *)
17+
18+
val create_builder : unit -> builder
19+
20+
val add_exception_ref :
21+
builder -> exception_path:Common.Path.t -> loc_from:Location.t -> unit
22+
(** Add a cross-file exception reference (defined in another file). *)
23+
24+
val add_optional_arg_call :
25+
builder ->
26+
pos_to:Lexing.position ->
27+
arg_names:string list ->
28+
arg_names_maybe:string list ->
29+
unit
30+
(** Add a cross-file optional argument call. *)
31+
32+
val add_function_reference :
33+
builder -> pos_from:Lexing.position -> pos_to:Lexing.position -> unit
34+
(** Add a cross-file function reference (for optional args combining). *)
35+
36+
(** {2 Merge API} *)
37+
38+
val merge_all : builder list -> t
39+
(** Merge all builders into one immutable result. Order doesn't matter. *)
40+
41+
(** {2 Processing API - for after merge} *)
42+
43+
val process_exception_refs :
44+
t ->
45+
refs:References.builder ->
46+
find_exception:(Common.Path.t -> Location.t option) ->
47+
config:DceConfig.t ->
48+
unit
49+
(** Process cross-file exception references. *)
50+
51+
val process_optional_args : t -> decls:Declarations.t -> unit
52+
(** Process cross-file optional argument calls and function references. *)

analysis/reanalyze/src/DceFileProcessing.ml

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,8 @@ let processSignature ~config ~decls ~(file : file_context) ~doValues ~doTypes
4040
type file_data = {
4141
annotations: FileAnnotations.builder;
4242
decls: Declarations.builder;
43+
refs: References.builder;
44+
cross_file: CrossFileItems.builder;
4345
}
4446

4547
let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
@@ -55,6 +57,8 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
5557
(* Mutable builders for AST processing *)
5658
let annotations = FileAnnotations.create_builder () in
5759
let decls = Declarations.create_builder () in
60+
let refs = References.create_builder () in
61+
let cross_file = CrossFileItems.create_builder () in
5862
(match cmt_infos.cmt_annots with
5963
| Interface signature ->
6064
CollectAnnotations.signature ~state:annotations ~config signature;
@@ -69,11 +73,11 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
6973
processSignature ~config ~decls ~file ~doValues:true ~doTypes:false
7074
structure.str_type;
7175
let doExternals = false in
72-
DeadValue.processStructure ~config ~decls ~file:dead_common_file
73-
~doTypes:true ~doExternals
76+
DeadValue.processStructure ~config ~decls ~refs ~cross_file
77+
~file:dead_common_file ~doTypes:true ~doExternals
7478
~cmt_value_dependencies:cmt_infos.cmt_value_dependencies structure
7579
| _ -> ());
76-
DeadType.TypeDependencies.forceDelayedItems ~config;
80+
DeadType.TypeDependencies.forceDelayedItems ~config ~refs;
7781
DeadType.TypeDependencies.clear ();
7882
(* Return builders - caller will merge and freeze *)
79-
{annotations; decls}
83+
{annotations; decls; refs; cross_file}

analysis/reanalyze/src/DceFileProcessing.mli

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,10 @@ type file_context = {
1414
type file_data = {
1515
annotations: FileAnnotations.builder;
1616
decls: Declarations.builder;
17+
refs: References.builder;
18+
cross_file: CrossFileItems.builder;
1719
}
18-
(** Result of processing a cmt file - both annotations and declarations *)
20+
(** Result of processing a cmt file - annotations, declarations, references, and delayed items *)
1921

2022
val process_cmt_file :
2123
config:DceConfig.t ->

0 commit comments

Comments
 (0)