reanalyze: make optional args analysis liveness-aware

Optional argument analysis now only considers calls from live code when
determining which optional arguments are unused. This prevents false
positives when a function is only called from dead code.

Changes:
- Add pos_from to optional_arg_call to track caller binding position
- Add is_live predicate to compute_optional_args_state
- Two-pass solve: first compute deadness, then optional args with liveness
- Add Decl.isLive helper to simplify repeated liveness checks
- Update ARCHITECTURE.md to document the two-pass approach

Signed-Off-By: Cristiano Calcagno <[email protected]>

Author: cristianoc

analysis/reanalyze/ARCHITECTURE.md

@@ -70,14 +70,17 @@ This design enables:
     ╠══════════════════════════════════════════════════╪══════════════════════╣
     ║                                                  ▼                      ║
     ║  ┌─────────────────────────────────────────────────────────────────┐   ║
-    ║  │ DeadCommon.solveDead                                            │   ║
-    ║  │   ~annotations ~decls ~refs ~file_deps                          │   ║
-    ║  │   ~optional_args_state ~config ~checkOptionalArg                │   ║
+    ║  │ Pass 1: DeadCommon.solveDead (core deadness)                    │   ║
+    ║  │   ~annotations ~decls ~refs ~file_deps ~config                  │   ║
+    ║  │   → AnalysisResult.t (dead/live status resolved)                │   ║
     ║  │                                                                  │   ║
+    ║  │ Pass 2: Optional args analysis (liveness-aware)                 │   ║
+    ║  │   CrossFileItems.compute_optional_args_state ~is_live           │   ║
+    ║  │   DeadOptionalArgs.check (only for live decls)                  │   ║
     ║  │   → AnalysisResult.t { issues: Issue.t list }                   │   ║
     ║  └─────────────────────────────────────────────────────────────────┘   ║
     ║                                                  │                      ║
-    ║  Pure function: immutable in → immutable out     │ issues               ║
+    ║  Pure functions: immutable in → immutable out    │ issues               ║
     ╚══════════════════════════════════════════════════╪══════════════════════╝
                                                        │
     ╔══════════════════════════════════════════════════╪══════════════════════╗
@@ -149,20 +152,30 @@ let file_deps = FileDeps.merge_all (file_data_list |> List.map (fun fd -> fd.fil
 
 ### Phase 3: SOLVE (Deadness Computation)
 
-**Entry point**: `DeadCommon.solveDead`
+**Entry point**: `DeadCommon.solveDead` + optional args second pass in `Reanalyze.runAnalysis`
 
 **Input**: All merged data + config
 
 **Output**: `AnalysisResult.t` containing `Issue.t list`
 
-**Algorithm**:
+**Algorithm** (two-pass for liveness-aware optional args):
+
+**Pass 1: Core deadness resolution**
 1. Build file dependency order (roots to leaves)
 2. Sort declarations by dependency order
 3. For each declaration, resolve references recursively
 4. Determine dead/live status based on reference count
 5. Collect issues for dead declarations
 
-**Key property**: Pure function - immutable in, immutable out. No side effects.
+**Pass 2: Liveness-aware optional args analysis**
+1. Use `Decl.isLive` to build an `is_live` predicate from Pass 1 results
+2. Compute optional args state via `CrossFileItems.compute_optional_args_state`, filtering out calls from dead code
+3. Collect optional args issues only for live declarations
+4. Merge optional args issues into the final result
+
+This two-pass approach ensures that optional argument warnings (e.g., "argument X is never used") only consider calls from live code, preventing false positives when a function is only called from dead code.
+
+**Key property**: Pure functions - immutable in, immutable out. No side effects.
 
 ### Phase 4: REPORT (Output)
 

analysis/reanalyze/src/CrossFileItems.ml

@@ -8,6 +8,7 @@
 type exception_ref = {exception_path: DcePath.t; loc_from: Location.t}
 
 type optional_arg_call = {
+  pos_from: Lexing.position;
   pos_to: Lexing.position;
   arg_names: string list;
   arg_names_maybe: string list;
@@ -37,9 +38,10 @@ let create_builder () : builder =
 let add_exception_ref (b : builder) ~exception_path ~loc_from =
   b.exception_refs <- {exception_path; loc_from} :: b.exception_refs
 
-let add_optional_arg_call (b : builder) ~pos_to ~arg_names ~arg_names_maybe =
+let add_optional_arg_call (b : builder) ~pos_from ~pos_to ~arg_names
+    ~arg_names_maybe =
   b.optional_arg_calls <-
-    {pos_to; arg_names; arg_names_maybe} :: b.optional_arg_calls
+    {pos_from; pos_to; arg_names; arg_names_maybe} :: b.optional_arg_calls
 
 let add_function_reference (b : builder) ~pos_from ~pos_to =
   b.function_refs <- {pos_from; pos_to} :: b.function_refs
@@ -71,7 +73,7 @@ let process_exception_refs (t : t) ~refs ~file_deps ~find_exception ~config =
 (** 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 compute_optional_args_state (t : t) ~decls ~is_live : OptionalArgsState.t =
   let state = OptionalArgsState.create () in
   (* Initialize state from declarations *)
   let get_state pos =
@@ -85,22 +87,24 @@ let compute_optional_args_state (t : t) ~decls : OptionalArgsState.t =
   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} ->
-         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);
+  |> List.iter (fun {pos_from; pos_to; arg_names; arg_names_maybe} ->
+         if is_live pos_from then
+           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} ->
-         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));
+         if is_live pos_from then
+           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

@@ -23,11 +23,12 @@ val add_exception_ref :
 
 val add_optional_arg_call :
   builder ->
+  pos_from:Lexing.position ->
   pos_to:Lexing.position ->
   arg_names:string list ->
   arg_names_maybe:string list ->
   unit
-(** Add a cross-file optional argument call. *)
+(** Add a cross-file optional argument call, recording caller and callee. *)
 
 val add_function_reference :
   builder -> pos_from:Lexing.position -> pos_to:Lexing.position -> unit
@@ -52,6 +53,10 @@ val process_exception_refs :
 (** {2 Optional Args State} *)
 
 val compute_optional_args_state :
-  t -> decls:Declarations.t -> OptionalArgsState.t
-(** Compute final optional args state from calls and function references.
+  t ->
+  decls:Declarations.t ->
+  is_live:(Lexing.position -> bool) ->
+  OptionalArgsState.t
+(** Compute final optional args state from calls and function references,
+    taking into account caller liveness via the [is_live] predicate.
     Pure function - does not mutate declarations. *)

analysis/reanalyze/src/DeadOptionalArgs.ml

@@ -40,20 +40,22 @@ let rec fromTypeExpr (texpr : Types.type_expr) =
   | _ -> []
 
 let addReferences ~config ~cross_file ~(locFrom : Location.t)
-    ~(locTo : Location.t) ~path (argNames, argNamesMaybe) =
+    ~(locTo : Location.t) ~(binding : Location.t) ~path (argNames, argNamesMaybe)
+    =
   if active () then (
     let posTo = locTo.loc_start in
-    let posFrom = locFrom.loc_start in
-    CrossFileItems.add_optional_arg_call cross_file ~pos_to:posTo
-      ~arg_names:argNames ~arg_names_maybe:argNamesMaybe;
+    let posFrom = binding.loc_start in
+    CrossFileItems.add_optional_arg_call cross_file ~pos_from:posFrom
+      ~pos_to:posTo ~arg_names:argNames ~arg_names_maybe:argNamesMaybe;
     if config.DceConfig.cli.debug then
+      let callPos = locFrom.loc_start in
       Log_.item
         "DeadOptionalArgs.addReferences %s called with optional argNames:%s \
          argNamesMaybe:%s %s@."
         (path |> DcePath.fromPathT |> DcePath.toString)
         (argNames |> String.concat ", ")
         (argNamesMaybe |> String.concat ", ")
-        (posFrom |> Pos.toString))
+        (callPos |> Pos.toString))
 
 (** Check for optional args issues. Returns issues instead of logging.
     Uses optional_args_state map for final computed state. *)

analysis/reanalyze/src/DeadValue.ml

@@ -78,7 +78,7 @@ let collectValueBinding ~config ~decls ~file ~(current_binding : Location.t)
   loc
 
 let processOptionalArgs ~config ~cross_file ~expType ~(locFrom : Location.t)
-    ~locTo ~path args =
+    ~(binding : Location.t) ~locTo ~path args =
   if expType |> DeadOptionalArgs.hasOptionalArgs then (
     let supplied = ref [] in
     let suppliedMaybe = ref [] in
@@ -107,7 +107,8 @@ let processOptionalArgs ~config ~cross_file ~expType ~(locFrom : Location.t)
              if argIsSupplied = None then suppliedMaybe := s :: !suppliedMaybe
            | _ -> ());
     (!supplied, !suppliedMaybe)
-    |> DeadOptionalArgs.addReferences ~config ~cross_file ~locFrom ~locTo ~path)
+    |> DeadOptionalArgs.addReferences ~config ~cross_file ~locFrom ~locTo
+         ~binding ~path)
 
 let rec collectExpr ~config ~refs ~file_deps ~cross_file
     ~(last_binding : Location.t) super self (e : Typedtree.expression) =
@@ -142,7 +143,7 @@ let rec collectExpr ~config ~refs ~file_deps ~cross_file
     args
     |> processOptionalArgs ~config ~cross_file ~expType:exp_type
          ~locFrom:(locFrom : Location.t)
-         ~locTo ~path
+         ~binding:last_binding ~locTo ~path
   | Texp_let
       ( (* generated for functions with optional args *)
         Nonrecursive,
@@ -183,7 +184,7 @@ let rec collectExpr ~config ~refs ~file_deps ~cross_file
     args
     |> processOptionalArgs ~config ~cross_file ~expType:exp_type
          ~locFrom:(locFrom : Location.t)
-         ~locTo ~path
+         ~binding:last_binding ~locTo ~path
   | Texp_field
       (_, _, {lbl_loc = {Location.loc_start = posTo; loc_ghost = false}; _}) ->
     if !Config.analyzeTypes then

analysis/reanalyze/src/Decl.ml

@@ -43,6 +43,12 @@ let isValue decl =
   | Value _ (* | Exception *) -> true
   | _ -> false
 
+(** Check if a declaration is live (or unknown). Returns false only if resolved as dead. *)
+let isLive decl =
+  match decl.resolvedDead with
+  | Some true -> false
+  | Some false | None -> true
+
 let compareUsingDependencies ~orderedFiles
     {
       declKind = kind1;

analysis/reanalyze/src/Reanalyze.ml

@@ -168,18 +168,43 @@ 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;
-    (* 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 =
+    (* Run the solver - returns immutable AnalysisResult.t.
+       Optional-args checks are done in a separate pass after liveness is known. *)
+    let empty_optional_args_state = OptionalArgsState.create () in
+    let analysis_result_core =
       DeadCommon.solveDead ~annotations ~decls ~refs ~file_deps
-        ~optional_args_state ~config:dce_config
-        ~checkOptionalArg:DeadOptionalArgs.check
+        ~optional_args_state:empty_optional_args_state ~config:dce_config
+        ~checkOptionalArg:(fun
+            ~optional_args_state:_ ~annotations:_ ~config:_ _ -> [])
+    in
+    (* Compute liveness-aware optional args state *)
+    let is_live pos =
+      match Declarations.find_opt decls pos with
+      | Some decl -> Decl.isLive decl
+      | None -> true
+    in
+    let optional_args_state =
+      CrossFileItems.compute_optional_args_state cross_file ~decls ~is_live
+    in
+    (* Collect optional args issues only for live declarations *)
+    let optional_args_issues =
+      Declarations.fold
+        (fun _pos decl acc ->
+          if Decl.isLive decl then
+            let issues =
+              DeadOptionalArgs.check ~optional_args_state ~annotations
+                ~config:dce_config decl
+            in
+            List.rev_append issues acc
+          else acc)
+        decls []
+      |> List.rev
+    in
+    let analysis_result =
+      AnalysisResult.add_issues analysis_result_core optional_args_issues
     in
     (* Report all issues *)
     AnalysisResult.get_issues analysis_result