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

Add pattern-variables.md #4592

Merged
merged 18 commits into from
Oct 4, 2021
Merged

Add pattern-variables.md #4592

merged 18 commits into from
Oct 4, 2021

Conversation

alrz
Copy link
Contributor

@alrz alrz commented Mar 28, 2021

Championed issue: #4018
Prototype: dotnet/roslyn@main...alrz:pattern-variables
Proposal: Rendered

@alrz alrz requested a review from a team as a code owner March 28, 2021 01:17
@alrz
Copy link
Contributor Author

alrz commented Mar 28, 2021

cc @333fred @RikkiGibson

Opening up this early due to possible interaction with improved def assignment proposal.


The following rules applies to any *primary_pattern* that declares a variable:

- The state of *v* is definitely assigned after *primary_pattern* if the pattern is irrefutable; permitting use after:
Copy link
Contributor Author

@alrz alrz Mar 29, 2021

Choose a reason for hiding this comment

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

This rule should probably be moved to the previous section under is_pattern_expression. Because only at the top-level we can determine if a pattern always succeeds (at least that's how it works today). Of course, except for a var-pattern with a single variable designation which always succeeds regardless of the input type.

Copy link
Member

Choose a reason for hiding this comment

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

Consider linking to those original/unmodified sections instead of replicating them here.

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's no one place for this in previous proposals. We have three documents each of which mentioned definite assignment in prose.

In any case, since we don't want to add expressions to the mix just yet, I think we no longer need to specify definite assignment in this format.

proposals/pattern-variables.md Outdated Show resolved Hide resolved
@RikkiGibson
Copy link
Contributor

I made a fairly quick pass over the proposal and didn't notice anywhere that it would conflict with "improved definite assignment". However, it will probably be a little while before I can review this proposal in detail, as our feature load for C# 10 is getting a little bit high.

proposals/pattern-variables.md Outdated Show resolved Hide resolved
@jcouv jcouv requested a review from 333fred August 18, 2021 18:43
@jcouv
Copy link
Member

jcouv commented Aug 18, 2021

Tagging @333fred since he's championing the proposal.

Note: This is the expression variation of the previous case.

For a *negated_pattern* of the form `not pattern_operand`:
- Pattern variables declared in the *pattern_operand* **must be redeclared** inside any other *negated_pattern* in the containing pattern; permitting:
Copy link
Member

@jcouv jcouv Aug 18, 2021

Choose a reason for hiding this comment

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

"... inside any other negated_pattern..."

What if there's more than one nested negated_pattern? e is not (0, var x) and not not (0, 0) #Pending

Copy link
Contributor Author

@alrz alrz Oct 1, 2021

Choose a reason for hiding this comment

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

Haven't thought this through yet. To keep things simple, we could add this at some later point if there's enough support to allow such patterns. (added to open issues)

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 13). Only minor comments

proposals/pattern-variables.md Outdated Show resolved Hide resolved
proposals/pattern-variables.md Outdated Show resolved Hide resolved
proposals/pattern-variables.md Outdated Show resolved Hide resolved
proposals/pattern-variables.md Outdated Show resolved Hide resolved
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

@jcouv please take a look.


Allow variable declarations under `or` patterns and across `case` labels in a `switch` section.

## Motivation
Copy link
Member

Choose a reason for hiding this comment

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

I'd add some more practical (less abstract) motivating examples.
Looking that the roslyn codebase, we often have a switch expression just to get some value out of different kinds of bound nodes. Many can be found just by searching for "node switch".
But I have to say looking at that search result, most of them don't look like they would benefit from this feature (either they use when clauses for different cases, or they call different property/method overloads in the different cases).

@jcouv jcouv merged commit 6e57bef into dotnet:main Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

4 participants