-
Notifications
You must be signed in to change notification settings - Fork 355
Match (updated) #2828
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
Match (updated) #2828
Conversation
…rning for unreachable arms
…f checks in the compiler. Add support for pattern matching, including typed bindings, literals, and unions. Enhance the handling of pattern variables and guards within match arms.
…pattern compilation
…ing typed bindings, literals, and unions. Add methods for binding pattern variables and extracting type names for instanceof checks.
…ype error handling
…chableArm in compiler error formatting
…luding match arms, typed bindings, literals, and unions. Implement methods for lowering match expressions and patterns from CST to HIR, enhancing the compiler's pattern matching capabilities.
…d UnreachableArm in error file ID retrieval
…ms and guards. Implement methods for parsing match patterns and elements, enhancing the parser's capabilities for the new Match token.
…associated methods for handling match expressions and patterns. Enhance the syntax tree with functionality for scrutinee extraction, arm iteration, and pattern analysis, including union and typed bindings.
…, arms, patterns, and guards in the syntax kind enum.
…nd pattern coverage analysis. enhanced the `infer_expr` function to handle match arms and more
…sions. furthered exhaustiveness checking and unreachable arm detection in match statements.
…pattern handling with typed bindings, literals, and unions. Update error formatting for non-exhaustive matches and unreachable arms.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
✅ Latest build complete • View logs |
|
🐑 BEPs Preview Ready Preview URL: https://dj7ggjkp4tlhz.cloudfront.net/paulo-match/ Commit: |
CodSpeed Performance ReportMerging #2828 will not alter performanceComparing Summary
Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add some of these tests to baml_codegen/tests/ as well?
| /// Lower a match expression from CST to HIR. | ||
| /// | ||
| /// MATCH_EXPR structure (from parser): | ||
| /// - Scrutinee expression (could be a PAREN_EXPR wrapping the actual expr, or a literal token) | ||
| /// - One or more MATCH_ARM nodes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I would add AST accessors to make lowering higher level (pun intended), not direct syntax nodes access. See this comment in PR #2838
| /// Lower a single match arm from CST to HIR. | ||
| /// | ||
| /// MATCH_ARM structure (from parser): | ||
| /// - MATCH_PATTERN node | ||
| /// - Optional MATCH_GUARD node (contains 'if' keyword + expression) | ||
| /// - FAT_ARROW token ('=>') | ||
| /// - Body expression (BLOCK_EXPR or other expression, or literal token) | ||
| fn lower_match_arm(&mut self, node: &baml_syntax::SyntaxNode) -> MatchArm { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, I'd add AST accessors.
| /// Lower a match pattern from CST to HIR. | ||
| /// | ||
| /// MATCH_PATTERN structure (from parser): | ||
| /// - Pattern elements (identifiers, literals, type expressions) | ||
| /// - Optional PIPE tokens for union patterns | ||
| /// | ||
| /// Pattern forms: | ||
| /// - Binding: `x`, `_` | ||
| /// - Typed binding: `s: Success` | ||
| /// - Literal: `null`, `true`, `42`, `"hello"` | ||
| /// - Enum variant: `Status.Active` | ||
| /// - Union: `200 | 201` or `Status.Active | Status.Pending` | ||
| fn lower_match_pattern(&mut self, node: &baml_syntax::SyntaxNode) -> PatId { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CST -> AST -> HIR
|
I think this is no longer necessary, we merged |
Important
Supersedes #2825
I renamed the
paulo/mapbranch topaulo/matchand had to recreate this PR. The original is referenced above to preserve the original comments.This is the first pass on
matchs implementation. I've tried to make sure there are plenty of comments referencing the BEP shared yesterday on critical moments; expect a cleanup later on.There are somethings to keep in mind:
Completed Work
Core Infrastructure
Type Checking
Phase 3: Code Generation
Tests (PARTIAL)
What's Next
Type System Improvements
1. Type Alias Expansion
Current: Type aliases like type
Result = Success | Failureare treated as opaqueTy::Named("Result").Required: Expand type aliases to their definitions for proper union exhaustiveness checking.
Without this, exhaustiveness checking doesn't work for type alias unions.
2. Enum Variant Exhaustiveness
Current: Enum variant patterns (
Status.Active) are treated as literals, not contributing to exhaustiveness.Required: Track which enum variants are covered and error if variants are missing.
3. Literal Type Exhaustiveness
Current: Literal patterns don't contribute to type-level exhaustiveness.
Required: For literal union types (
200 | 201 | 204), track which values are covered.Refinements
1. Overlapping Pattern Verification
The BEP says this should work (second arm only matches B, not unreachable):
Status: Likely works, but needs explicit test coverage.
2. Better Error Spans
Current: Error spans point to the match expression, not the specific arm.
Required: Point to the specific arm that's unreachable or the missing cases.