diff --git a/CHANGELOG.md b/CHANGELOG.md index 1389c93868..da34e45ac9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ #### :bug: Bug fix +- 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 + #### :memo: Documentation #### :nail_care: Polish diff --git a/analysis/reanalyze/README.md b/analysis/reanalyze/README.md index 4b97942ac1..4161c80fe7 100644 --- a/analysis/reanalyze/README.md +++ b/analysis/reanalyze/README.md @@ -60,6 +60,8 @@ rescript-tools reanalyze -config -reactive -timing -runs 3 | `-churn n` | Remove/re-add n random files between runs (incremental correctness/perf testing) | | `-timing` | Report timing of analysis phases | | `-mermaid` | Output Mermaid diagram of reactive pipeline (to stderr) | +| `-transitive` | Force transitive reporting (overrides rescript.json) | +| `-no-transitive` | Disable transitive reporting (overrides rescript.json) | | `-debug` | Print debug information | | `-json` | Output in JSON format | | `-ci` | Internal flag for CI mode | @@ -68,6 +70,18 @@ rescript-tools reanalyze -config -reactive -timing -runs 3 See [ARCHITECTURE.md](ARCHITECTURE.md) for details on the analysis pipeline. +### Regenerating the checked-in reactive pipeline diagram + +`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). + +To regenerate it: + +```bash +# Run from any ReScript project (so -config works), then capture stderr: +rescript-tools reanalyze -config -reactive -no-transitive -mermaid \ + >/dev/null 2> analysis/reanalyze/diagrams/reactive-pipeline-full.mmd +``` + The DCE analysis is structured as a pure pipeline: 1. **MAP** - Process each `.cmt` file independently → per-file data diff --git a/analysis/reanalyze/diagrams/reactive-pipeline-full.mmd b/analysis/reanalyze/diagrams/reactive-pipeline-full.mmd index cf2fc700d4..becf848391 100644 --- a/analysis/reanalyze/diagrams/reactive-pipeline-full.mmd +++ b/analysis/reanalyze/diagrams/reactive-pipeline-full.mmd @@ -25,7 +25,6 @@ graph TD solver.live_decls[solver.live_decls] type_deps.impl_decls[type_deps.impl_decls] liveness.all_roots[liveness.all_roots] - solver.dead_modules[solver.dead_modules] liveness.external_type_refs[liveness.external_type_refs] decl_refs.combined[decl_refs.combined] type_refs_from[type_refs_from] @@ -34,20 +33,20 @@ graph TD liveness.external_value_refs[liveness.external_value_refs] liveness.value_refs_from[liveness.value_refs_from] value_refs_from[value_refs_from] - solver.modules_with_dead[solver.modules_with_dead] solver.dead_decls[solver.dead_decls] exception_refs_collection[exception_refs_collection] type_deps.intf_decls[type_deps.intf_decls] file_data_collection[file_data_collection] solver.dead_module_issues[solver.dead_module_issues] decl_refs.with_type_refs[decl_refs.with_type_refs] - solver.modules_with_live[solver.modules_with_live] decl_refs.type_decl_refs[decl_refs.type_decl_refs] files[files] solver.modules_with_reported[solver.modules_with_reported] liveness.externally_referenced[liveness.externally_referenced] liveness.edges[liveness.edges] + solver.refs_token[solver.refs_token] liveness.live[liveness.live] + solver.dead_modules_empty[solver.dead_modules_empty] decls[decls] type_deps.intf_to_impl_refs_join{join} liveness.external_value_refs_join{join} @@ -57,7 +56,6 @@ graph TD exc_refs.resolved_refs_join{join} type_deps.combined_refs_to_union{union} liveness.externally_referenced_union{union} - solver.dead_modules_join{join} liveness.all_roots_union{union} solver.dead_module_issues_join{join} solver.dead_decls_join{join} @@ -68,6 +66,7 @@ graph TD decl_refs.with_value_refs_join{join} liveness.type_refs_from_union{union} type_deps.impl_needing_path2_join{join} + solver.issues_by_file_join{join} liveness.external_type_refs_join{join} liveness.live_fp{fixpoint} decl_refs.type_decl_refs_join{join} @@ -103,24 +102,22 @@ graph TD type_deps.decl_by_path -->|flatMap| type_deps.same_path_refs type_deps.u2 --> type_deps.combined_refs_to_union solver.live_decls --> solver.incorrect_dead_decls_join - solver.live_decls -->|flatMap| solver.modules_with_live type_deps.impl_decls --> type_deps.impl_needing_path2_join type_deps.impl_decls --> type_deps.impl_to_intf_refs_join liveness.all_roots --> liveness.live_fp - solver.dead_modules --> solver.dead_module_issues_join liveness.external_type_refs --> liveness.externally_referenced_union decl_refs.combined -->|flatMap| liveness.edges type_refs_from --> liveness.type_refs_from_union liveness.type_refs_from --> liveness.external_type_refs_join liveness.type_refs_from --> decl_refs.type_decl_refs_join - solver.dead_decls_by_file -->|flatMap| solver.issues_by_file + solver.dead_decls_by_file --> solver.issues_by_file_join liveness.external_value_refs --> liveness.externally_referenced_union liveness.value_refs_from --> liveness.external_value_refs_join liveness.value_refs_from --> decl_refs.value_decl_refs_join + value_refs_from -->|flatMap| solver.refs_token value_refs_from --> liveness.value_refs_from_union - solver.modules_with_dead --> solver.dead_modules_join solver.dead_decls -->|flatMap| solver.dead_decls_by_file - solver.dead_decls -->|flatMap| solver.modules_with_dead + solver.dead_decls -->|flatMap| solver.dead_modules_empty exception_refs_collection --> exc_refs.resolved_refs_join type_deps.intf_decls --> type_deps.intf_to_impl_refs_join file_data_collection -->|flatMap| files @@ -131,13 +128,14 @@ graph TD file_data_collection -->|flatMap| annotations file_data_collection -->|flatMap| decls decl_refs.with_type_refs --> decl_refs.combined_join - solver.modules_with_live --> solver.dead_modules_join decl_refs.type_decl_refs --> decl_refs.with_type_refs_join solver.modules_with_reported --> solver.dead_module_issues_join liveness.externally_referenced --> liveness.all_roots_union liveness.edges --> liveness.live_fp + solver.refs_token --> solver.issues_by_file_join liveness.live --> solver.live_decls_join liveness.live --> solver.dead_decls_join + solver.dead_modules_empty --> solver.dead_module_issues_join decls --> solver.live_decls_join decls --> solver.dead_decls_join decls --> liveness.annotated_roots_join @@ -158,7 +156,6 @@ graph TD exc_refs.resolved_refs_join --> exc_refs.resolved_refs type_deps.combined_refs_to_union --> type_deps.combined_refs_to liveness.externally_referenced_union --> liveness.externally_referenced - solver.dead_modules_join --> solver.dead_modules liveness.all_roots_union --> liveness.all_roots solver.dead_module_issues_join --> solver.dead_module_issues solver.dead_decls_join --> solver.dead_decls @@ -169,6 +166,7 @@ graph TD decl_refs.with_value_refs_join --> decl_refs.with_value_refs liveness.type_refs_from_union --> liveness.type_refs_from type_deps.impl_needing_path2_join --> type_deps.impl_needing_path2 + solver.issues_by_file_join --> solver.issues_by_file liveness.external_type_refs_join --> liveness.external_type_refs liveness.live_fp --> liveness.live decl_refs.type_decl_refs_join --> decl_refs.type_decl_refs @@ -180,7 +178,7 @@ graph TD classDef joinClass fill:#e6f3ff,stroke:#0066cc classDef unionClass fill:#fff0e6,stroke:#cc6600 classDef fixpointClass fill:#e6ffe6,stroke:#006600 - 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 + 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 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 class liveness.live_fp fixpointClass diff --git a/analysis/reanalyze/src/ReactiveSolver.ml b/analysis/reanalyze/src/ReactiveSolver.ml index 009d64d3fd..dbe21b2b43 100644 --- a/analysis/reanalyze/src/ReactiveSolver.ml +++ b/analysis/reanalyze/src/ReactiveSolver.ml @@ -121,53 +121,70 @@ let create ~(decls : (Lexing.position, Decl.t) Reactive.t) let transitive = config.DceConfig.run.transitive in - (* Reactive per-file issues - recomputed when dead_decls_by_file changes. - Returns (file, (value_issues, modules_with_reported_values)) where - modules_with_reported_values are modules that have at least one reported dead value. - Module issues are generated separately in collect_issues using dead_modules. *) + (* Reactive per-file issues. + IMPORTANT: in non-transitive mode, warning emission depends on hasRefBelow, + which depends on value_refs_from (cross-file refs). So we must recompute + issues when refs change, not only when the file's dead decls change. *) + let issues_for_file (_file : string) decls = + (* Track modules that have reported values *) + let modules_with_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 8 in + (* shouldReport checks annotations reactively *) + let shouldReport (decl : Decl.t) = + match Reactive.get annotations decl.pos with + | Some FileAnnotations.Live -> false + | Some FileAnnotations.GenType -> false + | Some FileAnnotations.Dead -> false + | None -> true + in + (* Don't emit module issues here - track modules for later *) + let checkModuleDead ~fileName:_ moduleName = + Hashtbl.replace modules_with_values moduleName (); + None (* Module issues generated separately *) + in + (* hasRefBelow: check if decl has any ref from "below" (including cross-file refs) *) + let hasRefBelow = + if transitive then fun _ -> false + else + match value_refs_from with + | None -> fun _ -> false + | Some refs_from -> + (* Must iterate ALL refs since cross-file refs also count as "below" *) + DeadCommon.make_hasRefBelow ~transitive + ~iter_value_refs_from:(fun f -> Reactive.iter f refs_from) + in + (* Sort within file and generate issues *) + let sorted = decls |> List.fast_sort Decl.compareForReporting in + let reporting_ctx = DeadCommon.ReportingContext.create () in + let file_issues = + sorted + |> List.concat_map (fun decl -> + DeadCommon.reportDeclaration ~config ~hasRefBelow ~checkModuleDead + ~shouldReport reporting_ctx decl) + in + let modules_list = + Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values [] + in + (file_issues, modules_list) + in let issues_by_file = - Reactive.flatMap ~name:"solver.issues_by_file" dead_decls_by_file - ~f:(fun file decls -> - (* Track modules that have reported values *) - let modules_with_values : (Name.t, unit) Hashtbl.t = Hashtbl.create 8 in - (* shouldReport checks annotations reactively *) - let shouldReport (decl : Decl.t) = - match Reactive.get annotations decl.pos with - | Some FileAnnotations.Live -> false - | Some FileAnnotations.GenType -> false - | Some FileAnnotations.Dead -> false - | None -> true - in - (* Don't emit module issues here - track modules for later *) - let checkModuleDead ~fileName:_ moduleName = - Hashtbl.replace modules_with_values moduleName (); - None (* Module issues generated separately *) - in - (* hasRefBelow: check if decl has any ref from "below" (including cross-file refs) *) - let hasRefBelow = - if transitive then fun _ -> false - else - match value_refs_from with - | None -> fun _ -> false - | Some refs_from -> - (* Must iterate ALL refs since cross-file refs also count as "below" *) - DeadCommon.make_hasRefBelow ~transitive - ~iter_value_refs_from:(fun f -> Reactive.iter f refs_from) - in - (* Sort within file and generate issues *) - let sorted = decls |> List.fast_sort Decl.compareForReporting in - let reporting_ctx = DeadCommon.ReportingContext.create () in - let file_issues = - sorted - |> List.concat_map (fun decl -> - DeadCommon.reportDeclaration ~config ~hasRefBelow - ~checkModuleDead ~shouldReport reporting_ctx decl) - in - let modules_list = - Hashtbl.fold (fun m () acc -> m :: acc) modules_with_values [] - in - [(file, (file_issues, modules_list))]) - () + match (transitive, value_refs_from) with + | true, _ | false, None -> + Reactive.flatMap ~name:"solver.issues_by_file" dead_decls_by_file + ~f:(fun file decls -> [(file, issues_for_file file decls)]) + () + | false, Some refs_from -> + (* Create a singleton "refs token" that changes whenever refs_from changes, + and join every file against it so per-file issues recompute. *) + let refs_token = + Reactive.flatMap ~name:"solver.refs_token" refs_from + ~f:(fun _posFrom _targets -> [((), ())]) + ~merge:(fun _ _ -> ()) + () + in + Reactive.join ~name:"solver.issues_by_file" dead_decls_by_file refs_token + ~key_of:(fun _file _decls -> ()) + ~f:(fun file decls _token_opt -> [(file, issues_for_file file decls)]) + () in (* Reactive incorrect @dead: live decls with @dead annotation *) diff --git a/analysis/reanalyze/src/Reanalyze.ml b/analysis/reanalyze/src/Reanalyze.ml index 36638026f4..56c879341b 100644 --- a/analysis/reanalyze/src/Reanalyze.ml +++ b/analysis/reanalyze/src/Reanalyze.ml @@ -644,6 +644,8 @@ let runAnalysisAndReport ~cmtRoot = let parse_argv (argv : string array) : string option = let analysisKindSet = ref false in let cmtRootRef = ref None in + (* CLI override for transitive mode (overrides rescript.json if provided). *) + let transitive_override : bool option ref = ref None in let usage = "reanalyze version " ^ Version.version in let versionAndExit () = print_endline usage; @@ -678,6 +680,14 @@ let parse_argv (argv : string array) : string option = path" ); ("-ci", Unit (fun () -> Cli.ci := true), "Internal flag for use in CI"); ("-config", Unit setConfig, "Read the analysis mode from rescript.json"); + ( "-transitive", + Unit (fun () -> transitive_override := Some true), + "Force transitive reporting (overrides rescript.json \ + reanalyze.transitive)" ); + ( "-no-transitive", + Unit (fun () -> transitive_override := Some false), + "Disable transitive reporting (overrides rescript.json \ + reanalyze.transitive)" ); ("-dce", Unit (fun () -> setDCE None), "Eperimental DCE"); ("-debug", Unit (fun () -> Cli.debug := true), "Print debug information"); ( "-dce-cmt", @@ -767,6 +777,9 @@ let parse_argv (argv : string array) : string option = let current = ref 0 in Arg.parse_argv ~current argv speclist print_endline usage; if !analysisKindSet = false then setConfig (); + (match !transitive_override with + | None -> () + | Some b -> RunConfig.transitive b); !cmtRootRef (** Default socket location invariant: