Skip to content

Commit ae08f4b

Browse files
committed
reanalyze: localise dead-code binding and reporting state
- Introduce BindingContext in DeadValue to track the current binding location during dead-code traversal, so binding context is explicit and locally encapsulated. - Introduce ReportingContext in DeadCommon to track, per file, the end position of the last reported value when deciding whether to suppress nested warnings. - Replace addValueReference_state with addValueReference ~binding, so value-reference bookkeeping is driven by an explicit binding location rather than a threaded analysis state. - Update dead-code value and exception handling to use the new addValueReference API. - Refresh DEADCODE_REFACTOR_PLAN.md to mark these state-localisation steps as completed and to narrow the remaining follow-up to making the binding context fully pure. - Verified with make test-analysis that behaviour and expected outputs remain unchanged.
1 parent 687ce52 commit ae08f4b

File tree

4 files changed

+146
-126
lines changed

4 files changed

+146
-126
lines changed

analysis/reanalyze/DEADCODE_REFACTOR_PLAN.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,9 @@ Each bullet above should be done as a separate patch touching only a small set o
200200
Goal: remove `DeadCommon.Current` globals for binding/reporting by threading explicit state.
201201

202202
- [x] Add `Current.state`/helpers in `DeadCommon` and thread it through `DeadValue` (bindings) and `DeadException.markAsUsed` so `last_binding` is no longer a global ref.
203-
- [x] Replace `Current.maxValuePosEnd` with a per‑reporting `Current.state` in `Decl.report`/`reportDead`.
204-
- [ ] Follow‑up: remove `Current.state ref` usage by making traversals return an updated state (pure, no mutation). Adjust `addValueReference_state` (or its successor) to be purely functional and always return the new state.
203+
- [x] Replace `Current.maxValuePosEnd` with a per‑reporting state in `Decl.report`/`reportDead` (now encapsulated in `ReportingContext`).
204+
- [x] Replace `addValueReference_state` with `addValueReference ~binding` so reference bookkeeping no longer threads `Current.state` or returns a fake “updated state”.
205+
- [ ] Follow‑up: remove the remaining local `Current.state ref` in `BindingContext` by making traversals return an updated binding context (pure, no mutation). At that point, binding context becomes an explicit input/output of the traversal, not hidden state.
205206

206207
### 4.4 Make `ProcessDeadAnnotations` state explicit
207208

analysis/reanalyze/src/DeadCommon.ml

Lines changed: 25 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,10 @@ module Config = struct
1919
end
2020

2121
module Current = struct
22-
type state = {
23-
last_binding: Location.t;
24-
max_value_pos_end: Lexing.position;
25-
}
22+
type state = {last_binding: Location.t; max_value_pos_end: Lexing.position}
2623

2724
let empty_state =
28-
{
29-
last_binding = Location.none;
30-
max_value_pos_end = Lexing.dummy_pos;
31-
}
25+
{last_binding = Location.none; max_value_pos_end = Lexing.dummy_pos}
3226

3327
let get_last_binding (s : state) = s.last_binding
3428

@@ -83,6 +77,17 @@ module ValueReferences = struct
8377
let find pos = PosHash.findSet table pos
8478
end
8579

80+
(* Local reporting context used only while emitting dead-code warnings.
81+
It tracks, per file, the end position of the last value we reported on,
82+
so nested values inside that range don't get duplicate warnings. *)
83+
module ReportingContext = struct
84+
type t = Lexing.position ref
85+
86+
let create () : t = ref Lexing.dummy_pos
87+
let get_max_end (ctx : t) = !ctx
88+
let set_max_end (ctx : t) (pos : Lexing.position) = ctx := pos
89+
end
90+
8691
module TypeReferences = struct
8792
(** all type references *)
8893
let table = (PosHash.create 256 : PosSet.t PosHash.t)
@@ -103,14 +108,9 @@ let declGetLoc decl =
103108
in
104109
{Location.loc_start; loc_end = decl.posEnd; loc_ghost = false}
105110

106-
let addValueReference_state ~(current : Current.state) ~addFileReference
111+
let addValueReference ~(binding : Location.t) ~addFileReference
107112
~(locFrom : Location.t) ~(locTo : Location.t) : unit =
108-
let lastBinding = current.last_binding in
109-
let effectiveFrom =
110-
match lastBinding = Location.none with
111-
| true -> locFrom
112-
| false -> lastBinding
113-
in
113+
let effectiveFrom = if binding = Location.none then locFrom else binding in
114114
if not effectiveFrom.loc_ghost then (
115115
if !Cli.debug then
116116
Log_.item "addValueReference %s --> %s@."
@@ -121,8 +121,7 @@ let addValueReference_state ~(current : Current.state) ~addFileReference
121121
addFileReference && (not locTo.loc_ghost)
122122
&& (not effectiveFrom.loc_ghost)
123123
&& effectiveFrom.loc_start.pos_fname <> locTo.loc_start.pos_fname
124-
then FileReferences.add effectiveFrom locTo);
125-
()
124+
then FileReferences.add effectiveFrom locTo)
126125

127126
let iterFilesFromRootsToLeaves iterFun =
128127
(* For each file, the number of incoming references *)
@@ -519,20 +518,21 @@ module Decl = struct
519518
(fname1, lnum1, bol1, cnum1, kind1)
520519
(fname2, lnum2, bol2, cnum2, kind2)
521520

522-
let isInsideReportedValue (current_state : Current.state ref) decl =
523-
let max_end = Current.get_max_end !current_state in
521+
let isInsideReportedValue (ctx : ReportingContext.t) decl =
522+
let max_end = ReportingContext.get_max_end ctx in
524523
let fileHasChanged = max_end.pos_fname <> decl.pos.pos_fname in
525524
let insideReportedValue =
526-
decl |> isValue && (not fileHasChanged) && max_end.pos_cnum > decl.pos.pos_cnum
525+
decl |> isValue && (not fileHasChanged)
526+
&& max_end.pos_cnum > decl.pos.pos_cnum
527527
in
528528
if not insideReportedValue then
529529
if decl |> isValue then
530530
if fileHasChanged || decl.posEnd.pos_cnum > max_end.pos_cnum then
531-
current_state := Current.with_max_end decl.posEnd !current_state;
531+
ReportingContext.set_max_end ctx decl.posEnd;
532532
insideReportedValue
533533

534-
let report current_state decl =
535-
let insideReportedValue = decl |> isInsideReportedValue current_state in
534+
let report (ctx : ReportingContext.t) decl =
535+
let insideReportedValue = decl |> isInsideReportedValue ctx in
536536
if decl.report then
537537
let name, message =
538538
match decl.declKind with
@@ -729,6 +729,5 @@ let reportDead ~checkOptionalArg =
729729
let sortedDeadDeclarations =
730730
!deadDeclarations |> List.fast_sort Decl.compareForReporting
731731
in
732-
(* XXX *)
733-
let current_state = ref Current.empty_state in
734-
sortedDeadDeclarations |> List.iter (Decl.report current_state)
732+
let reporting_ctx = ReportingContext.create () in
733+
sortedDeadDeclarations |> List.iter (Decl.report reporting_ctx)

analysis/reanalyze/src/DeadException.ml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,10 @@ let forceDelayedItems () =
2222
| None -> ()
2323
| Some locTo ->
2424
(* Delayed exception references don't need a binding context; use an empty state. *)
25-
DeadCommon.addValueReference_state
26-
~current:DeadCommon.Current.empty_state ~addFileReference:true
27-
~locFrom ~locTo)
25+
DeadCommon.addValueReference ~binding:Location.none
26+
~addFileReference:true ~locFrom ~locTo)
2827

29-
let markAsUsed ~(current_state : Current.state ref) ~(locFrom : Location.t)
28+
let markAsUsed ~(binding : Location.t) ~(locFrom : Location.t)
3029
~(locTo : Location.t) path_ =
3130
if locTo.loc_ghost then
3231
(* Probably defined in another file, delay processing and check at the end *)
@@ -35,5 +34,4 @@ let markAsUsed ~(current_state : Current.state ref) ~(locFrom : Location.t)
3534
in
3635
delayedItems := {exceptionPath; locFrom} :: !delayedItems
3736
else
38-
DeadCommon.addValueReference_state ~current:!current_state
39-
~addFileReference:true ~locFrom ~locTo
37+
DeadCommon.addValueReference ~binding ~addFileReference:true ~locFrom ~locTo

0 commit comments

Comments
 (0)