Repository navigation
Fix compiler crash on unreachable guarded or-patterns - #8896
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #8896 +/- ##
=======================================
Coverage 81.04% 81.04%
=======================================
Files 455 455
Lines 63508 63518 +10
=======================================
+ Hits 51469 51481 +12
+ Misses 12039 12037 -2
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8896 |
|
@codex review |
1 similar comment
|
@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: a00b41e16b
ℹ️ 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 |
cknitt
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adf8ee9208
ℹ️ 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. Hooray! 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". |
A guarded or-pattern after a case that matches everything crashed the
compiler with "Fatal error: Matching.comp_exit". The or-pattern is compiled
to a handler that its rows jump to. Its rows were dropped as unreachable, but
the handler was still compiled; when its guard fails it falls through to the
following cases, and in a match without a default there is nothing to fall
through to. A handler that the compiled match never jumps to is now skipped.
Example (rescript):
let g = () => true
let f = x =>
switch x {
| _ => 2
| 1 | 2 if g() => 3
}
Before (text):
>> Fatal error: Matching.comp_exit
Fatal error: exception Misc.Fatal_error
After (js):
function f(x) {
return 2;
}
Source: compiled with the parent commit and with this one.
Review: Can skipping a handler drop reachable code? The handler is skipped
only when no `Lstaticraise` to its exit occurs in the compiled match, so no
path can reach it.
Area: lambda tests
Output: unchanged, all committed generated JS is byte-identical; programs
that crashed now compile
Tests: +tests/tests/src/unreachable_or_guard_test.res
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JALXUyHhkDqzcB3WuW9Svo
Signed-off-by: Christoph Knittel <ck@cca.io>
The regression test disabled warning 11 for its module. It is now a super_errors fixture, which compiles with all warnings and records the four unused-case warnings; without the fix its snapshot shows the fatal error. Area: tests Output: unchanged Tests: +tests/build_tests/super_errors/fixtures/warning_11_unreachable_guarded_or_pattern.res -tests/tests/src/unreachable_or_guard_test.res Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JALXUyHhkDqzcB3WuW9Svo Signed-off-by: Christoph Knittel <ck@cca.io>
Whether a handler is reachable was checked by scanning the whole compiled match, including function bodies, once per handler. The exits are now collected into a table once, extended with each compiled handler, and function bodies are skipped, since static exits cannot leave a function. Area: lambda Output: unchanged Tests: none, because the output is unchanged; the reachability check is covered by warning_11_unreachable_guarded_or_pattern.res Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JALXUyHhkDqzcB3WuW9Svo Signed-off-by: Christoph Knittel <ck@cca.io>
2 participants
A guarded or-pattern after a case that matches everything crashed the compiler:
This happens with int, char and string constants and in tuples; a fuzzer working on #8893 found it.
Cause: the error is an invariant check in
comp_exit. When a sub-match has no rows left, it jumps to the first entry of its default list, which holds the handlers of the rows that come after it. That list must not be empty if the sub-match can fail. Here, it fails like this:precompile_orcompiles1 | 2 if g() => 3into a handler with an exit number. In the main match, its alternatives become rows that jump to that exit. The handler is one guarded row, and its default list holds the rows after the or-pattern: none._covers every value, so the match is total. A partial match gets a final default that raisesMatch_failure; a total one gets none, so the handler's default list is empty._matches every value, so the main match compiles toreturn 2, and the rows that jump to the handler are dropped as unreachable.compile_orhandlerscompiles the handler anyway. Its guarded row compiles a path for a failing guard: no rows remain and the default list is empty, socomp_exitraises the fatal error.In reachable code the invariant holds: in a total match, a value whose guard fails must be matched by a later row, and later rows are what the default list holds. Here the value is matched by an earlier row (
_), which only happens when the handler is unreachable.Fix: a handler that the compiled match never jumps to is not compiled. "Never jumps to" means no
Lstaticraiseto its exit occurs in the compiled match. The check incomp_exitstays in place, so it still catches real bugs. Making it tolerate an empty default would hide them. Every committed JS snapshot is byte-identical; only programs that used to crash compile differently.New test: the super_errors fixture
warning_11_unreachable_guarded_or_pattern.res, with int, char, string and tuple cases. It expects four unused-case warnings; without the fix, its snapshot shows the fatal error, and each case crashes the compiler on its own.🤖 Generated with Claude Code
https://claude.ai/code/session_01JALXUyHhkDqzcB3WuW9Svo