Skip to content

Fix DCE false positives found on the ReScript compiler sources - #222

Closed
cknitt wants to merge 1 commit into
masterfrom
dce-false-positives-multi-tree
Closed

cknitt wants to merge 1 commit into
masterfrom
dce-false-positives-multi-tree

Conversation

@cknitt

@cknitt cknitt commented Nov 2, 2026

Copy link
Copy Markdown
Member

Running DCE on the ReScript compiler's OCaml sources (OCaml 5.5.2, dune, native + browser profiles) surfaced several classes of false positives. This fixes them.

Fixes

  1. Case-insensitive file systems (macOS, Windows). candidateFilesForUnit probes sibling files as both Unit.cmt and unit.cmt. On a case-insensitive FS both exist, so one file was listed under two paths with different unit keys, and the lookup was rejected as ambiguous. Exceptions raised across modules of a dune wrapped library (raise (Lib.Mod.Exn ...)) were then reported as never raised. Sibling candidates now require an exact directory entry. (Same fix in value-dependency extraction.)
  2. Type re-exports type t = M.t = A | B. Constructors and labels are shared with M.t, but were tracked separately: constructing Reexp.A left Orig.A "never constructed", and reading orig.x left the re-export's label "never read". The original declaration is now resolved through shapes (typeDeclOfPath, OCaml 5.3+) and same-named constructors/labels are linked both ways.
  3. Local module aliases in shape lookup. module Child = Orig has an alias shape without the binding's uid, so moduleShapeOfPath (Pident Child) failed. It now follows the aliased path. This makes (2) work for type t = Child.t = ....
  4. Types in a submodule of an interface (module Color : sig type setting = Auto | Never end) were declared without the submodule in their path (setting.Auto instead of Color.setting.Auto), so they never matched the implementation and the .ml constructors looked unused. processSignatureItem now passes its path to DeadType.addDeclaration.
  5. Analyzing several build trees together (e.g. _build/default and a --profile browser build that contains extra executables). Needed so that uses that exist in only one configuration count.
    • Source paths in dune .cmts are relative to the build root and cmt_builddir is normalized to /workspace_root. When a source isn't found from the cwd or scan root, it is now looked up in the ancestors of the .cmt (dune's .ml-gen wrappers excepted). The directory is remembered so reports and -write read the right file.
    • A unit compiled from the same source in both trees has two copies with equal interface digests but different dependency contexts, which was treated as ambiguous. The copy strictly closest to the consumer (by path components, i.e. in its own build tree) is now used. Ties and differing sources stay ambiguous, so the existing ambiguous-provider-contexts expectation is unchanged.

Effect on the ReScript compiler

Reports dropped from 1221 to 1138. That's about 80 false positives removed plus 13 recovered uses from the browser-only playground. The remaining reports I checked were genuine, apart from known semantics (fields read only via polymorphic =, exception aliases that are only matched).

New tests

  • examples/regression: Type_reexport* (re-export via another module and via a local alias; constructors, labels, and genuinely unused ones still reported) and Nested_sig_type* (types in an .mli submodule), with assertions in run_integration_tests.js. The re-export assertions are gated on OCaml ≥ 5.3.
  • scripts/test_build_trees.js: one library built in two trees with different generated platform.ml, consumers in another directory, analyzed together from the source tree. It fails without the ancestor lookup and without the closest-copy rule.

The case-insensitive FS fix has no automated test, because CI is Linux-only and a case-insensitive FS can't be simulated there. I verified it manually on a macOS-backed mount: a wrapped-library repro went from 4 false "never raised" reports to 0.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Kbf3gLDo8mBJKpNE25WAwT

Running DCE on the ReScript compiler (OCaml 5.5, dune, native and
browser profiles) produced several classes of false positives:

- On case-insensitive file systems (macOS, Windows), a unit's sibling
  .cmt was found under two spellings, so the unit looked ambiguous and
  exceptions raised across modules of a wrapped library were reported
  as never raised. Sibling candidates now need an exact directory entry.
- Type re-exports `type t = M.t = A | B` declare the same constructors
  and labels as `M.t`. They are now linked both ways (OCaml 5.3+, found
  through shapes), also when `M` is a local module alias, whose shape
  carries no uid of its own.
- Types in a submodule of an interface (`module M : sig type t = A end`)
  were declared without the submodule path, so they never matched their
  implementation.
- Several build trees of one project (e.g. two dune profiles) can now be
  analyzed together: sources found only in a build tree (generated
  files) are read from the nearest ancestor of the .cmt that contains
  them, and a unit compiled from the same source in several trees
  resolves to the copy strictly closest to the consumer. Ties stay
  ambiguous, as before.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kbf3gLDo8mBJKpNE25WAwT
@cknitt

cknitt commented Nov 2, 2026

Copy link
Copy Markdown
Member Author

Superseded by #224, now stacked on the CI fix in #223.

@cknitt cknitt closed this Nov 2, 2026
@cknitt
cknitt deleted the dce-false-positives-multi-tree branch November 2, 2026 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant