Skip to content

Add list patterns proposal#3245

Merged
jcouv merged 16 commits intodotnet:mainfrom
alrz:list-patterns
Mar 19, 2021
Merged

Add list patterns proposal#3245
jcouv merged 16 commits intodotnet:mainfrom
alrz:list-patterns

Conversation

@alrz
Copy link
Copy Markdown
Member

@alrz alrz commented Mar 2, 2020 •
edited
Loading

Tracking issue: #3435
Discussion: #1510, #1039, #898
Prototype: dotnet/roslyn@main...alrz:list-patterns
Proposal: Rendered

@gafter
Copy link
Copy Markdown
Member

gafter commented Mar 3, 2020 •
edited
Loading

Really nice work!

List initializers (e.g. in an object creation expression) look like { 1, 2, 3 }, so by A design principle for Programming With Data in #3107 the pattern form "should" use curly braces instead of square brackets. Do you see issues with that? I don't think there is a clash because a property pattern contains PropertyName: before each pattern.

yaakov-h
yaakov-h reviewed Mar 3, 2020
View reviewed changes
yaakov-h
yaakov-h reviewed Mar 3, 2020
View reviewed changes
@alrz
Copy link
Copy Markdown
Member Author

alrz commented Mar 3, 2020

the pattern form "should" use curly braces instead of square brackets. Do you see issues with that?

Unfortunately, with braces, empty list pattern would be ambigious with property pattern clause.

alrz
alrz commented Mar 3, 2020
View reviewed changes
@gafter gafter mentioned this pull request May 7, 2020
Open
ILMTitan
ILMTitan reviewed Sep 30, 2020
View reviewed changes
@YairHalberstadt
Copy link
Copy Markdown
Contributor

YairHalberstadt commented Oct 2, 2020

IEnumerable support can def be added

I would suggest allowing pattern based enumeration as well, same as foreach.

@alrz

This comment has been minimized.

Sign in to view
@alrz

This comment has been minimized.

Sign in to view
jnm2
jnm2 reviewed Oct 10, 2020
View reviewed changes
@YairHalberstadt YairHalberstadt mentioned this pull request Oct 28, 2020
Closed
4 tasks
@alrz alrz mentioned this pull request Oct 30, 2020
Closed
@333fred 333fred self-assigned this Nov 2, 2020
@YairHalberstadt YairHalberstadt mentioned this pull request Nov 19, 2020
Closed
@alrz

This comment has been minimized.

Sign in to view
@alrz alrz closed this Feb 11, 2021
@alrz alrz reopened this Mar 2, 2021
@alrz
Copy link
Copy Markdown
Member Author

alrz commented Mar 2, 2021

Updated based on the latest LDM decisions. @333fred @jcouv please review.

alrz
alrz commented Mar 2, 2021
View reviewed changes
@alrz alrz mentioned this pull request Mar 2, 2021
Merged
333fred
333fred reviewed Mar 8, 2021
View reviewed changes
@alrz alrz requested a review from 333fred March 9, 2021 05:56
jcouv
jcouv reviewed Mar 9, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 9, 2021
View reviewed changes
alrz
alrz commented Mar 9, 2021
View reviewed changes
Base automatically changed from master to main March 12, 2021 19:15
jcouv
jcouv reviewed Mar 15, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 15, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 15, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 15, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 15, 2021
View reviewed changes
Copy link
Copy Markdown
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 11)

@alrz alrz requested a review from jcouv March 17, 2021 04:52
alrz
alrz commented Mar 17, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 17, 2021
View reviewed changes
jcouv
jcouv reviewed Mar 17, 2021
View reviewed changes
jcouv
jcouv approved these changes Mar 17, 2021
View reviewed changes
Copy link
Copy Markdown
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.

LGTM Thanks (iteration 12) with minor follow-up comments.

@jcouv
Copy link
Copy Markdown
Member

jcouv commented Mar 17, 2021

Tagging @333fred for a second look. Thanks

333fred
333fred reviewed Mar 18, 2021
View reviewed changes
333fred
333fred reviewed Mar 18, 2021
View reviewed changes
333fred
333fred reviewed Mar 18, 2021
View reviewed changes
333fred
333fred reviewed Mar 18, 2021
View reviewed changes
Copy link
Copy Markdown
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.

A couple of clarity comments.

@alrz alrz requested a review from a team as a code owner March 19, 2021 10:03
@alrz alrz requested a review from 333fred March 19, 2021 10:06
@jcouv jcouv merged commit b864efc into dotnet:main Mar 19, 2021
@jcouv
Copy link
Copy Markdown
Member

jcouv commented Mar 19, 2021

Cool. Thanks for writing this up and sorting many questions. We'll start looking at your roslyn-side change now :-)

@333fred 333fred mentioned this pull request Dec 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jnm2 jnm2 jnm2 left review comments

@chsienki chsienki chsienki left review comments

@jcouv jcouv jcouv approved these changes

@333fred 333fred Awaiting requested review from 333fred

+3 more reviewers

@yaakov-h yaakov-h yaakov-h left review comments

@CyrusNajmabadi CyrusNajmabadi CyrusNajmabadi left review comments

@ILMTitan ILMTitan ILMTitan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@333fred 333fred

@jcouv jcouv

Labels

None yet

Projects

Compiler: Julien's umbrellas
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

10 participants

@alrz @gafter @YairHalberstadt @jcouv @yaakov-h @333fred @CyrusNajmabadi @jnm2 @chsienki @ILMTitan