Skip to content

pattern-matching refactoring: refine the type of complete_constrs#9599

Merged
gasche merged 2 commits intoocaml:trunkfrom
trefis:rematch-complete-constrs
May 27, 2020
Merged

pattern-matching refactoring: refine the type of complete_constrs#9599
gasche merged 2 commits intoocaml:trunkfrom
trefis:rematch-complete-constrs

Conversation

@gasche
Copy link
Copy Markdown
Member

@gasche gasche commented May 25, 2020

This is an offspring of #9563 (the n-th piece of the big pattern-matching refactoring #9321) that was suggested by @Octachron during review.

We change the amount of information carried about variant constructors during pattern-matching compilation, to be able to refine the type of the function complete_constrs that sits at the Matching/Parmatch boundary, which would previously accept any pattern statically, and do a dynamic check that fails unless it is in fact a variant constructor pattern.

This loses no information (descriptions contain the tag), but it will
make it easier to obtain the descriptions inside `combine_constructor`
without doing a dynamic check on the patterns. This will in turn help
simplify the interaction with `Parmatch.complete_constrs`.

Note: after this patch we use `Types.equal_tag` instead of `( = )` to
compare tags during pattern-matching compilation. This is better code,
and we believe that it does not change the behavior: `Types.equal_tag`
is mostly similar to a type-specialized version of `( = )`, except
that it calls `Ident.same` that just compares the stamps and ignore
the names, which (assuming well-formedness of idents) is equivalent
and slightly faster.
Copy link
Copy Markdown
Contributor

@trefis trefis left a comment

Choose a reason for hiding this comment

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

Neat!

This simplifies this particular interface boundary between Matching
and Parmatch.

(Suggested by Florian Angeletti)
@gasche gasche force-pushed the rematch-complete-constrs branch from b3b5ab6 to bf95a24 Compare May 26, 2020 13:47
@gasche
Copy link
Copy Markdown
Member Author

gasche commented May 26, 2020

Thanks for the review! I ran make depend and fixed the typos, and pushed again. I will merge if/when the CI is green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants