[Feature Request] Print pattern matching exhaustiveness warnings in rescript syntax
Some (Done _)
->
Some(Done(_))
@bobzhang I'm going to work on this do you think it should be done in syntax codebase as a module for printing patterns (like what we have for outcome tree) or it should be done inside current parmtch.ml module which prints in ml syntax?
cc @IwanKaramazow Do you need outcomtree printer -> res syntax or parsetree -> res syntax?
It seems that pretty_val in parmatch.ml operates on Typedtree.pattern. Am I correct that this pretty prints the error?
If we can convert the Typedtree.pattern to the outcome tree then we can reuse the outcome printer, otherwise we'll need to implement a new one.
@bobzhang do you have a preference for whether this is done in the syntax repo or in the compiler repo?
I had a look at parmatch.ml, it seems that it implements an ad-hoc Typedtree.pattern printer, so
- We just adapted it for rescript syntax (minimal changes, drop the ml syntax for error message)
- Typedtree.pattern --> Untypedast -> Parsetree.pattern -> print
IIRC, outcometree does not have patterns, it is only for values.
@amiralies which route would you prefer?
I think both are okay. the second approach would look something like this
let print_pat (p: Typedtree.pattern) =
let pattern = Untypeast.default_mapper.pat Untypeast.default_mapper p in
let doc = Res_printer.printPattern pattern Res_comments_table.empty in
Res_doc.toString ~width:80 doc
is this ok for the above function to live in Parmatch? if so, how can i access to napkin modules? i'm not familiar with build configurations in the project.
Currently Napkin depends on compiler-libs which is ml directory, so here you have cycles.
You can use a reference let res_pattern_printer = fun _ -> assert false and populate it with the right one in bs_conditional_initial.
When you make the contributions, make sure the diff is small so that it is easy to review, the review itself would take lots of time, thanks!
If you miss any API for pattern printer, @IwanKaramazow is the best to reach out
Okay I got something working but the way Untypeast.default_mapper.pat untypes patterns is not going well with pattern printer in Res_printer so I think i've to introduce another function to untype a pattern.
Now I think it's the right way the we are going to use the pattern printer from Res_printer rathar than a custom pattern printer like the one in parmatch for ml because it results in duplicated logic and there are some edge case like exotic idents/variants etc that are well handled in Res_printer
but the way Untypeast.default_mapper.pat untypes patterns is not going well with pattern printer in Res_printer
Can you elaborate
When converting Tpat_construct and Tpat_record it uses pattern's longident to convert into Ppat_... For example for a constructor like Done the longident is like #$Done0 i think it should use cstr_desc.cstr_name. same story for record labels.
@bobzhang sorry for pinging, should i create a new untypeast mapper and use cstr_name or just go with updating printer in parmatch.ml to print rescript syntax?
This is already done in the meantime.