Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,16 @@

#### :bug: Bug fix

- Reanalyze: make optional args analysis liveness-aware, preventing false positives when functions are only called from dead code. https://github.com/rescript-lang/rescript/pull/8082

#### :memo: Documentation

#### :nail_care: Polish

#### :house: Internal

- Reanalyze: refactor DCE to pure pipeline architecture for order-independence and incremental update support. https://github.com/rescript-lang/rescript/pull/8043

# 12.0.1

#### :bug: Bug fix
Expand Down
27 changes: 20 additions & 7 deletions analysis/reanalyze/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 ║
╚══════════════════════════════════════════════════╪══════════════════════╝
╔══════════════════════════════════════════════════╪══════════════════════╗
Expand Down Expand Up @@ -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)

Expand Down
40 changes: 22 additions & 18 deletions analysis/reanalyze/src/CrossFileItems.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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
11 changes: 8 additions & 3 deletions analysis/reanalyze/src/CrossFileItems.mli
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. *)
12 changes: 7 additions & 5 deletions analysis/reanalyze/src/DeadOptionalArgs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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. *)
Expand Down
9 changes: 5 additions & 4 deletions analysis/reanalyze/src/DeadValue.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) =
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions analysis/reanalyze/src/Decl.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
41 changes: 33 additions & 8 deletions analysis/reanalyze/src/Reanalyze.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading
Loading