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

Pattern matching syntax and semantics #2188

Merged
merged 28 commits into from
Jan 7, 2023

Conversation

zygoloid
Copy link
Contributor

@zygoloid zygoloid commented Sep 15, 2022

This paper proposes concrete syntax and semantic choices for Carbon patterns.

@zygoloid zygoloid added proposal A proposal proposal draft Proposal in draft, not ready for review labels Sep 15, 2022
@zygoloid zygoloid marked this pull request as ready for review September 15, 2022 21:39
@github-actions github-actions bot added proposal rfc Proposal with request-for-comment sent out and removed proposal draft Proposal in draft, not ready for review labels Sep 15, 2022
@zygoloid zygoloid added proposal rfc Proposal with request-for-comment sent out and removed proposal rfc Proposal with request-for-comment sent out labels Sep 15, 2022
Copy link
Contributor

@micttyl micttyl left a comment

Choose a reason for hiding this comment

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

I need to read more to understand. This is because I haven't caught up the current state of Carbon.

proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
zygoloid and others added 4 commits September 16, 2022 13:48
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Copy link
Contributor

@micttyl micttyl left a comment

Choose a reason for hiding this comment

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

It is clear.

proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
Copy link
Contributor

@micttyl micttyl left a comment

Choose a reason for hiding this comment

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

Thank you. Understood. Confusing though.

}
```

The `unused` keyword indicates that the binding is intended to not be used.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you show the use case of unused

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is now an example of this a little later in the proposal, plus a cross-reference to #2022 which introduces this keyword.

Comment on lines +662 to +663
separate topic from pattern matching syntax, even though the semantic behavior
of the two may be quite similar or identical.
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a proposal for pattern matching syntax and semantics, though. Even at the syntactic level, they're closely connected: a deduced parameter list always comes immediately before a pattern, and always consists of a list of binding patterns. At the semantic level, it's not just that the behavior of the two is "similar", it's that they are directly coupled: as far as I can tell, type deduction is always driven by a pattern matching operation, and I think it can feed back into that pattern matching operation.

To be clear, I'm not saying this proposal needs to have a design for type deduction; I just don't think type deduction is as cleanly separable from pattern matching as this makes it sound.

Copy link
Contributor Author

@zygoloid zygoloid Sep 21, 2022

Choose a reason for hiding this comment

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

I don't think they're necessarily coupled. I think we are confident that we're going to do deduction when performing function calls to generic functions and when selecting an impl. I think we are confident that we're going to do pattern matching in a match statement and in a let or var binding. I don't think it's guaranteed that we'll do pattern matching in a function call -- it may be reasonable to restrict a function to have only a list of bindings as its parameters, which only needs pattern matching in a degenerate sense, and I think we're not going to do pattern matching in impl selection. So I think there might actually be no situations where we perform both type deduction and non-degenerate pattern matching.

I also think it's possible that we'll model the matching process for the two cases quite differently: for pattern matching, we have a top-down process where the pattern syntax determines what happens at each level (such as evaluating an expression pattern and performing an == comparison, or converting the scrutinee), whereas for type deduction I think it's likely we'll instead perform a symbolic evaluation of the type expression (the "pattern") and perform a value comparison between (symbolic) type values to find a set of substitutions. The difference is fairly subtle, but I think deduction ends up having stricter matching semantics than a pattern match -- identical after substitution versus a looser notion of "match".

Copy link
Contributor

Choose a reason for hiding this comment

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

So I think there might actually be no situations where we perform both type deduction and non-degenerate pattern matching.

It seems like variadic functions will be such a situation. Maybe we can say that semantically, variadics relate only to deduction, and not pattern matching (although I'm not sure of that), but syntactically they're going to appear in the parentheses as well as the square brackets.

proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
@camio
Copy link
Contributor

camio commented Nov 22, 2022

I looked at all the changes since my prior review and they look good to me.

I'm hesitant to copy and translate someone else's work without permission from all the authors. Maybe I can use those as inspiration at least.

Most of those examples come from my paper https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0095r1.html. Feel free to use them. The red-black tree example from the newer paper was also written by me, so feel free to use that one as well.

@zygoloid
Copy link
Contributor Author

zygoloid commented Dec 1, 2022

Most of those examples come from my paper https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0095r1.html. Feel free to use them. The red-black tree example from the newer paper was also written by me, so feel free to use that one as well.

Thanks, added!

proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
proposals/p2188.md Outdated Show resolved Hide resolved
zygoloid and others added 2 commits December 2, 2022 17:20
Co-authored-by: josh11b <josh11b@users.noreply.github.com>
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Really awesome to see all of this coming together.

I do still have a few areas I'd like to push on, but to be clear I think all of these are relatively small and narrow aspects of the proposal. Even the syntactic questions, while they'll show up everywhere, are ultimately a small part of the proposal.

extensible, which if used would mean that a set of patterns for that choice type
would only be considered exhaustive if it includes a wildcard pattern.

## Future work
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe also mention an expression-form as future work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

}
```

### Introducer syntax for expression patterns
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like this section doesn't fully capture what we're giving up without this. Is it possible to capture some of the context from the discussion leading up to this Discord message? https://discord.com/channels/655572317891461132/709488742942900284/1020559500597538886

Generally, I wonder if we want to more deeply revisit whether we can and should shift the syntax in this direction. For example, I think I see plausible solutions to the purely syntactic concerns with identifying an introducer below.

That said, I'd be happy to either move that discussion to an issue for leads, and even do that as a fix-forward with even a minor note here that we anticipate revisiting this specific aspect of the syntax to see if there are approaches that might result in an overall better model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added some more notes and links. I favor the fix-forward approach, given that the most promising path involves changing the syntax for where expressions to remove the other, conflicting meaning of is.

Comment on lines +746 to +750
### Struct pattern syntax

We could omit the `, _` syntax. This would simplify struct patterns, but at the
cost of removing a feature that can be useful for reducing verbosity and making
library evolution easier.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not fully convinced that we should add this yet TBH.

I wonder if we'll be able to address this usecase with variadics, and with tools that also address discarding an unspecified number of tuple elements.

I lean a little bit towards maybe documenting this option but waiting to see if we instead can solve this as part of variadic patterns more generally. What do other folks think here?

The flip side is that I think this is a minor feature and easily removed if we find that variadics obviate it, so I also don't think this is a big deal either way.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that struct fields aren't expressions or values, so the ability to create and match packs of struct fields would be a separate, bespoke feature, not something that would emerge automatically from a more general variadics design. I don't think we've discussed having such a feature, and it's not clear to me if there are any other use cases for it (maybe metaprogramming?). So my inclination would be to keep , _ for now, and then replace it if and when we discover we want packs-of-struct-fields for other reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree that it's not a big deal either way, because this should be easy to change later. I lean towards adding it and seeing if we can replace it with something else later, such as variadics, and similarly seeing if it gets used heavily.

proposals/p2188.md Show resolved Hide resolved
with permission from the author of those examples, David Sankel. Thank you,
David!

### Examples from P0095R1
Copy link
Contributor

Choose a reason for hiding this comment

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

Does it make sense to provide a comparison to the syntax proposed in https://wg21.link/p2392 and more generally acknowledge some of the differences in design there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If that comparison is useful when evaluating this proposal, I can add it, but I'm a little hesitant to get too deep into comparing this with in-flight WG21 proposals. I'm a little tempted to remove the P0095R1 / P1371R3 comparisons, given that they're not comparing against an existing language.

@chandlerc chandlerc self-requested a review January 4, 2023 21:57
Copy link
Contributor

@chandlerc chandlerc left a comment

Choose a reason for hiding this comment

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

Just officially approving for leads. The future work and other todos cover everything that I think is outstanding, and this seems like a really clear step forward.

@zygoloid zygoloid merged commit d70e237 into carbon-language:trunk Jan 7, 2023
zygoloid added a commit that referenced this pull request Jun 2, 2023
Integrates the content of #2188 into `pattern_matching.md`

Closes #2852 

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
github-merge-queue bot pushed a commit that referenced this pull request Aug 2, 2023
This reflects changes from a number of approved proposals:
- #920 : concrete statements about orphan and overlap in Carbon
- #2138 : "generic" -> "checked generic", "template" -> "template
generic"
- #2188 : binding patterns are forbidden in type position
- #2360 : "type", "facet type", "facet". Note: I am not using the term
"generic type" from #2360 since that meaning conflicts with the
generally accepted meaning of "generic type" of a type with a
compile-time parameter.
- #2760 / #2770 : internal/external impl -> extending impl
- #2964 : "symbolic constant" and "template constant"

---------

Co-authored-by: Geoff Romer <gromer@google.com>
Co-authored-by: Richard Smith <richard@metafoo.co.uk>
github-merge-queue bot pushed a commit that referenced this pull request Sep 8, 2023
Incorporates proposals: #1885, #2138, #2188, #2200, #2360, #2760, #2964,
and #3162.

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>
github-merge-queue bot pushed a commit that referenced this pull request Sep 23, 2023
Includes proposals:
- #990
- #2188
- #2138
- #2200
- #2360
- #2760
- #2964
- #3162

Also tries to use more precise language when talking about:
- implementations, to avoid confusing `impl` declaration and definitions
with the `impls` operator used in `where` clauses, an issue brought up
in #2495 and #2483;
- "binding patterns", like `x: i32`, and "bindings" like `x`.

---------

Co-authored-by: Chandler Carruth <chandlerc@gmail.com>
@zygoloid zygoloid deleted the proposal-pattern-match branch October 4, 2023 23:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal rfc Proposal with request-for-comment sent out proposal A proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants