PR#7169: Rephrase non-exhaustivity warning message to avoid confusion#501
Conversation
typing/parmatch.ml
Outdated
| List.iter check_defined lbl_pat_list; | ||
| Some(all,defined) | ||
|
|
||
| let pretty_record_closure ppf lbl_list = |
There was a problem hiding this comment.
I find the "closure" wording confusing (I think of function closures). You could maybe use "ending" instead?
There was a problem hiding this comment.
Maybe pretty_record_elision_mark to be more explicit?
|
Thanks a lot for this implementation work! cc @maranget: he is the expert on the implementation aspect of parmatch, and I would like to know whether he agrees with the proposed change in phrasing of the warning. |
|
Looks good to me. Could you please rebase (unfortunately, there are conflicts in the testsuite) and add an entry to Changes? |
389cc3b to
d34a05b
Compare
|
Done: I have rebased the PR, squashed the fix commit, and added an entry to Changes. |
|
Thanks! |
|
I'm worried that there may have been a problem during the rebase; I tested locally with intent to merge and the testsuite fails on my machine. |
|
So I reran the test from a clean state and the tests failure are gone. This may come from the fact that |
PR#7169: Rephrase non-exhaustivity warning message to avoid confusion
Safepoints lazy fix -- Attempt 2
This PR proposes to rephrase the warning for non-exhaustive patterns to make it clearer that counter-examples printed by the warning are patterns or cases and not values (that could be constructed).
For instance, with the current warning message, the following code
raises a warning
This warning is confusing for beginners on at least two fronts. First,
{left=1}is not a valid ocaml value. A beginner trying to see what happens when computingf {left=1}may be lost in the error raised by the compiler. The counter-example{left=1}is a pattern and should not be introduced as a value. Rewording the warning message assolves this ambiguity without decreasing the information density of the warning.
This is the object of the first commit in this PR.
Second, the pattern
{left=1}elides the fieldrightwithout any indication that fields have been elided. Adding a trailing; _when fields have been elided would correct this lack of information and reinforces the notion that the counter-example is a pattern:The second commit in this PR modifies the pattern pretty printer in
typing\parmatch.mlto add this trailing
_when record patterns are not closed.See also mantis:7169 for more discussions.