effect syntax: effect patterns as computation patterns#13135
Conversation
At the typechecker level, effect cases are similar to the other computation patterns. This commit refactorize the parsing and typechecking of effect cases to fully fall in the computation pattern category
Octachron
force-pushed
the
effect-pattern-as-computation-pattern
branch
from
e50ee90 to
aa28263
Compare
There was a problem hiding this comment.
Made a round of review with a few comments. I did not enter deeply into the typecore changes but it looks all-rigth from afar.
(I don't know what is the correct etiquette regarding formatting nitpicking, they are quite a few in the translcore module...)
| and transl_cont (type a) cont pat_cont body = | ||
| match pat_cont with | ||
| | Some (id2,_) -> Llet(Alias, Pgenval, id2, Lvar cont, body) | ||
| | None -> body |
There was a problem hiding this comment.
| | None -> body | |
| | None -> body |
| | None, None -> assert false | ||
| let sp = split_pattern c_lhs in | ||
| match sp.value, sp.exn with | ||
| | None, None -> assert false |
There was a problem hiding this comment.
| | None, None -> assert false | |
| | None, None -> assert false |
| (lbl, ids_kinds, rhs) :: static_handlers | ||
| in | ||
| let val_cases, exn_cases, static_handlers = | ||
| let val_cases, exn_cases, static_handlers = |
There was a problem hiding this comment.
| let val_cases, exn_cases, static_handlers = | |
| let val_cases, exn_cases, static_handlers = |
| ^^^ | ||
| Line 3, characters 4-19: | ||
| 3 | | effect A _, [k] -> () | ||
| ^^^^^^^^^^^^^^^ |
There was a problem hiding this comment.
So this position is less precise than before ?
| add_row vpss vp, add_row epss ep | ||
| ) patl ([], []) | ||
| let val_pss, exn_pss, eff_pss = | ||
| List.fold_right (fun pat (vpss, epss,effss)-> |
There was a problem hiding this comment.
| List.fold_right (fun pat (vpss, epss,effss)-> | |
| List.fold_right (fun pat (vpss, epss, effss)-> |
| ident_create "Undefined_recursive_module" | ||
| and ident_continuation_already_taken = ident_create "Continuation_already_taken" | ||
|
|
||
|
|
There was a problem hiding this comment.
What is the compiler policy about unrelated formatting changes ?
There was a problem hiding this comment.
There are generally not accepted. If this case, it looks like this is reverting the addition of a newline in the effect syntax PR.
| type _ pattern_category = | ||
| | Value : value pattern_category | ||
| | Computation : computation pattern_category | ||
| | Computation : {exn:bool} -> computation pattern_category |
There was a problem hiding this comment.
I think commenting about the role of exn would be useful here.
| | Ppat_any -> None | ||
| | Ppat_var name -> | ||
| let ty = instance cont_ty in | ||
| let id, _uid = |
There was a problem hiding this comment.
I think it would be useful to store the uid of the continuation in the Tpat_effect node in the same way as we do for Tpat_var and Tpat_alias. (Merlin relies on this information when querying local occurrences from an identifier's declaration.)
4 participants
This PR is meant to be read at the top of the effect-syntax PR(#12309). In particular, only the last commit is new.
This commit aims at making the typechecker part of the effect syntax changes smaller.
This is done by merging back effect patterns into the computation pattern umbrella in the typechecker part of the compiler. Splitting patterns by kind is then done when compiling to the lambda IR.
This change also has the advantage of making the error messages for non-toplevel computation patterns more uniform:
Incidentally, it makes the compiler support cases containing mixed effect, exception and value cases:
However, the feature is mostly useless, since effect types require type equations to be useful, and we discard type equations introduced under or-patterns. In other words, it is impossible to bind the continuation in the pattern above: