pattern-matching refactoring: refactor the make_<foo>_matching functions#9520
Conversation
2a921ef to
35a3637
Compare
lambda/matching.ml
Outdated
| let def = Default_environment.specialize head def | ||
| and ctx = Context.specialize head ctx in | ||
| { pm = { cases = []; args = argl; default = def }; ctx; discr = head } | ||
| let get_expr_args_variant_constant = get_expr_args_constant |
There was a problem hiding this comment.
This is the only case where a get_expr_args function is reused. Is there any specific reason?
There was a problem hiding this comment.
After discussing more with @Octachron, I will systematically define all constant get_{pat,expr}_args definitions as equal to the _constant one.
7c4449f to
e58afea
Compare
|
@Octachron I rebased the PR. (On my local machine there is a mysterious failure with a |
e58afea to
2a3a3a7
Compare
| | _ -> fatal_error "Matching.get_expr_args_constr" | ||
| in | ||
| let loc = head_loc ~scopes head in | ||
| let make_field_accesses binding_kind first_pos last_pos argl = |
There was a problem hiding this comment.
Small remark: the function is only called with Alias as a binding_kind.
Octachron
left a comment
There was a problem hiding this comment.
This PR extracts the part of make_matching specific to each pattern head and refactorizes the common code. It seems good to merge once the rest of the CI return green.
Before, each head construction had a `make_<foo>_matching` construct that was responsible for three things: - consuming the argument from the argument list (the "argument" is a piece of lambda code to access the value of the current scrutinee) - building arguments for the subpatterns of the scrutinee and pushing them to the argument list - building a `cell` structure out of this, representing a head group during compilation Only the second point is really specific to each construction. This refactoring turns this second point into a construct-specific `get_expr_args_<foo>` function (similarly to `get_pat_args_<foo>`), and moves the first and third point to a generic `make_matching` function. Note: this commit contains a minor improvement to the location used to force (lazy ..) arguments.
2a3a3a7 to
0651396
Compare
|
Merged. Many thanks again (as usual, but still worth saying) for the fine review work. I will prepare the next PR in the stack. |
This is the next bit of the big pattern-matching refactoring change (#9321) after #9493 has been merged.
(cc @trefis @Octachron)
Before, each head construction had a
make_<foo>_matchingconstruct thatwas responsible for three things:
(the "argument" is a piece of lambda code to access
the value of the current scrutinee)
and pushing them to the argument list
cellstructure out of this, representing a head groupduring compilation
Only the second point is really specific to each construction.
This refactoring turns this second point into a construct-specific
get_expr_args_<foo>function (similarly toget_pat_args_<foo>),and moves the first and third point to a generic
make_matchingfunction.
Note: this PR contains a minor improvement to the location used to
force (lazy ..) arguments, and also removes the redundant expansion
that was discussed in the last PR. (The latter is a separate commit
to ease separate reviewing.)