Add tentative support for OCaml 5.3#203
Conversation
The cmt format has changed, this tries to support value dependencies just based on the type of what's available in the new `Cmt_format.cmt_infos`. #202
|
|
||
| let extractValueDependencies (cmt_infos : CL.Cmt_format.cmt_infos) = | ||
| #if OCAML_VERSION >= (5, 3, 0) | ||
| let deps = ref [] in |
There was a problem hiding this comment.
This tentatively tries to extract the old cmt_value_dependencies.
|
The dependencies seems to have changed, currently none are found, and this attempt does not seem to work either: let extractValueDependencies (cmt_infos : Cmt_format.cmt_infos) =
#if OCAML_VERSION >= (5, 3, 0)
let deps = ref [] in
let ident_occurrences = cmt_infos.cmt_ident_occurrences in
Printf.eprintf "ident_occurrences: %d\n" (List.length ident_occurrences);
let process_id ({Location.txt; loc}, res) = match res with
| Shape_reduce.Resolved uid | Resolved_alias (uid, _) | Approximated (Some uid) ->
(match Types.Uid.Tbl.find_opt cmt_infos.cmt_uid_to_decl uid with
| Some (Value v) ->
deps := (loc, v.val_loc) :: !deps
| Some (Type td) ->
deps := (loc, td.typ_loc) :: !deps
| Some (Value_binding vb) ->
deps := (loc, vb.vb_loc) :: !deps
| Some (Module_binding mb) ->
deps := (loc, mb.mb_loc) :: !deps
| Some (Module_type mt) ->
deps := (loc, mt.mtd_loc) :: !deps
| Some (Constructor cd) ->
deps := (loc, cd.cd_loc) :: !deps
| Some (Class c) ->
deps := (loc, c.ci_loc) :: !deps
| Some (Class_type ct) ->
deps := (loc, ct.ci_loc) :: !deps
| Some (Extension_constructor ec) ->
deps := (loc, ec.ext_loc) :: !deps
| Some (Label ld) ->
deps := (loc, ld.ld_loc) :: !deps
| Some (Module m) ->
deps := (loc, m.md_loc) :: !deps
| Some (Module_substitution ms) ->
deps := (loc, ms.ms_loc) :: !deps
| _ -> ())
| _ -> () in
List.iter process_id ident_occurrences;
List.rev !depsMoving this to draft. |
|
Hi @cristianoc ! Shameless plug here, this problem has been solved in the dead_code_analyzer (LexiFi/dead_code_analyzer#38) and you might be inspired by a similar solution. TL;DR: the |
OCaml 5.3 replaces cmt_value_dependencies with cmt_declaration_dependencies, which references declarations by Shape.Uid. Recover value locations by looking up uids in cmt_uid_to_decl, falling back to the companion .cmti and to a cross-cmt table populated as files are processed (the per-file table is not guaranteed to be complete). Thanks to @fantazio for pointing to the same approach used in LexiFi's dead_code_analyzer (LexiFi/dead_code_analyzer#38) in #203 (comment).
- ocaml/setup-ocaml@v2 -> @V3 (v2 fails to install on current ubuntu-latest and cache@v4 returns 400). v3 manages opam caching itself, so the separate actions/cache step is removed. - Drop the ocaml.4.08 matrix row: the base compiler is no longer in opam-repository (same reason as the 4.06 removal in 2e0caa0). - Add ocaml.5.3.x so the branch's target version is actually built. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The rescript-target jobs only existed to produce per-OS binaries for the npm package. Since 2e0caa0 removed 406 support they share no logic with ReScript anymore, and we no longer distribute the binary that way. Drop: - The macos-13/macos-14/windows-latest builds and the ubuntu rescript job. - The Compress + upload-artifact steps and the npm_pack job. - The matrix.build/matrix.test indirection (now uniform across jobs). - The Windows-only "Set git to use LF" step. - npm ci at the root (package has no dependencies). CI is now the 5-version OCaml matrix (4.14 - 5.3) on ubuntu-latest, running dune build and the integration tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The cmtUidToDecl global table plus the rememberCmtFile pre-walk in processCmtFiles were intended to cover the "cmt_uid_to_decl may not be complete" case fantazio flagged, but knockout tests show that for the current example set, per-file cmt_uid_to_decl plus the companion .cmti read are sufficient: disabling the accumulator leaves deadcode.txt byte-identical. Drop it; if a later example exposes the missing case, reintroduce. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Sweet, thank you!
This seems to work.
…On Mon, 11 May 2026 at 18:19, Corentin De Souza ***@***.***> wrote:
*fantazio* left a comment (rescript-lang/reanalyze#203)
<#203 (comment)>
Hi @cristianoc <https://github.com/cristianoc> ! Shameless plug here,
this problem has been solved in the dead_code_analyzer (
LexiFi/dead_code_analyzer#38
<LexiFi/dead_code_analyzer#38>) and you might be
inspired by a similar solution.
TL;DR: the cmt_infos.cmt_value_dependencies is replaced by
cmt_infos.cmt_declaration_dependencies, and one needs to read the
cmt_infos.cmt_uid_to_decl to recreate the old association. A very
important trick is that the table in the current cmt_infos may not be
complete (see https://github.com/ocaml/ocaml/blob/5.3.0/typing/shape.mli),
in which case it must be read in multiple .cmt and .cmti files to gather
all the necessary information.
—
Reply to this email directly, view it on GitHub
<#203 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4YVF2O7H5YMVSMLYZR4OT42H4PTAVCNFSM6AAAAACYZMM2TCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHM2DIMRSGU3DEOJWGQ>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hey @cristianoc I can see this is WIP but I took this branch for a spin on the rescript-compiler itself. I built it against a 5.3 switch, ran dune build @check, then -dce-cmt _build/default. It runs cleanly end to end, but the cross-module reference tracking looks pretty off. It flags a lot of clearly-live stuff as dead. The one that jumped out was ast_helper is a dead module for example. Same thing for Lam_compile and Js_dump. Anything local to a module seems right, but as soon as something is exported via its .mli and used elsewhere, those uses seem to disappear and it gets called dead. Either way still lots of useable warnings for dce, excited for this to work 💪🏼 |
Might be worth checking whether it has to do with this (copied from the readme):
|
I ran it with |
The cmt format has changed, this tries to support value dependencies just based on the type of what's available in the new
Cmt_format.cmt_infos.#202