pattern-matching compiler: refactor the toplevel handling of partiality #9651
Conversation
gasche
force-pushed
the
rematch-partial-handler
branch
from
442dddb
to
e1932fc
Compare
|
This pattern-matching refactoring PR is still open. Anyone is free to help reviewing it! |
gasche
force-pushed
the
rematch-partial-handler
branch
from
e1932fc
to
07b128c
Compare
|
(recently I wondered about which of the pattern-matching PR are still in-flight; this one is) |
lambda/matching.ml
Outdated
| with Unused -> assert false | ||
|
|
||
| (* ; partial_function loc () *) | ||
| List.fold_right2 (bind Strict) idl paraml lam, total |
There was a problem hiding this comment.
This moves the bindings below the handler for the match failure. However, making variables more local seems good.
| handler (fun partial pm -> | ||
| compile_match ~scopes None partial | ||
| (Context.start (List.length paraml)) pm | ||
| ) |
There was a problem hiding this comment.
Beyond the removal of the Unused handler that indeed seemed erroneous, this also optimizes the Total case by avoiding creating a default environment and checking if we need to install a match failure handler.
There was a problem hiding this comment.
The factorization of toplevel handlers seems a worthwhile simplification.
Indeed, both do_for_multiple_match and compile_match re-implemented independently an equivalent Partial and Total branch.
The fact that the remaining handler, for_tupled_function only implemented the Partial branch is another point in favor of the PR.
Concerning the Unused exception, the new assert false seems as correct as the preexisting one in the non-optimized compilation path: if it is possible to reach this assert false, with for_tupled_function, the same pattern match compiled with for_function should also reach it.
|
I just rebased the PR against trunk. The rebase was non-obvious due to the interaction with the |
gasche
force-pushed
the
rematch-partial-handler
branch
from
07b128c
to
9a57ea4
Compare
This comes from a suggestion by Florian Angeletti in ocaml#9447 (comment)
This appears to change the function behavior with respect to the Unused exception, but we believe that the change is correct. It makes the code more consistent with other toplevel compilation functions.
gasche
force-pushed
the
rematch-partial-handler
branch
from
9a57ea4
to
67ba8c3
Compare
This PR is built on top of #9647, it shares the toplevel logic to call pattern-matching compilation functions.
(The PR contains a change to the handling of the
Unusedexception byfor_tupled_function. I believe the change is correct, and I checked with @maranget who agrees in principle but isn't sure about the details.)(cc @maranget @trefis @Octachron)