Skip to content

Commit dbe2e26

Browse files
committed
reanalyze: fix reactive invalidation on refs-only changes
When transitive mode is off, dead-value reporting depends on hasRefBelow, which depends on value_refs_from (cross-file refs). Fix ReactiveSolver so per-file issues are recomputed when value_refs_from changes, not only when dead_decls_by_file changes. This prevents stale warnings in long-lived reanalyze-server sessions. Also add -transitive/-no-transitive CLI overrides (overriding rescript.json) and document/regenerate the checked-in non-transitive reactive pipeline mermaid diagram. Signed-off-by: Cristiano Calcagno <[email protected]>
1 parent 8ab2246 commit dbe2e26

File tree

5 files changed

+102
-58
lines changed

5 files changed

+102
-58
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020

2121
#### :bug: Bug fix
2222

23+
- Reanalyze: fix reactive/server stale results when cross-file references change without changing dead declarations (non-transitive mode). https://github.com/rescript-lang/rescript/pull/8173
24+
2325
#### :memo: Documentation
2426

2527
#### :nail_care: Polish

analysis/reanalyze/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ rescript-tools reanalyze -config -reactive -timing -runs 3
6060
| `-churn n` | Remove/re-add n random files between runs (incremental correctness/perf testing) |
6161
| `-timing` | Report timing of analysis phases |
6262
| `-mermaid` | Output Mermaid diagram of reactive pipeline (to stderr) |
63+
| `-transitive` | Force transitive reporting (overrides rescript.json) |
64+
| `-no-transitive` | Disable transitive reporting (overrides rescript.json) |
6365
| `-debug` | Print debug information |
6466
| `-json` | Output in JSON format |
6567
| `-ci` | Internal flag for CI mode |
@@ -68,6 +70,18 @@ rescript-tools reanalyze -config -reactive -timing -runs 3
6870

6971
See [ARCHITECTURE.md](ARCHITECTURE.md) for details on the analysis pipeline.
7072

73+
### Regenerating the checked-in reactive pipeline diagram
74+
75+
`analysis/reanalyze/diagrams/reactive-pipeline-full.mmd` is generated from the live reactive graph printer (`Reactive.to_mermaid()`), and **we check in the non-transitive (`-no-transitive`) variant** because that is where cross-file `hasRefBelow` suppression is relevant (and where reactive invalidation bugs are easiest to spot).
76+
77+
To regenerate it:
78+
79+
```bash
80+
# Run from any ReScript project (so -config works), then capture stderr:
81+
rescript-tools reanalyze -config -reactive -no-transitive -mermaid \
82+
>/dev/null 2> analysis/reanalyze/diagrams/reactive-pipeline-full.mmd
83+
```
84+
7185
The DCE analysis is structured as a pure pipeline:
7286

7387
1. **MAP** - Process each `.cmt` file independently → per-file data

analysis/reanalyze/diagrams/reactive-pipeline-full.mmd

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ graph TD
2525
solver.live_decls[solver.live_decls]
2626
type_deps.impl_decls[type_deps.impl_decls]
2727
liveness.all_roots[liveness.all_roots]
28-
solver.dead_modules[solver.dead_modules]
2928
liveness.external_type_refs[liveness.external_type_refs]
3029
decl_refs.combined[decl_refs.combined]
3130
type_refs_from[type_refs_from]
@@ -34,20 +33,20 @@ graph TD
3433
liveness.external_value_refs[liveness.external_value_refs]
3534
liveness.value_refs_from[liveness.value_refs_from]
3635
value_refs_from[value_refs_from]
37-
solver.modules_with_dead[solver.modules_with_dead]
3836
solver.dead_decls[solver.dead_decls]
3937
exception_refs_collection[exception_refs_collection]
4038
type_deps.intf_decls[type_deps.intf_decls]
4139
file_data_collection[file_data_collection]
4240
solver.dead_module_issues[solver.dead_module_issues]
4341
decl_refs.with_type_refs[decl_refs.with_type_refs]
44-
solver.modules_with_live[solver.modules_with_live]
4542
decl_refs.type_decl_refs[decl_refs.type_decl_refs]
4643
files[files]
4744
solver.modules_with_reported[solver.modules_with_reported]
4845
liveness.externally_referenced[liveness.externally_referenced]
4946
liveness.edges[liveness.edges]
47+
solver.refs_token[solver.refs_token]
5048
liveness.live[liveness.live]
49+
solver.dead_modules_empty[solver.dead_modules_empty]
5150
decls[decls]
5251
type_deps.intf_to_impl_refs_join{join}
5352
liveness.external_value_refs_join{join}
@@ -57,7 +56,6 @@ graph TD
5756
exc_refs.resolved_refs_join{join}
5857
type_deps.combined_refs_to_union{union}
5958
liveness.externally_referenced_union{union}
60-
solver.dead_modules_join{join}
6159
liveness.all_roots_union{union}
6260
solver.dead_module_issues_join{join}
6361
solver.dead_decls_join{join}
@@ -68,6 +66,7 @@ graph TD
6866
decl_refs.with_value_refs_join{join}
6967
liveness.type_refs_from_union{union}
7068
type_deps.impl_needing_path2_join{join}
69+
solver.issues_by_file_join{join}
7170
liveness.external_type_refs_join{join}
7271
liveness.live_fp{fixpoint}
7372
decl_refs.type_decl_refs_join{join}
@@ -103,24 +102,22 @@ graph TD
103102
type_deps.decl_by_path -->|flatMap| type_deps.same_path_refs
104103
type_deps.u2 --> type_deps.combined_refs_to_union
105104
solver.live_decls --> solver.incorrect_dead_decls_join
106-
solver.live_decls -->|flatMap| solver.modules_with_live
107105
type_deps.impl_decls --> type_deps.impl_needing_path2_join
108106
type_deps.impl_decls --> type_deps.impl_to_intf_refs_join
109107
liveness.all_roots --> liveness.live_fp
110-
solver.dead_modules --> solver.dead_module_issues_join
111108
liveness.external_type_refs --> liveness.externally_referenced_union
112109
decl_refs.combined -->|flatMap| liveness.edges
113110
type_refs_from --> liveness.type_refs_from_union
114111
liveness.type_refs_from --> liveness.external_type_refs_join
115112
liveness.type_refs_from --> decl_refs.type_decl_refs_join
116-
solver.dead_decls_by_file -->|flatMap| solver.issues_by_file
113+
solver.dead_decls_by_file --> solver.issues_by_file_join
117114
liveness.external_value_refs --> liveness.externally_referenced_union
118115
liveness.value_refs_from --> liveness.external_value_refs_join
119116
liveness.value_refs_from --> decl_refs.value_decl_refs_join
117+
value_refs_from -->|flatMap| solver.refs_token
120118
value_refs_from --> liveness.value_refs_from_union
121-
solver.modules_with_dead --> solver.dead_modules_join
122119
solver.dead_decls -->|flatMap| solver.dead_decls_by_file
123-
solver.dead_decls -->|flatMap| solver.modules_with_dead
120+
solver.dead_decls -->|flatMap| solver.dead_modules_empty
124121
exception_refs_collection --> exc_refs.resolved_refs_join
125122
type_deps.intf_decls --> type_deps.intf_to_impl_refs_join
126123
file_data_collection -->|flatMap| files
@@ -131,13 +128,14 @@ graph TD
131128
file_data_collection -->|flatMap| annotations
132129
file_data_collection -->|flatMap| decls
133130
decl_refs.with_type_refs --> decl_refs.combined_join
134-
solver.modules_with_live --> solver.dead_modules_join
135131
decl_refs.type_decl_refs --> decl_refs.with_type_refs_join
136132
solver.modules_with_reported --> solver.dead_module_issues_join
137133
liveness.externally_referenced --> liveness.all_roots_union
138134
liveness.edges --> liveness.live_fp
135+
solver.refs_token --> solver.issues_by_file_join
139136
liveness.live --> solver.live_decls_join
140137
liveness.live --> solver.dead_decls_join
138+
solver.dead_modules_empty --> solver.dead_module_issues_join
141139
decls --> solver.live_decls_join
142140
decls --> solver.dead_decls_join
143141
decls --> liveness.annotated_roots_join
@@ -158,7 +156,6 @@ graph TD
158156
exc_refs.resolved_refs_join --> exc_refs.resolved_refs
159157
type_deps.combined_refs_to_union --> type_deps.combined_refs_to
160158
liveness.externally_referenced_union --> liveness.externally_referenced
161-
solver.dead_modules_join --> solver.dead_modules
162159
liveness.all_roots_union --> liveness.all_roots
163160
solver.dead_module_issues_join --> solver.dead_module_issues
164161
solver.dead_decls_join --> solver.dead_decls
@@ -169,6 +166,7 @@ graph TD
169166
decl_refs.with_value_refs_join --> decl_refs.with_value_refs
170167
liveness.type_refs_from_union --> liveness.type_refs_from
171168
type_deps.impl_needing_path2_join --> type_deps.impl_needing_path2
169+
solver.issues_by_file_join --> solver.issues_by_file
172170
liveness.external_type_refs_join --> liveness.external_type_refs
173171
liveness.live_fp --> liveness.live
174172
decl_refs.type_decl_refs_join --> decl_refs.type_decl_refs
@@ -180,7 +178,7 @@ graph TD
180178
classDef joinClass fill:#e6f3ff,stroke:#0066cc
181179
classDef unionClass fill:#fff0e6,stroke:#cc6600
182180
classDef fixpointClass fill:#e6ffe6,stroke:#006600
183-
class decl_refs.with_type_refs_join,decl_refs.combined_join,decl_refs.type_decl_refs_join,liveness.external_type_refs_join,type_deps.impl_needing_path2_join,decl_refs.with_value_refs_join,solver.live_decls_join,type_deps.impl_to_intf_refs_join,decl_refs.value_decl_refs_join,liveness.annotated_roots_join,solver.dead_decls_join,solver.dead_module_issues_join,solver.dead_modules_join,exc_refs.resolved_refs_join,solver.incorrect_dead_decls_join,type_deps.impl_to_intf_refs_path2_join,liveness.external_value_refs_join,type_deps.intf_to_impl_refs_join joinClass
181+
class decl_refs.with_type_refs_join,decl_refs.combined_join,decl_refs.type_decl_refs_join,liveness.external_type_refs_join,solver.issues_by_file_join,type_deps.impl_needing_path2_join,decl_refs.with_value_refs_join,solver.live_decls_join,type_deps.impl_to_intf_refs_join,decl_refs.value_decl_refs_join,liveness.annotated_roots_join,solver.dead_decls_join,solver.dead_module_issues_join,exc_refs.resolved_refs_join,solver.incorrect_dead_decls_join,type_deps.impl_to_intf_refs_path2_join,liveness.external_value_refs_join,type_deps.intf_to_impl_refs_join joinClass
184182
class type_deps.u1_union,type_deps.u2_union,liveness.type_refs_from_union,liveness.all_roots_union,liveness.externally_referenced_union,type_deps.combined_refs_to_union,liveness.value_refs_from_union unionClass
185183
class liveness.live_fp fixpointClass
186184

analysis/reanalyze/src/ReactiveSolver.ml

Lines changed: 63 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -121,53 +121,70 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t)
121121

122122
let transitive = config.DceConfig.run.transitive in
123123

124-
(* Reactive per-file issues - recomputed when dead_decls_by_file changes.
125-
Returns (file, (value_issues, modules_with_reported_values)) where
126-
modules_with_reported_values are modules that have at least one reported dead value.
127-
Module issues are generated separately in collect_issues using dead_modules. *)
124+
(* Reactive per-file issues.
125+
IMPORTANT: in non-transitive mode, warning emission depends on hasRefBelow,
126+
which depends on value_refs_from (cross-file refs). So we must recompute
127+
issues when refs change, not only when the file's dead decls change. *)
128+
let issues_for_file (_file : string) decls =
129+
(* Track modules that have reported values *)
130+
let modules_with_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 8 in
131+
(* shouldReport checks annotations reactively *)
132+
let shouldReport (decl : Decl.t) =
133+
match Reactive.get annotations decl.pos with
134+
| Some FileAnnotations.Live -> false
135+
| Some FileAnnotations.GenType -> false
136+
| Some FileAnnotations.Dead -> false
137+
| None -> true
138+
in
139+
(* Don't emit module issues here - track modules for later *)
140+
let checkModuleDead ~fileName:_ moduleName =
141+
Hashtbl.replace modules_with_values moduleName ();
142+
None (* Module issues generated separately *)
143+
in
144+
(* hasRefBelow: check if decl has any ref from "below" (including cross-file refs) *)
145+
let hasRefBelow =
146+
if transitive then fun _ -> false
147+
else
148+
match value_refs_from with
149+
| None -> fun _ -> false
150+
| Some refs_from ->
151+
(* Must iterate ALL refs since cross-file refs also count as "below" *)
152+
DeadCommon.make_hasRefBelow ~transitive
153+
~iter_value_refs_from:(fun f -> Reactive.iter f refs_from)
154+
in
155+
(* Sort within file and generate issues *)
156+
let sorted = decls |> List.fast_sort Decl.compareForReporting in
157+
let reporting_ctx = DeadCommon.ReportingContext.create () in
158+
let file_issues =
159+
sorted
160+
|> List.concat_map (fun decl ->
161+
DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead
162+
~shouldReport reporting_ctx decl)
163+
in
164+
let modules_list =
165+
Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values []
166+
in
167+
(file_issues, modules_list)
168+
in
128169
let issues_by_file =
129-
Reactive.flatMap ~name:"solver.issues_by_file" dead_decls_by_file
130-
~f:(fun file decls ->
131-
(* Track modules that have reported values *)
132-
let modules_with_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 8 in
133-
(* shouldReport checks annotations reactively *)
134-
let shouldReport (decl : Decl.t) =
135-
match Reactive.get annotations decl.pos with
136-
| Some FileAnnotations.Live -> false
137-
| Some FileAnnotations.GenType -> false
138-
| Some FileAnnotations.Dead -> false
139-
| None -> true
140-
in
141-
(* Don't emit module issues here - track modules for later *)
142-
let checkModuleDead ~fileName:_ moduleName =
143-
Hashtbl.replace modules_with_values moduleName ();
144-
None (* Module issues generated separately *)
145-
in
146-
(* hasRefBelow: check if decl has any ref from "below" (including cross-file refs) *)
147-
let hasRefBelow =
148-
if transitive then fun _ -> false
149-
else
150-
match value_refs_from with
151-
| None -> fun _ -> false
152-
| Some refs_from ->
153-
(* Must iterate ALL refs since cross-file refs also count as "below" *)
154-
DeadCommon.make_hasRefBelow ~transitive
155-
~iter_value_refs_from:(fun f -> Reactive.iter f refs_from)
156-
in
157-
(* Sort within file and generate issues *)
158-
let sorted = decls |> List.fast_sort Decl.compareForReporting in
159-
let reporting_ctx = DeadCommon.ReportingContext.create () in
160-
let file_issues =
161-
sorted
162-
|> List.concat_map (fun decl ->
163-
DeadCommon.reportDeclaration ~config ~hasRefBelow
164-
~checkModuleDead ~shouldReport reporting_ctx decl)
165-
in
166-
let modules_list =
167-
Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values []
168-
in
169-
[(file, (file_issues, modules_list))])
170-
()
170+
match (transitive, value_refs_from) with
171+
| true, _ | false, None ->
172+
Reactive.flatMap ~name:"solver.issues_by_file" dead_decls_by_file
173+
~f:(fun file decls -> [(file, issues_for_file file decls)])
174+
()
175+
| false, Some refs_from ->
176+
(* Create a singleton "refs token" that changes whenever refs_from changes,
177+
and join every file against it so per-file issues recompute. *)
178+
let refs_token =
179+
Reactive.flatMap ~name:"solver.refs_token" refs_from
180+
~f:(fun _posFrom _targets -> [((), ())])
181+
~merge:(fun _ _ -> ())
182+
()
183+
in
184+
Reactive.join ~name:"solver.issues_by_file" dead_decls_by_file refs_token
185+
~key_of:(fun _file _decls -> ())
186+
~f:(fun file decls _token_opt -> [(file, issues_for_file file decls)])
187+
()
171188
in
172189

173190
(* Reactive incorrect @dead: live decls with @dead annotation *)

analysis/reanalyze/src/Reanalyze.ml

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -644,6 +644,8 @@ let runAnalysisAndReport ~cmtRoot =
644644
let parse_argv (argv : string array) : string option =
645645
let analysisKindSet = ref false in
646646
let cmtRootRef = ref None in
647+
(* CLI override for transitive mode (overrides rescript.json if provided). *)
648+
let transitive_override : bool option ref = ref None in
647649
let usage = "reanalyze version " ^ Version.version in
648650
let versionAndExit () =
649651
print_endline usage;
@@ -678,6 +680,14 @@ let parse_argv (argv : string array) : string option =
678680
path" );
679681
("-ci", Unit (fun () -> Cli.ci := true), "Internal flag for use in CI");
680682
("-config", Unit setConfig, "Read the analysis mode from rescript.json");
683+
( "-transitive",
684+
Unit (fun () -> transitive_override := Some true),
685+
"Force transitive reporting (overrides rescript.json \
686+
reanalyze.transitive)" );
687+
( "-no-transitive",
688+
Unit (fun () -> transitive_override := Some false),
689+
"Disable transitive reporting (overrides rescript.json \
690+
reanalyze.transitive)" );
681691
("-dce", Unit (fun () -> setDCE None), "Eperimental DCE");
682692
("-debug", Unit (fun () -> Cli.debug := true), "Print debug information");
683693
( "-dce-cmt",
@@ -767,6 +777,9 @@ let parse_argv (argv : string array) : string option =
767777
let current = ref 0 in
768778
Arg.parse_argv ~current argv speclist print_endline usage;
769779
if !analysisKindSet = false then setConfig ();
780+
(match !transitive_override with
781+
| None -> ()
782+
| Some b -> RunConfig.transitive b);
770783
!cmtRootRef
771784

772785
(** Default socket location invariant:

0 commit comments

Comments
 (0)