Skip to content

Commit feb98a0

Browse files
committed
DCE: Task 7 - FileDeps pattern for file dependencies
Applies the map → list → merge pattern to file dependencies. ## New module: FileDeps.ml/.mli - `builder` (mutable) for AST processing - `t` (immutable) for solver - `add_file`: Register a file as existing - `add_dep`: Add a dependency from one file to another - `merge_all`: Merge all builders into immutable result - `iter_files_from_roots_to_leaves`: Pure topological ordering ## Changes - Thread `~file_deps:FileDeps.builder` through AST processing - `addValueReference` records cross-file dependencies to builder - `process_cmt_file` returns `file_deps` in `file_data` - `reportDead` takes `~file_deps:FileDeps.t` (immutable) - Moved topological sort from DeadCommon to FileDeps (pure function) ## Global state deleted - `Common.FileReferences.table` - replaced by per-file FileDeps builders ## Data flow process_cmt_file (per-file) → file_data { ..., file_deps: builder } Merge phase: FileDeps builders merged for cross-file items Freeze: file_deps_builder → file_deps (immutable) Solver: reportDead ~file_deps (uses iter_files_from_roots_to_leaves)
1 parent 90726f9 commit feb98a0

File tree

12 files changed

+268
-144
lines changed

12 files changed

+268
-144
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -392,11 +392,14 @@ They should follow the same pattern as everything else.
392392
**Pattern**: Same as Task 3/4/5/6.
393393

394394
**Changes**:
395-
- [ ] Create `FileDeps` module with `builder` and `t` types
396-
- [ ] `process_cmt_file` returns `FileDeps.builder`
397-
- [ ] `FileDeps.merge_all : builder list -> FileGraph.t`
398-
- [ ] `topological_order : FileGraph.t -> string list` (pure function)
399-
- [ ] `DeadModules` state becomes part of per-file data
395+
- [x] Create `FileDeps` module with `builder` and `t` types
396+
- [x] `process_cmt_file` returns `FileDeps.builder`
397+
- [x] `FileDeps.merge_all : builder list -> t`
398+
- [x] Thread `~file_deps` through `addValueReference`
399+
- [x] `iter_files_from_roots_to_leaves : t -> (string -> unit) -> unit` (pure function)
400+
- [x] Delete global `FileReferences` from `Common.ml`
401+
402+
**Status**: Complete ✅
400403

401404
**Test**: Build file graph, verify topological ordering is correct.
402405

@@ -414,6 +417,7 @@ Can be parallelized, memoized, reordered.
414417
- [ ] `Decl.report`: Return `issue` instead of logging
415418
- [ ] Remove all `Log_.warning`, `Log_.item` calls from analysis path
416419
- [ ] Side effects (logging, JSON) only in final reporting phase
420+
- [ ] Make `DeadModules` state part of `analysis_result` (currently mutated during solver)
417421

418422
**Architecture**:
419423
```

analysis/reanalyze/src/Common.ml

Lines changed: 1 addition & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -49,31 +49,7 @@ module FileHash = struct
4949
end)
5050
end
5151

52-
module FileReferences = struct
53-
(* references across files *)
54-
let table = (FileHash.create 256 : FileSet.t FileHash.t)
55-
56-
let findSet table key =
57-
try FileHash.find table key with Not_found -> FileSet.empty
58-
59-
let add (locFrom : Location.t) (locTo : Location.t) =
60-
let key = locFrom.loc_start.pos_fname in
61-
let set = findSet table key in
62-
FileHash.replace table key (FileSet.add locTo.loc_start.pos_fname set)
63-
64-
let addFile fileName =
65-
let set = findSet table fileName in
66-
FileHash.replace table fileName set
67-
68-
let exists fileName = FileHash.mem table fileName
69-
70-
let find fileName =
71-
match FileHash.find_opt table fileName with
72-
| Some set -> set
73-
| None -> FileSet.empty
74-
75-
let iter f = FileHash.iter f table
76-
end
52+
(* NOTE: FileReferences has been moved to FileDeps module *)
7753

7854
module Path = struct
7955
type t = Name.t list

analysis/reanalyze/src/CrossFileItems.ml

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,14 +60,15 @@ let merge_all (builders : builder list) : t =
6060

6161
(** {2 Processing API} *)
6262

63-
let process_exception_refs (t : t) ~refs ~find_exception ~config =
63+
let process_exception_refs (t : t) ~refs ~file_deps ~find_exception ~config =
6464
t.exception_refs
6565
|> List.iter (fun {exception_path; loc_from} ->
6666
match find_exception exception_path with
6767
| None -> ()
6868
| Some loc_to ->
69-
DeadCommon.addValueReference ~config ~refs ~binding:Location.none
70-
~addFileReference:true ~locFrom:loc_from ~locTo:loc_to)
69+
DeadCommon.addValueReference ~config ~refs ~file_deps
70+
~binding:Location.none ~addFileReference:true ~locFrom:loc_from
71+
~locTo:loc_to)
7172

7273
let process_optional_args (t : t) ~decls =
7374
(* Process optional arg calls *)

analysis/reanalyze/src/CrossFileItems.mli

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ val merge_all : builder list -> t
4343
val process_exception_refs :
4444
t ->
4545
refs:References.builder ->
46+
file_deps:FileDeps.builder ->
4647
find_exception:(Common.Path.t -> Location.t option) ->
4748
config:DceConfig.t ->
4849
unit

analysis/reanalyze/src/DceFileProcessing.ml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ type file_data = {
4242
decls: Declarations.builder;
4343
refs: References.builder;
4444
cross_file: CrossFileItems.builder;
45+
file_deps: FileDeps.builder;
4546
}
4647

4748
let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
@@ -59,6 +60,9 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
5960
let decls = Declarations.create_builder () in
6061
let refs = References.create_builder () in
6162
let cross_file = CrossFileItems.create_builder () in
63+
let file_deps = FileDeps.create_builder () in
64+
(* Register this file *)
65+
FileDeps.add_file file_deps file.source_path;
6266
(match cmt_infos.cmt_annots with
6367
| Interface signature ->
6468
CollectAnnotations.signature ~state:annotations ~config signature;
@@ -73,11 +77,11 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
7377
processSignature ~config ~decls ~file ~doValues:true ~doTypes:false
7478
structure.str_type;
7579
let doExternals = false in
76-
DeadValue.processStructure ~config ~decls ~refs ~cross_file
80+
DeadValue.processStructure ~config ~decls ~refs ~file_deps ~cross_file
7781
~file:dead_common_file ~doTypes:true ~doExternals
7882
~cmt_value_dependencies:cmt_infos.cmt_value_dependencies structure
7983
| _ -> ());
8084
DeadType.TypeDependencies.forceDelayedItems ~config ~refs;
8185
DeadType.TypeDependencies.clear ();
8286
(* Return builders - caller will merge and freeze *)
83-
{annotations; decls; refs; cross_file}
87+
{annotations; decls; refs; cross_file; file_deps}

analysis/reanalyze/src/DceFileProcessing.mli

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,9 @@ type file_data = {
1616
decls: Declarations.builder;
1717
refs: References.builder;
1818
cross_file: CrossFileItems.builder;
19+
file_deps: FileDeps.builder;
1920
}
20-
(** Result of processing a cmt file - annotations, declarations, references, and delayed items *)
21+
(** Result of processing a cmt file - annotations, declarations, references, cross-file items, and file dependencies *)
2122

2223
val process_cmt_file :
2324
config:DceConfig.t ->

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 13 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ let declGetLoc decl =
8787
in
8888
{Location.loc_start; loc_end = decl.posEnd; loc_ghost = false}
8989

90-
let addValueReference ~config ~refs ~(binding : Location.t) ~addFileReference
91-
~(locFrom : Location.t) ~(locTo : Location.t) : unit =
90+
let addValueReference ~config ~refs ~file_deps ~(binding : Location.t)
91+
~addFileReference ~(locFrom : Location.t) ~(locTo : Location.t) : unit =
9292
let effectiveFrom = if binding = Location.none then locFrom else binding in
9393
if not effectiveFrom.loc_ghost then (
9494
if config.DceConfig.cli.debug then
@@ -101,82 +101,14 @@ let addValueReference ~config ~refs ~(binding : Location.t) ~addFileReference
101101
addFileReference && (not locTo.loc_ghost)
102102
&& (not effectiveFrom.loc_ghost)
103103
&& effectiveFrom.loc_start.pos_fname <> locTo.loc_start.pos_fname
104-
then FileReferences.add effectiveFrom locTo)
105-
106-
let iterFilesFromRootsToLeaves iterFun =
107-
(* For each file, the number of incoming references *)
108-
let inverseReferences = (Hashtbl.create 1 : (string, int) Hashtbl.t) in
109-
(* For each number of incoming references, the files *)
110-
let referencesByNumber = (Hashtbl.create 1 : (int, FileSet.t) Hashtbl.t) in
111-
let getNum fileName =
112-
try Hashtbl.find inverseReferences fileName with Not_found -> 0
113-
in
114-
let getSet num =
115-
try Hashtbl.find referencesByNumber num with Not_found -> FileSet.empty
116-
in
117-
let addIncomingEdge fileName =
118-
let oldNum = getNum fileName in
119-
let newNum = oldNum + 1 in
120-
let oldSetAtNum = getSet oldNum in
121-
let newSetAtNum = FileSet.remove fileName oldSetAtNum in
122-
let oldSetAtNewNum = getSet newNum in
123-
let newSetAtNewNum = FileSet.add fileName oldSetAtNewNum in
124-
Hashtbl.replace inverseReferences fileName newNum;
125-
Hashtbl.replace referencesByNumber oldNum newSetAtNum;
126-
Hashtbl.replace referencesByNumber newNum newSetAtNewNum
127-
in
128-
let removeIncomingEdge fileName =
129-
let oldNum = getNum fileName in
130-
let newNum = oldNum - 1 in
131-
let oldSetAtNum = getSet oldNum in
132-
let newSetAtNum = FileSet.remove fileName oldSetAtNum in
133-
let oldSetAtNewNum = getSet newNum in
134-
let newSetAtNewNum = FileSet.add fileName oldSetAtNewNum in
135-
Hashtbl.replace inverseReferences fileName newNum;
136-
Hashtbl.replace referencesByNumber oldNum newSetAtNum;
137-
Hashtbl.replace referencesByNumber newNum newSetAtNewNum
138-
in
139-
let addEdge fromFile toFile =
140-
if FileReferences.exists fromFile then addIncomingEdge toFile
141-
in
142-
let removeEdge fromFile toFile =
143-
if FileReferences.exists fromFile then removeIncomingEdge toFile
144-
in
145-
FileReferences.iter (fun fromFile set ->
146-
if getNum fromFile = 0 then
147-
Hashtbl.replace referencesByNumber 0 (FileSet.add fromFile (getSet 0));
148-
set |> FileSet.iter (fun toFile -> addEdge fromFile toFile));
149-
while getSet 0 <> FileSet.empty do
150-
let filesWithNoIncomingReferences = getSet 0 in
151-
Hashtbl.remove referencesByNumber 0;
152-
filesWithNoIncomingReferences
153-
|> FileSet.iter (fun fileName ->
154-
iterFun fileName;
155-
let references = FileReferences.find fileName in
156-
references |> FileSet.iter (fun toFile -> removeEdge fileName toFile))
157-
done;
158-
(* Process any remaining items in case of circular references *)
159-
referencesByNumber
160-
|> Hashtbl.iter (fun _num set ->
161-
if FileSet.is_empty set then ()
162-
else
163-
set
164-
|> FileSet.iter (fun fileName ->
165-
let pos = {Lexing.dummy_pos with pos_fname = fileName} in
166-
let loc =
167-
{Location.none with loc_start = pos; loc_end = pos}
168-
in
169-
if Config.warnOnCircularDependencies then
170-
Log_.warning ~loc
171-
(Circular
172-
{
173-
message =
174-
Format.asprintf
175-
"Results for %s could be inaccurate because of \
176-
circular references"
177-
fileName;
178-
});
179-
iterFun fileName))
104+
then
105+
FileDeps.add_dep file_deps ~from_file:effectiveFrom.loc_start.pos_fname
106+
~to_file:locTo.loc_start.pos_fname)
107+
108+
(* NOTE: iterFilesFromRootsToLeaves moved to FileDeps.iter_files_from_roots_to_leaves *)
109+
110+
let iterFilesFromRootsToLeaves ~file_deps iterFun =
111+
FileDeps.iter_files_from_roots_to_leaves file_deps iterFun
180112

181113
let addDeclaration_ ~config ~decls ~(file : FileContext.t) ?posEnd ?posStart
182114
~declKind ~path ~(loc : Location.t) ?(posAdjustment = Nothing) ~moduleLoc
@@ -498,7 +430,7 @@ let rec resolveRecursiveRefs ~all_refs ~annotations ~config ~decls
498430
refsString level);
499431
isDead
500432

501-
let reportDead ~annotations ~config ~decls ~refs
433+
let reportDead ~annotations ~config ~decls ~refs ~file_deps
502434
~checkOptionalArg:
503435
(checkOptionalArgFn :
504436
annotations:FileAnnotations.t -> config:DceConfig.t -> decl -> unit) =
@@ -517,7 +449,7 @@ let reportDead ~annotations ~config ~decls ~refs
517449
if config.DceConfig.cli.debug then (
518450
Log_.item "@.File References@.@.";
519451
let fileList = ref [] in
520-
FileReferences.iter (fun file files ->
452+
FileDeps.iter_deps file_deps (fun file files ->
521453
fileList := (file, files) :: !fileList);
522454
!fileList
523455
|> List.sort (fun (f1, _) (f2, _) -> String.compare f1 f2)
@@ -532,7 +464,7 @@ let reportDead ~annotations ~config ~decls ~refs
532464
decls []
533465
in
534466
let orderedFiles = Hashtbl.create 256 in
535-
iterFilesFromRootsToLeaves
467+
iterFilesFromRootsToLeaves ~file_deps
536468
(let current = ref 0 in
537469
fun fileName ->
538470
incr current;

analysis/reanalyze/src/DeadException.ml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ let add ~config ~decls ~file ~path ~loc ~(strLoc : Location.t) name =
1313

1414
let find_exception path = Hashtbl.find_opt declarations path
1515

16-
let markAsUsed ~config ~refs ~cross_file ~(binding : Location.t)
16+
let markAsUsed ~config ~refs ~file_deps ~cross_file ~(binding : Location.t)
1717
~(locFrom : Location.t) ~(locTo : Location.t) path_ =
1818
if locTo.loc_ghost then
1919
(* Probably defined in another file, delay processing and check at the end *)
@@ -23,5 +23,5 @@ let markAsUsed ~config ~refs ~cross_file ~(binding : Location.t)
2323
CrossFileItems.add_exception_ref cross_file ~exception_path:exceptionPath
2424
~loc_from:locFrom
2525
else
26-
addValueReference ~config ~refs ~binding ~addFileReference:true ~locFrom
27-
~locTo
26+
addValueReference ~config ~refs ~file_deps ~binding ~addFileReference:true
27+
~locFrom ~locTo

analysis/reanalyze/src/DeadValue.ml

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ let processOptionalArgs ~config ~cross_file ~expType ~(locFrom : Location.t)
109109
(!supplied, !suppliedMaybe)
110110
|> DeadOptionalArgs.addReferences ~config ~cross_file ~locFrom ~locTo ~path)
111111

112-
let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
113-
self (e : Typedtree.expression) =
112+
let rec collectExpr ~config ~refs ~file_deps ~cross_file
113+
~(last_binding : Location.t) super self (e : Typedtree.expression) =
114114
let locFrom = e.exp_loc in
115115
let binding = last_binding in
116116
(match e.exp_desc with
@@ -126,8 +126,8 @@ let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
126126
References.add_value_ref refs ~posTo:locTo.loc_start
127127
~posFrom:Location.none.loc_start)
128128
else
129-
addValueReference ~config ~refs ~binding ~addFileReference:true ~locFrom
130-
~locTo
129+
addValueReference ~config ~refs ~file_deps ~binding ~addFileReference:true
130+
~locFrom ~locTo
131131
| Texp_apply
132132
{
133133
funct =
@@ -195,8 +195,8 @@ let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
195195
(match cstr_tag with
196196
| Cstr_extension path ->
197197
path
198-
|> DeadException.markAsUsed ~config ~refs ~cross_file ~binding ~locFrom
199-
~locTo
198+
|> DeadException.markAsUsed ~config ~refs ~file_deps ~cross_file ~binding
199+
~locFrom ~locTo
200200
| _ -> ());
201201
if !Config.analyzeTypes && not loc_ghost then
202202
DeadType.addTypeReference ~config ~refs ~posTo ~posFrom:locFrom.loc_start
@@ -208,7 +208,8 @@ let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
208208
->
209209
(* Punned field in OCaml projects has ghost location in expression *)
210210
let e = {e with exp_loc = {exp_loc with loc_ghost = false}} in
211-
collectExpr ~config ~refs ~cross_file ~last_binding super self e
211+
collectExpr ~config ~refs ~file_deps ~cross_file ~last_binding
212+
super self e
212213
|> ignore
213214
| _ -> ())
214215
| _ -> ());
@@ -294,7 +295,7 @@ let rec processSignatureItem ~config ~decls ~file ~doTypes ~doValues ~moduleLoc
294295
ModulePath.setCurrent oldModulePath
295296

296297
(* Traverse the AST *)
297-
let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
298+
let traverseStructure ~config ~decls ~refs ~file_deps ~cross_file ~file ~doTypes
298299
~doExternals (structure : Typedtree.structure) : unit =
299300
let rec create_mapper (last_binding : Location.t) =
300301
let super = Tast_mapper.default in
@@ -304,7 +305,8 @@ let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
304305
expr =
305306
(fun _self e ->
306307
e
307-
|> collectExpr ~config ~refs ~cross_file ~last_binding super mapper);
308+
|> collectExpr ~config ~refs ~file_deps ~cross_file ~last_binding
309+
super mapper);
308310
pat = (fun _self p -> p |> collectPattern ~config ~refs super mapper);
309311
structure_item =
310312
(fun _self (structureItem : Typedtree.structure_item) ->
@@ -408,7 +410,7 @@ let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
408410
mapper.structure mapper structure |> ignore
409411

410412
(* Merge a location's references to another one's *)
411-
let processValueDependency ~config ~decls ~refs ~cross_file
413+
let processValueDependency ~config ~decls ~refs ~file_deps ~cross_file
412414
( ({
413415
val_loc =
414416
{loc_start = {pos_fname = fnTo} as posTo; loc_ghost = ghost1} as
@@ -423,16 +425,17 @@ let processValueDependency ~config ~decls ~refs ~cross_file
423425
Types.value_description) ) =
424426
if (not ghost1) && (not ghost2) && posTo <> posFrom then (
425427
let addFileReference = fileIsImplementationOf fnTo fnFrom in
426-
addValueReference ~config ~refs ~binding:Location.none ~addFileReference
427-
~locFrom ~locTo;
428+
addValueReference ~config ~refs ~file_deps ~binding:Location.none
429+
~addFileReference ~locFrom ~locTo;
428430
DeadOptionalArgs.addFunctionReference ~config ~decls ~cross_file ~locFrom
429431
~locTo)
430432

431-
let processStructure ~config ~decls ~refs ~cross_file ~file
433+
let processStructure ~config ~decls ~refs ~file_deps ~cross_file ~file
432434
~cmt_value_dependencies ~doTypes ~doExternals
433435
(structure : Typedtree.structure) =
434-
traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes ~doExternals
435-
structure;
436+
traverseStructure ~config ~decls ~refs ~file_deps ~cross_file ~file ~doTypes
437+
~doExternals structure;
436438
let valueDependencies = cmt_value_dependencies |> List.rev in
437439
valueDependencies
438-
|> List.iter (processValueDependency ~config ~decls ~refs ~cross_file)
440+
|> List.iter
441+
(processValueDependency ~config ~decls ~refs ~file_deps ~cross_file)

0 commit comments

Comments
 (0)