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

Tree-sitter grammar: support match expressions #5364

Merged
merged 9 commits into from
May 6, 2024

Conversation

OceanOak
Copy link
Collaborator

@OceanOak OceanOak commented May 2, 2024

Changelog:

Tree-sitter-darklang
- Update the grammar to support match expressions

#5321

@OceanOak OceanOak force-pushed the grammar-updates-11 branch 2 times, most recently from 623d80c to 02bf119 Compare May 3, 2024 09:41
@OceanOak OceanOak changed the title wip: Tree-sitter grammar: support match expressions Tree-sitter grammar: support match expressions May 3, 2024
@OceanOak
Copy link
Collaborator Author

OceanOak commented May 3, 2024

Questions:

  • What should we do about wildcard eg. Error _ -> ... or | _ -> ...
    we can add support for it in the grammar, but I am not sure how it will be captured in writtenTypes. Could you please provide some insights on this?
  • why don't we have MPDict?

@OceanOak OceanOak marked this pull request as ready for review May 3, 2024 14:23
@OceanOak OceanOak requested a review from StachuDotNet May 3, 2024 14:47
@StachuDotNet
Copy link
Member

StachuDotNet commented May 6, 2024

  • What should we do about wildcard eg. Error _ -> ... or | _ -> ...
    we can add support for it in the grammar, but I am not sure how it will be captured in writtenTypes. Could you please provide some insights on this?

It's OK to capture these as Variable match patterns.
Maybe someday we'll have a separate Wildcard MatchPattern, though -- I've thought that for a while - I'm not sure, then, what _this would be captured as (wildcard or variable?).

  • why don't we have MPDict?

I'm not sure what that would look like. I don't believe similar languages have one we could use for inspiration (besides's F#'s record pattern but I don't think it perfectly applies).
I suppose the answer here is that our other match patterns are used to deconstruct structures and extract values out of them, safely, but the fields of a dict are unstable, so unsafe to deconstruct them
Also, our other patterns can be checked at parse-time, whereas a dict one (if we could imagine how it looks) probably couldn't.

@@ -531,7 +613,64 @@ module.exports = grammar({
),

//
// Match expression
Copy link
Member

Choose a reason for hiding this comment

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

let's add a TODO to allow for one-line match exprs (match myInt with | 1 -> ...)

Copy link
Member

@StachuDotNet StachuDotNet left a comment

Choose a reason for hiding this comment

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

Looking great! After adding that TODO and a test for | _ -> ..., I think this is good to go.

@@ -184,6 +184,7 @@ module.exports = grammar({
alias($.mp_tuple, $.tuple),
alias($.mp_enum, $.enum),
alias($.variable_identifier, $.variable),
alias(/_/, $.wildcard),
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

should we allow variable_identifierto start with _ instead of handling it this way?

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, yeah I think so.

I've been thinking, and there's some value to (future) having a specific wildcard pattern: anything with _:

  • may not be accessed
  • should not be complained about, by the type-checker, for lack of usage (where we might otherwise warn "hey you captured this variable but didn't use it!")

@OceanOak OceanOak requested a review from StachuDotNet May 6, 2024 16:26
@OceanOak OceanOak force-pushed the grammar-updates-11 branch from b9ae44a to e7c7b17 Compare May 6, 2024 19:48
@StachuDotNet StachuDotNet merged commit 29807a1 into darklang:main May 6, 2024
9 checks passed
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.

None yet

2 participants