Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Matching: more debug #12553

Merged
merged 4 commits into from Sep 13, 2023
Merged

Matching: more debug #12553

merged 4 commits into from Sep 13, 2023

Conversation

gasche
Copy link
Member

@gasche gasche commented Sep 12, 2023

This is an update on the now-merged #12532, adding smaller, more specialized changes to the pattern-matching debug output that were useful to track #7241.

The notable change is in the first commit. Quoting the commit message:

Before this commit, a record pattern that accepts all values for all
its fields will be pretty-printed (in exhaustivity counter-examples
and in the debug output of the pattern-matching compiler0 as
a wildcard pattern _. This is correct for users, but it gives
inaccurate information in the debug output, as the two patterns are
distinct.

Instead I propose to show { _ } in this case, to mean a record whose
field values do not matter. This happens to not be valid OCaml syntax,
but it is perfectly understandable (it is the 0-ary version of the
{ <fields>; _ } syntax) and thus unlikely to confuse users.

The same code is used to print patterns in debug output, and to show exhaustiveness counter-examples to users. For debug output it is really necessary to differentiate _ from say { contents = _ } (which reads a mutable field) to understand what is going on. (I am not sure that such patterns are ever shown to users, as intuitively all-wildcard record patterns should not occur in exhaustiveness counter-examples.)

@trefis
Copy link
Contributor

trefis commented Sep 13, 2023

FTR: I wondered if the first commit might affect merlin in anyway (as the destruct feature relies on parmatch); after checking, I can confirm that it does not.

Before this commit, a record pattern that accepts all values for all
its fields will be pretty-printed (in exhaustivity counter-examples
and in the debug output of the pattern-matching compiler0 as
a wildcard pattern `_`. This is correct for users, but it gives
inaccurate information in the debug output, as the two patterns are
distinct.

Instead I propose to show "{ _ }" in this case, to mean a record whose
field values do not matter. This happens to not be valid OCaml syntax,
but it is perfectly understandable (it is the 0-ary version of the
{ <fields>; _ } syntax) and thus unlikely to confuse users.
Copy link
Member

@Octachron Octachron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that this PR improves the debug log of the pattern matching compiler.
The updated record counter-example seems fine too.

@gasche
Copy link
Member Author

gasche commented Sep 13, 2023

Thanks!

@gasche gasche merged commit 9023585 into ocaml:trunk Sep 13, 2023
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants