Skip to content

fix: pattern match on chars#917

Open
clementblaudeau wants to merge 4 commits intoAeneasVerif:mainfrom
clementblaudeau:fix-pattern-match-chars
Open

fix: pattern match on chars#917
clementblaudeau wants to merge 4 commits intoAeneasVerif:mainfrom
clementblaudeau:fix-pattern-match-chars

Conversation

@clementblaudeau
Copy link
Copy Markdown

@clementblaudeau clementblaudeau commented Apr 7, 2026

Context

This PR comes as a fix for #797: pattern-matching on chars was not supported. I've tried to keep the commits as minimal and logically separated as possible, but as it's my first contribution to this project, please tell me if the style is not right.

Example

 match c {
     'a' => 0,
     _ => 1,
 }

used to throw an error, now produces

match c with
| 'a' => ok 0#usize
| _ => ok 1#usize

Fix

While pattern-match on concrete chars is easy, pattern-match on symbolic ones required a bit more work. The expand_symbolic_int function was renamed into expand_symbolic_literal and now supports both ints (signed/unsigned) and chars. Downstream, a new ExpandChar case was added to expansion. I've used Claude for some of the design/fix but reviewed it very carefully.

Design choices

  • Instead of modifying expand_symbolic_int, we could also duplicate it and create an expand_symbolic_char.
  • Instead of creating a new ExpandChar, we could merge it with ExpandInt

fixes #797

Add test for concrete and some symbolic matches
[expand_symbolic_int] is renamed into [expand_symbolic_literal], and made to
support both integers (signed/unsigned) and chars. The sanity check ensures that
the target list of literals has the right type wrt the symbolic value.
@clementblaudeau
Copy link
Copy Markdown
Author

I just saw #896, which I believe this PR subsumes.

Copy link
Copy Markdown
Member

@sonmarcho sonmarcho left a comment

Choose a reason for hiding this comment

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

Sorry for taking so long to review this: I have been very busy lately... I have a minor remark but otherwise it looks good to me, thanks for this PR!

(literal_as_integer int_ty)
(List.map literal_as_scalar values)
ctx
int_ty values ctx
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you rename int_ty to lit_ty?

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.

bug: Matching on chars throws an Uncaught exception

2 participants