DCE: Make OptionalArgs tracking immutable (Optional Task)

OptionalArgs.t is now fully immutable with no mutation of declarations.

Key changes:
- OptionalArgs.t: removed mutable fields
- apply_call: pure function, returns new state
- combine_pair: pure function, returns pair of new states
- OptionalArgsState module in Common.ml for computed state map
- compute_optional_args_state: returns immutable OptionalArgsState.t
- DeadOptionalArgs.check: looks up state from map

Architecture:
- Declaration's optionalArgs = initial state (what args exist)
- OptionalArgsState.t = computed state (after all calls/combines)
- Solver uses OptionalArgsState.find_opt to get final state

This completes the pure analysis pipeline - no mutation anywhere.

Author: cristianoc

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

@@ -613,32 +613,23 @@ add `@dead` annotations.
 
 ## Optional Future Tasks
 
-### Optional Task: Make OptionalArgs tracking immutable
+### Optional Task: Make OptionalArgs tracking immutable ✅
 
-**Value**: Currently `CrossFileItems.process_optional_args` mutates `optionalArgs` inside declarations.
-Making this immutable would complete the pure pipeline.
+**Value**: `OptionalArgs.t` is now fully immutable. No mutation of declarations.
 
-**Current state**:
-- `OptionalArgs.t` inside `decl.declKind = Value {optionalArgs}` is mutable
-- `OptionalArgs.call` and `OptionalArgs.combine` mutate the record
-- This happens after merge but before solver
+**Changes made**:
+- [x] Made `OptionalArgs.t` immutable (no mutable fields)
+- [x] Added pure functions: `apply_call`, `combine_pair`
+- [x] Created `OptionalArgsState` module in `Common.ml` for state map
+- [x] `compute_optional_args_state` returns immutable state map
+- [x] `DeadOptionalArgs.check` looks up state from map
 
-**Why it's acceptable now**:
-- Mutation happens in a well-defined phase (after merge, before solver)
-- Solver sees effectively immutable data
-- Order independence is maintained (calls accumulate, order doesn't matter)
+**Architecture**:
+- Declaration's `optionalArgs` = initial state (what args exist)
+- `OptionalArgsState.t` = computed state (after all calls/combines)
+- Solver uses `OptionalArgsState.find_opt` to get final state
 
-**Changes needed**:
-- [ ] Make `OptionalArgs.t` an immutable data structure
-- [ ] Collect call info during AST processing as `OptionalArgCalls.builder`
-- [ ] Return calls from `process_cmt_file` in `file_data`
-- [ ] Merge all calls after file processing
-- [ ] Build final `OptionalArgs` state from merged calls (pure)
-- [ ] Store immutable `OptionalArgs` in declarations
-
-**Estimated effort**: Medium-High (touches core data structures)
-
-**Priority**: Low (current design works, just not fully pure)
+**Status**: Complete ✅
 
 ---
 

analysis/reanalyze/src/Common.ml

@@ -99,11 +99,11 @@ module Path = struct
 end
 
 module OptionalArgs = struct
-  type t = {
-    mutable count: int;
-    mutable unused: StringSet.t;
-    mutable alwaysUsed: StringSet.t;
-  }
+  type t = {count: int; unused: StringSet.t; alwaysUsed: StringSet.t}
+  (** Immutable record tracking optional argument usage.
+      - unused: args that have never been passed
+      - alwaysUsed: args that are always passed (when count > 0)
+      - count: number of calls observed *)
 
   let empty =
     {unused = StringSet.empty; alwaysUsed = StringSet.empty; count = 0}
@@ -113,23 +113,27 @@ module OptionalArgs = struct
 
   let isEmpty x = StringSet.is_empty x.unused
 
-  let call ~argNames ~argNamesMaybe x =
+  (** Apply a call to the optional args state. Returns new state. *)
+  let apply_call ~argNames ~argNamesMaybe x =
     let nameSet = argNames |> StringSet.of_list in
     let nameSetMaybe = argNamesMaybe |> StringSet.of_list in
     let nameSetAlways = StringSet.diff nameSet nameSetMaybe in
-    if x.count = 0 then x.alwaysUsed <- nameSetAlways
-    else x.alwaysUsed <- StringSet.inter nameSetAlways x.alwaysUsed;
-    argNames
-    |> List.iter (fun name -> x.unused <- StringSet.remove name x.unused);
-    x.count <- x.count + 1
-
-  let combine x y =
+    let alwaysUsed =
+      if x.count = 0 then nameSetAlways
+      else StringSet.inter nameSetAlways x.alwaysUsed
+    in
+    let unused =
+      argNames
+      |> List.fold_left (fun acc name -> StringSet.remove name acc) x.unused
+    in
+    {count = x.count + 1; unused; alwaysUsed}
+
+  (** Combine two optional args states (for function references).
+      Returns a pair of updated states with intersected unused/alwaysUsed. *)
+  let combine_pair x y =
     let unused = StringSet.inter x.unused y.unused in
-    x.unused <- unused;
-    y.unused <- unused;
     let alwaysUsed = StringSet.inter x.alwaysUsed y.alwaysUsed in
-    x.alwaysUsed <- alwaysUsed;
-    y.alwaysUsed <- alwaysUsed
+    ({x with unused; alwaysUsed}, {y with unused; alwaysUsed})
 
   let iterUnused f x = StringSet.iter f x.unused
   let iterAlwaysUsed f x = StringSet.iter (fun s -> f s x.count) x.alwaysUsed
@@ -140,6 +144,29 @@ module OptionalArgs = struct
     StringSet.fold (fun s acc -> f s x.count acc) x.alwaysUsed init
 end
 
+(* Position-keyed hashtable - shared across modules *)
+module PosHash = Hashtbl.Make (struct
+  type t = Lexing.position
+
+  let hash x =
+    let s = Filename.basename x.Lexing.pos_fname in
+    Hashtbl.hash (x.Lexing.pos_cnum, s)
+
+  let equal (x : t) y = x = y
+end)
+
+(** State map for computed OptionalArgs.
+    Maps declaration position to final state after all calls/combines. *)
+module OptionalArgsState = struct
+  type t = OptionalArgs.t PosHash.t
+
+  let create () : t = PosHash.create 256
+
+  let find_opt (state : t) pos = PosHash.find_opt state pos
+
+  let set (state : t) pos value = PosHash.replace state pos value
+end
+
 module DeclKind = struct
   type t =
     | Exception

analysis/reanalyze/src/CrossFileItems.ml

@@ -5,6 +5,17 @@
 
 open Common
 
+(* Position-keyed hashtable *)
+module PosHash = Hashtbl.Make (struct
+  type t = Lexing.position
+
+  let hash x =
+    let s = Filename.basename x.Lexing.pos_fname in
+    Hashtbl.hash (x.Lexing.pos_cnum, s)
+
+  let equal (x : t) y = x = y
+end)
+
 (** {2 Item types} *)
 
 type exception_ref = {exception_path: Path.t; loc_from: Location.t}
@@ -70,24 +81,39 @@ let process_exception_refs (t : t) ~refs ~file_deps ~find_exception ~config =
              ~binding:Location.none ~addFileReference:true ~locFrom:loc_from
              ~locTo:loc_to)
 
-let process_optional_args (t : t) ~decls =
+(** Compute optional args state from calls and function references.
+    Returns a map from position to final OptionalArgs.t state.
+    Pure function - does not mutate declarations. *)
+let compute_optional_args_state (t : t) ~decls : OptionalArgsState.t =
+  let state = OptionalArgsState.create () in
+  (* Initialize state from declarations *)
+  let get_state pos =
+    match OptionalArgsState.find_opt state pos with
+    | Some s -> s
+    | None -> (
+      match Declarations.find_opt decls pos with
+      | Some {declKind = Value {optionalArgs}} -> optionalArgs
+      | _ -> OptionalArgs.empty)
+  in
+  let set_state pos s = OptionalArgsState.set state pos s in
   (* Process optional arg calls *)
   t.optional_arg_calls
   |> List.iter (fun {pos_to; arg_names; arg_names_maybe} ->
-         match Declarations.find_opt decls pos_to with
-         | Some {declKind = Value r} ->
-           r.optionalArgs
-           |> OptionalArgs.call ~argNames:arg_names
-                ~argNamesMaybe:arg_names_maybe
-         | _ -> ());
+         let current = get_state pos_to in
+         let updated =
+           OptionalArgs.apply_call ~argNames:arg_names
+             ~argNamesMaybe:arg_names_maybe current
+         in
+         set_state pos_to updated);
   (* Process function references *)
   t.function_refs
   |> List.iter (fun {pos_from; pos_to} ->
-         match
-           ( Declarations.find_opt decls pos_from,
-             Declarations.find_opt decls pos_to )
-         with
-         | Some {declKind = Value rFrom}, Some {declKind = Value rTo}
-           when not (OptionalArgs.isEmpty rTo.optionalArgs) ->
-           OptionalArgs.combine rFrom.optionalArgs rTo.optionalArgs
-         | _ -> ())
+         let state_from = get_state pos_from in
+         let state_to = get_state pos_to in
+         if not (OptionalArgs.isEmpty state_to) then (
+           let updated_from, updated_to =
+             OptionalArgs.combine_pair state_from state_to
+           in
+           set_state pos_from updated_from;
+           set_state pos_to updated_to));
+  state

analysis/reanalyze/src/CrossFileItems.mli

@@ -49,5 +49,9 @@ val process_exception_refs :
   unit
 (** Process cross-file exception references. *)
 
-val process_optional_args : t -> decls:Declarations.t -> unit
-(** Process cross-file optional argument calls and function references. *)
+(** {2 Optional Args State} *)
+
+val compute_optional_args_state :
+  t -> decls:Declarations.t -> Common.OptionalArgsState.t
+(** Compute final optional args state from calls and function references.
+    Pure function - does not mutate declarations. *)

analysis/reanalyze/src/DeadCommon.ml

@@ -430,9 +430,10 @@ let rec resolveRecursiveRefs ~all_refs ~annotations ~config ~decls
           refsString level);
     isDead
 
-let reportDead ~annotations ~config ~decls ~refs ~file_deps
+let reportDead ~annotations ~config ~decls ~refs ~file_deps ~optional_args_state
     ~checkOptionalArg:
       (checkOptionalArgFn :
+        optional_args_state:OptionalArgsState.t ->
         annotations:FileAnnotations.t ->
         config:DceConfig.t ->
         decl ->
@@ -444,7 +445,7 @@ let reportDead ~annotations ~config ~decls ~refs ~file_deps
       | false -> References.find_type_refs refs decl.pos
     in
     resolveRecursiveRefs ~all_refs:refs ~annotations ~config ~decls
-      ~checkOptionalArg:(checkOptionalArgFn ~annotations)
+      ~checkOptionalArg:(checkOptionalArgFn ~optional_args_state ~annotations)
       ~deadDeclarations ~issues ~level:0 ~orderedFiles
       ~refsBeingResolved:(ref PosSet.empty) ~refs:decl_refs decl
     |> ignore

analysis/reanalyze/src/DeadOptionalArgs.ml

@@ -56,14 +56,21 @@ let addReferences ~config ~cross_file ~(locFrom : Location.t)
         (argNamesMaybe |> String.concat ", ")
         (posFrom |> posToString))
 
-(** Check for optional args issues. Returns issues instead of logging. *)
-let check ~annotations ~config:_ decl : Common.issue list =
+(** Check for optional args issues. Returns issues instead of logging.
+    Uses optional_args_state map for final computed state. *)
+let check ~optional_args_state ~annotations ~config:_ decl : Common.issue list =
   match decl with
   | {declKind = Value {optionalArgs}}
     when active ()
          && not
               (FileAnnotations.is_annotated_gentype_or_live annotations decl.pos)
     ->
+    (* Look up computed state from map, fall back to declaration's initial state *)
+    let state =
+      match OptionalArgsState.find_opt optional_args_state decl.pos with
+      | Some s -> s
+      | None -> optionalArgs
+    in
     let loc = decl |> declGetLoc in
     let unused_issues =
       OptionalArgs.foldUnused
@@ -87,7 +94,7 @@ let check ~annotations ~config:_ decl : Common.issue list =
             }
           in
           issue :: acc)
-        optionalArgs []
+        state []
     in
     let redundant_issues =
       OptionalArgs.foldAlwaysUsed
@@ -112,7 +119,7 @@ let check ~annotations ~config:_ decl : Common.issue list =
             }
           in
           issue :: acc)
-        optionalArgs []
+        state []
     in
     (* Reverse to maintain original order from iterUnused/iterAlwaysUsed *)
     List.rev unused_issues @ List.rev redundant_issues

analysis/reanalyze/src/Reanalyze.ml

@@ -147,15 +147,18 @@ let runAnalysis ~dce_config ~cmtRoot =
     CrossFileItems.process_exception_refs cross_file ~refs:refs_builder
       ~file_deps:file_deps_builder ~find_exception:DeadException.find_exception
       ~config:dce_config;
-    (* Process cross-file optional args - they read decls *)
-    CrossFileItems.process_optional_args cross_file ~decls;
+    (* Compute optional args state (pure - no mutation) *)
+    let optional_args_state =
+      CrossFileItems.compute_optional_args_state cross_file ~decls
+    in
     (* Now freeze refs and file_deps for solver *)
     let refs = References.freeze_builder refs_builder in
     let file_deps = FileDeps.freeze_builder file_deps_builder in
     (* Run the solver - returns immutable AnalysisResult.t *)
     let analysis_result =
       DeadCommon.reportDead ~annotations ~decls ~refs ~file_deps
-        ~config:dce_config ~checkOptionalArg:DeadOptionalArgs.check
+        ~optional_args_state ~config:dce_config
+        ~checkOptionalArg:DeadOptionalArgs.check
     in
     (* Report all issues *)
     AnalysisResult.get_issues analysis_result