Fix DCE false positives for exception aliases raised across modules - #220
Conversation
230a07a to
968019b
Compare
|
@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: 968019b2b5
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e165ee725
ℹ️ 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".
83fd515 to
39f1c8f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 39f1c8fef9
ℹ️ 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".
|
@codex review |
|
Codex Review: Didn't find any major issues. 🎉 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
An exception declared as an alias (exception Alias = A) and raised from another module via the alias name is reported as never raised. The expected output only reports DeadAlias, which is genuinely unused.
For exception X = Y, the constructor description at a raise site in
another module carries the location of Y (ghost for predefined
exceptions), so DeadException falls back to resolving the declaration by
path. That lookup never matched for OCaml projects:
- The declaration key used the source file name ("location"), while
the path in the typed tree uses the module name ("Location").
- Paths in dune wrapped libraries and executables are prefixed with the
wrapper module (Dune__exe, Deadcode_fixture), which is not part of the
declaration key.
- Exception declarations count references from the type reference
table, but the delayed path only recorded a value reference.
Normalize the module name in the key, retry the lookup with the
outermost module dropped, and record a type reference like the
non-delayed case.
e50da11 to
93ee257
Compare
|
@cristianoc 2 more prs to merge in this stack. I've just rebased after merging the 2 below. All codex reviews were satisfied and the rebase doesn't change any code. |
Great. Go ahead! |
Awesome, this seems to be the last of at least the false positives that were coming through on the compiler. There may well be more unreported dead code. But I will do one more session on the compiler for clean up and then I think we could use it in CI etc 👍🏼 |
2 participants
Stacked on #218. An exception alias can be reported as never raised even when live code raises it from another module.
Before:
Usedcan be reported as never raised. Expected: onlyUnused.References to exception aliases can have ghost declaration locations, requiring deferred lookup by module path. Dune wrappers and mismatched compilation-unit names prevented these references from reaching the alias declaration.
Fix:
Regression fixtures cover used and unused aliases across modules, including wrapped libraries, and ensure references to external exceptions do not keep unrelated local aliases live.
Validated with
dune buildandnpm teston OCaml 4.14.2, 5.2.1, 5.3.0, and 5.5.0. Also verified that the ReScript compiler’s usedLocation.Already_displayed_erroralias is no longer reported.