DCE: Task 7 - FileDeps pattern for file dependencies

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

@@ -392,11 +392,14 @@ They should follow the same pattern as everything else.
 **Pattern**: Same as Task 3/4/5/6.
 
 **Changes**:
-- [ ] Create `FileDeps` module with `builder` and `t` types
-- [ ] `process_cmt_file` returns `FileDeps.builder`
-- [ ] `FileDeps.merge_all : builder list -> FileGraph.t`
-- [ ] `topological_order : FileGraph.t -> string list` (pure function)
-- [ ] `DeadModules` state becomes part of per-file data
+- [x] Create `FileDeps` module with `builder` and `t` types
+- [x] `process_cmt_file` returns `FileDeps.builder`
+- [x] `FileDeps.merge_all : builder list -> t`
+- [x] Thread `~file_deps` through `addValueReference`
+- [x] `iter_files_from_roots_to_leaves : t -> (string -> unit) -> unit` (pure function)
+- [x] Delete global `FileReferences` from `Common.ml`
+
+**Status**: Complete ✅
 
 **Test**: Build file graph, verify topological ordering is correct.
 
@@ -414,6 +417,7 @@ Can be parallelized, memoized, reordered.
 - [ ] `Decl.report`: Return `issue` instead of logging
 - [ ] Remove all `Log_.warning`, `Log_.item` calls from analysis path
 - [ ] Side effects (logging, JSON) only in final reporting phase
+- [ ] Make `DeadModules` state part of `analysis_result` (currently mutated during solver)
 
 **Architecture**:
 ```

analysis/reanalyze/src/Common.ml

@@ -49,31 +49,7 @@ module FileHash = struct
   end)
 end
 
-module FileReferences = struct
-  (* references across files *)
-  let table = (FileHash.create 256 : FileSet.t FileHash.t)
-
-  let findSet table key =
-    try FileHash.find table key with Not_found -> FileSet.empty
-
-  let add (locFrom : Location.t) (locTo : Location.t) =
-    let key = locFrom.loc_start.pos_fname in
-    let set = findSet table key in
-    FileHash.replace table key (FileSet.add locTo.loc_start.pos_fname set)
-
-  let addFile fileName =
-    let set = findSet table fileName in
-    FileHash.replace table fileName set
-
-  let exists fileName = FileHash.mem table fileName
-
-  let find fileName =
-    match FileHash.find_opt table fileName with
-    | Some set -> set
-    | None -> FileSet.empty
-
-  let iter f = FileHash.iter f table
-end
+(* NOTE: FileReferences has been moved to FileDeps module *)
 
 module Path = struct
   type t = Name.t list

analysis/reanalyze/src/CrossFileItems.ml

@@ -60,14 +60,15 @@ let merge_all (builders : builder list) : t =
 
 (** {2 Processing API} *)
 
-let process_exception_refs (t : t) ~refs ~find_exception ~config =
+let process_exception_refs (t : t) ~refs ~file_deps ~find_exception ~config =
   t.exception_refs
   |> List.iter (fun {exception_path; loc_from} ->
          match find_exception exception_path with
          | None -> ()
          | Some loc_to ->
-           DeadCommon.addValueReference ~config ~refs ~binding:Location.none
-             ~addFileReference:true ~locFrom:loc_from ~locTo:loc_to)
+           DeadCommon.addValueReference ~config ~refs ~file_deps
+             ~binding:Location.none ~addFileReference:true ~locFrom:loc_from
+             ~locTo:loc_to)
 
 let process_optional_args (t : t) ~decls =
   (* Process optional arg calls *)

analysis/reanalyze/src/CrossFileItems.mli

@@ -43,6 +43,7 @@ val merge_all : builder list -> t
 val process_exception_refs :
   t ->
   refs:References.builder ->
+  file_deps:FileDeps.builder ->
   find_exception:(Common.Path.t -> Location.t option) ->
   config:DceConfig.t ->
   unit

analysis/reanalyze/src/DceFileProcessing.ml

@@ -42,6 +42,7 @@ type file_data = {
   decls: Declarations.builder;
   refs: References.builder;
   cross_file: CrossFileItems.builder;
+  file_deps: FileDeps.builder;
 }
 
 let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
@@ -59,6 +60,9 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
   let decls = Declarations.create_builder () in
   let refs = References.create_builder () in
   let cross_file = CrossFileItems.create_builder () in
+  let file_deps = FileDeps.create_builder () in
+  (* Register this file *)
+  FileDeps.add_file file_deps file.source_path;
   (match cmt_infos.cmt_annots with
   | Interface signature ->
     CollectAnnotations.signature ~state:annotations ~config signature;
@@ -73,11 +77,11 @@ let process_cmt_file ~config ~(file : file_context) ~cmtFilePath
     processSignature ~config ~decls ~file ~doValues:true ~doTypes:false
       structure.str_type;
     let doExternals = false in
-    DeadValue.processStructure ~config ~decls ~refs ~cross_file
+    DeadValue.processStructure ~config ~decls ~refs ~file_deps ~cross_file
       ~file:dead_common_file ~doTypes:true ~doExternals
       ~cmt_value_dependencies:cmt_infos.cmt_value_dependencies structure
   | _ -> ());
   DeadType.TypeDependencies.forceDelayedItems ~config ~refs;
   DeadType.TypeDependencies.clear ();
   (* Return builders - caller will merge and freeze *)
-  {annotations; decls; refs; cross_file}
+  {annotations; decls; refs; cross_file; file_deps}

analysis/reanalyze/src/DceFileProcessing.mli

@@ -16,8 +16,9 @@ type file_data = {
   decls: Declarations.builder;
   refs: References.builder;
   cross_file: CrossFileItems.builder;
+  file_deps: FileDeps.builder;
 }
-(** Result of processing a cmt file - annotations, declarations, references, and delayed items *)
+(** Result of processing a cmt file - annotations, declarations, references, cross-file items, and file dependencies *)
 
 val process_cmt_file :
   config:DceConfig.t ->

analysis/reanalyze/src/DeadCommon.ml

@@ -87,8 +87,8 @@ let declGetLoc decl =
   in
   {Location.loc_start; loc_end = decl.posEnd; loc_ghost = false}
 
-let addValueReference ~config ~refs ~(binding : Location.t) ~addFileReference
-    ~(locFrom : Location.t) ~(locTo : Location.t) : unit =
+let addValueReference ~config ~refs ~file_deps ~(binding : Location.t)
+    ~addFileReference ~(locFrom : Location.t) ~(locTo : Location.t) : unit =
   let effectiveFrom = if binding = Location.none then locFrom else binding in
   if not effectiveFrom.loc_ghost then (
     if config.DceConfig.cli.debug then
@@ -101,82 +101,14 @@ let addValueReference ~config ~refs ~(binding : Location.t) ~addFileReference
       addFileReference && (not locTo.loc_ghost)
       && (not effectiveFrom.loc_ghost)
       && effectiveFrom.loc_start.pos_fname <> locTo.loc_start.pos_fname
-    then FileReferences.add effectiveFrom locTo)
-
-let iterFilesFromRootsToLeaves iterFun =
-  (* For each file, the number of incoming references *)
-  let inverseReferences = (Hashtbl.create 1 : (string, int) Hashtbl.t) in
-  (* For each number of incoming references, the files *)
-  let referencesByNumber = (Hashtbl.create 1 : (int, FileSet.t) Hashtbl.t) in
-  let getNum fileName =
-    try Hashtbl.find inverseReferences fileName with Not_found -> 0
-  in
-  let getSet num =
-    try Hashtbl.find referencesByNumber num with Not_found -> FileSet.empty
-  in
-  let addIncomingEdge fileName =
-    let oldNum = getNum fileName in
-    let newNum = oldNum + 1 in
-    let oldSetAtNum = getSet oldNum in
-    let newSetAtNum = FileSet.remove fileName oldSetAtNum in
-    let oldSetAtNewNum = getSet newNum in
-    let newSetAtNewNum = FileSet.add fileName oldSetAtNewNum in
-    Hashtbl.replace inverseReferences fileName newNum;
-    Hashtbl.replace referencesByNumber oldNum newSetAtNum;
-    Hashtbl.replace referencesByNumber newNum newSetAtNewNum
-  in
-  let removeIncomingEdge fileName =
-    let oldNum = getNum fileName in
-    let newNum = oldNum - 1 in
-    let oldSetAtNum = getSet oldNum in
-    let newSetAtNum = FileSet.remove fileName oldSetAtNum in
-    let oldSetAtNewNum = getSet newNum in
-    let newSetAtNewNum = FileSet.add fileName oldSetAtNewNum in
-    Hashtbl.replace inverseReferences fileName newNum;
-    Hashtbl.replace referencesByNumber oldNum newSetAtNum;
-    Hashtbl.replace referencesByNumber newNum newSetAtNewNum
-  in
-  let addEdge fromFile toFile =
-    if FileReferences.exists fromFile then addIncomingEdge toFile
-  in
-  let removeEdge fromFile toFile =
-    if FileReferences.exists fromFile then removeIncomingEdge toFile
-  in
-  FileReferences.iter (fun fromFile set ->
-      if getNum fromFile = 0 then
-        Hashtbl.replace referencesByNumber 0 (FileSet.add fromFile (getSet 0));
-      set |> FileSet.iter (fun toFile -> addEdge fromFile toFile));
-  while getSet 0 <> FileSet.empty do
-    let filesWithNoIncomingReferences = getSet 0 in
-    Hashtbl.remove referencesByNumber 0;
-    filesWithNoIncomingReferences
-    |> FileSet.iter (fun fileName ->
-           iterFun fileName;
-           let references = FileReferences.find fileName in
-           references |> FileSet.iter (fun toFile -> removeEdge fileName toFile))
-  done;
-  (* Process any remaining items in case of circular references *)
-  referencesByNumber
-  |> Hashtbl.iter (fun _num set ->
-         if FileSet.is_empty set then ()
-         else
-           set
-           |> FileSet.iter (fun fileName ->
-                  let pos = {Lexing.dummy_pos with pos_fname = fileName} in
-                  let loc =
-                    {Location.none with loc_start = pos; loc_end = pos}
-                  in
-                  if Config.warnOnCircularDependencies then
-                    Log_.warning ~loc
-                      (Circular
-                         {
-                           message =
-                             Format.asprintf
-                               "Results for %s could be inaccurate because of \
-                                circular references"
-                               fileName;
-                         });
-                  iterFun fileName))
+    then
+      FileDeps.add_dep file_deps ~from_file:effectiveFrom.loc_start.pos_fname
+        ~to_file:locTo.loc_start.pos_fname)
+
+(* NOTE: iterFilesFromRootsToLeaves moved to FileDeps.iter_files_from_roots_to_leaves *)
+
+let iterFilesFromRootsToLeaves ~file_deps iterFun =
+  FileDeps.iter_files_from_roots_to_leaves file_deps iterFun
 
 let addDeclaration_ ~config ~decls ~(file : FileContext.t) ?posEnd ?posStart
     ~declKind ~path ~(loc : Location.t) ?(posAdjustment = Nothing) ~moduleLoc
@@ -498,7 +430,7 @@ let rec resolveRecursiveRefs ~all_refs ~annotations ~config ~decls
           refsString level);
     isDead
 
-let reportDead ~annotations ~config ~decls ~refs
+let reportDead ~annotations ~config ~decls ~refs ~file_deps
     ~checkOptionalArg:
       (checkOptionalArgFn :
         annotations:FileAnnotations.t -> config:DceConfig.t -> decl -> unit) =
@@ -517,7 +449,7 @@ let reportDead ~annotations ~config ~decls ~refs
   if config.DceConfig.cli.debug then (
     Log_.item "@.File References@.@.";
     let fileList = ref [] in
-    FileReferences.iter (fun file files ->
+    FileDeps.iter_deps file_deps (fun file files ->
         fileList := (file, files) :: !fileList);
     !fileList
     |> List.sort (fun (f1, _) (f2, _) -> String.compare f1 f2)
@@ -532,7 +464,7 @@ let reportDead ~annotations ~config ~decls ~refs
       decls []
   in
   let orderedFiles = Hashtbl.create 256 in
-  iterFilesFromRootsToLeaves
+  iterFilesFromRootsToLeaves ~file_deps
     (let current = ref 0 in
      fun fileName ->
        incr current;

analysis/reanalyze/src/DeadException.ml

@@ -13,7 +13,7 @@ let add ~config ~decls ~file ~path ~loc ~(strLoc : Location.t) name =
 
 let find_exception path = Hashtbl.find_opt declarations path
 
-let markAsUsed ~config ~refs ~cross_file ~(binding : Location.t)
+let markAsUsed ~config ~refs ~file_deps ~cross_file ~(binding : Location.t)
     ~(locFrom : Location.t) ~(locTo : Location.t) path_ =
   if locTo.loc_ghost then
     (* 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)
     CrossFileItems.add_exception_ref cross_file ~exception_path:exceptionPath
       ~loc_from:locFrom
   else
-    addValueReference ~config ~refs ~binding ~addFileReference:true ~locFrom
-      ~locTo
+    addValueReference ~config ~refs ~file_deps ~binding ~addFileReference:true
+      ~locFrom ~locTo

analysis/reanalyze/src/DeadValue.ml

@@ -109,8 +109,8 @@ let processOptionalArgs ~config ~cross_file ~expType ~(locFrom : Location.t)
     (!supplied, !suppliedMaybe)
     |> DeadOptionalArgs.addReferences ~config ~cross_file ~locFrom ~locTo ~path)
 
-let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
-    self (e : Typedtree.expression) =
+let rec collectExpr ~config ~refs ~file_deps ~cross_file
+    ~(last_binding : Location.t) super self (e : Typedtree.expression) =
   let locFrom = e.exp_loc in
   let binding = last_binding in
   (match e.exp_desc with
@@ -126,8 +126,8 @@ let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
       References.add_value_ref refs ~posTo:locTo.loc_start
         ~posFrom:Location.none.loc_start)
     else
-      addValueReference ~config ~refs ~binding ~addFileReference:true ~locFrom
-        ~locTo
+      addValueReference ~config ~refs ~file_deps ~binding ~addFileReference:true
+        ~locFrom ~locTo
   | Texp_apply
       {
         funct =
@@ -195,8 +195,8 @@ let rec collectExpr ~config ~refs ~cross_file ~(last_binding : Location.t) super
     (match cstr_tag with
     | Cstr_extension path ->
       path
-      |> DeadException.markAsUsed ~config ~refs ~cross_file ~binding ~locFrom
-           ~locTo
+      |> DeadException.markAsUsed ~config ~refs ~file_deps ~cross_file ~binding
+           ~locFrom ~locTo
     | _ -> ());
     if !Config.analyzeTypes && not loc_ghost then
       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
              ->
              (* Punned field in OCaml projects has ghost location in expression *)
              let e = {e with exp_loc = {exp_loc with loc_ghost = false}} in
-             collectExpr ~config ~refs ~cross_file ~last_binding super self e
+             collectExpr ~config ~refs ~file_deps ~cross_file ~last_binding
+               super self e
              |> ignore
            | _ -> ())
   | _ -> ());
@@ -294,7 +295,7 @@ let rec processSignatureItem ~config ~decls ~file ~doTypes ~doValues ~moduleLoc
   ModulePath.setCurrent oldModulePath
 
 (* Traverse the AST *)
-let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
+let traverseStructure ~config ~decls ~refs ~file_deps ~cross_file ~file ~doTypes
     ~doExternals (structure : Typedtree.structure) : unit =
   let rec create_mapper (last_binding : Location.t) =
     let super = Tast_mapper.default in
@@ -304,7 +305,8 @@ let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
         expr =
           (fun _self e ->
             e
-            |> collectExpr ~config ~refs ~cross_file ~last_binding super mapper);
+            |> collectExpr ~config ~refs ~file_deps ~cross_file ~last_binding
+                 super mapper);
         pat = (fun _self p -> p |> collectPattern ~config ~refs super mapper);
         structure_item =
           (fun _self (structureItem : Typedtree.structure_item) ->
@@ -408,7 +410,7 @@ let traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes
   mapper.structure mapper structure |> ignore
 
 (* Merge a location's references to another one's *)
-let processValueDependency ~config ~decls ~refs ~cross_file
+let processValueDependency ~config ~decls ~refs ~file_deps ~cross_file
     ( ({
          val_loc =
            {loc_start = {pos_fname = fnTo} as posTo; loc_ghost = ghost1} as
@@ -423,16 +425,17 @@ let processValueDependency ~config ~decls ~refs ~cross_file
         Types.value_description) ) =
   if (not ghost1) && (not ghost2) && posTo <> posFrom then (
     let addFileReference = fileIsImplementationOf fnTo fnFrom in
-    addValueReference ~config ~refs ~binding:Location.none ~addFileReference
-      ~locFrom ~locTo;
+    addValueReference ~config ~refs ~file_deps ~binding:Location.none
+      ~addFileReference ~locFrom ~locTo;
     DeadOptionalArgs.addFunctionReference ~config ~decls ~cross_file ~locFrom
       ~locTo)
 
-let processStructure ~config ~decls ~refs ~cross_file ~file
+let processStructure ~config ~decls ~refs ~file_deps ~cross_file ~file
     ~cmt_value_dependencies ~doTypes ~doExternals
     (structure : Typedtree.structure) =
-  traverseStructure ~config ~decls ~refs ~cross_file ~file ~doTypes ~doExternals
-    structure;
+  traverseStructure ~config ~decls ~refs ~file_deps ~cross_file ~file ~doTypes
+    ~doExternals structure;
   let valueDependencies = cmt_value_dependencies |> List.rev in
   valueDependencies
-  |> List.iter (processValueDependency ~config ~decls ~refs ~cross_file)
+  |> List.iter
+       (processValueDependency ~config ~decls ~refs ~file_deps ~cross_file)