Fix DCE false positives for functors constrained by a named module type - #217
Conversation
Values in a functor body whose result is constrained to a module type defined in another module (via .mli or inline) are reported dead even when instances are used from live code.
When a module or functor result is constrained to a module type defined in another file (via .mli or inline), the typed tree's declaration dependencies link each implementation value to the `val` item of that module type, which lives in another compilation unit's cmt. The extractor only loaded the local cmt and cmti, so those links were dropped and every value in the functor body was reported dead. - Index all cmt/cmti files by compilation unit up front and resolve foreign uids from them (OCaml 5.3+). - Defer processing of value dependencies until all files are scanned. When the signature side is not a declaration (a `val` inside a named module type), forward the references made to it onto the implementation rather than treating it as an always-live reference, so unused items in the signature are still reported.
- In ReScript project mode, collect the cmt files of every source directory before processing any, so declaration dependencies across directories resolve. - When forwarding references from a module-type val item onto its implementation, also forward the recorded optional-argument calls. Otherwise a call like H.f ~x:1 () through a constrained functor result would report x as never used. Covered by with_opt in the fixture.
The analysis root can contain several units with the same name (e.g. unwrapped libraries in different directories). Their declarations share uids, so merging every candidate into one table first-one-wins could resolve a functor's module type constraint to the wrong unit and leave a used value reported dead. Cross-unit lookups now keep one declaration table per candidate file, narrow the candidates down using the interface digest the importer recorded in cmt_imports, and consult every remaining candidate when the digest cannot disambiguate. Adds a regression fixture with two unwrapped libraries defining Dup_sig.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc2d4de2bb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| never be resolved as dead. Forward the references made to the | ||
| signature item onto the implementation instead. *) | ||
| DeadOptionalArgs.forwardDelayedItems ~posFrom ~posTo; | ||
| ValueReferences.find posFrom |
There was a problem hiding this comment.
On OCaml 5.3+, when two functors or modules are constrained by the same named module type, references such as H1.f and H2.f share the source position of that module type's val f. Every implementation dependency performs this global lookup, so a call through H1.f is replayed onto both implementations and an unused H2.f is incorrectly considered live. Preserve the originating constrained module or application when forwarding instead of broadcasting every reference stored at the shared signature position.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I'm aware of this and that's what #218 is for. The scope of this PR is just to silence false positives.
|
@cristianoc please note the stacked pr on top of this. Regarding the codex comment. |
1 participant
When a functor result is constrained to a module type defined in another file, every value in the functor body is reported dead even when instances are used from live code.
Before:
find_optandhash.Makeare reported dead. Expected: onlyunused_in_sigandtruly_dead.On OCaml 5.3+ the
find_optbinding depends on the uid ofval find_optingen.ml, but the dependency extractor only loaded the local.cmt/.cmti, so the link was dropped.Fix:
valin a module type rather than a declaration, forward its references onto theimplementation, so unused signature items are still reported.
Fixtures cover
.mliand inline constraints, plus a duplicate-unit regre