Skip to content

List patterns: change syntax to use square brackets#54335

Merged
jcouv merged 39 commits intodotnet:features/list-patternsfrom
alrz:list-patterns-syntax
Jul 13, 2021
Merged

List patterns: change syntax to use square brackets#54335
jcouv merged 39 commits intodotnet:features/list-patternsfrom
alrz:list-patterns-syntax

Conversation

@alrz
Copy link
Copy Markdown
Member

@alrz alrz commented Jun 23, 2021

@ghost ghost added the Area-Compilers label Jun 23, 2021
@alrz alrz marked this pull request as ready for review June 24, 2021 03:28
@alrz alrz requested review from a team as code owners June 24, 2021 03:28
@alrz alrz requested review from a team and 333fred June 30, 2021 19:21
case BoundKind.ListPattern:
{
return ((BoundListPattern)node).Variable as LocalSymbol;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Are the changes in iteration 34-37 (DataFlowOutWalker, DefiniteAssignemtn and VariablesDeclaredWalker) all part of fixing test ListPattern_Negated_01?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I looked around for RecursivePattern to see what's missed. see comments below.

Comment on lines +130 to +133
case BoundKind.ListPattern:
{
return ((BoundListPattern)node).Variable as LocalSymbol;
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📝 This is debug-only to assert data flow result. (currently disabled)

break;
}

case BoundKind.ListPattern:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📝 This is covered in ListPattern_Negated_01

}
}
break;
case BoundListPattern list:
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

📝 Region test added.

@alrz alrz force-pushed the list-patterns-syntax branch from 3e5c0ca to 1fd452c Compare July 2, 2021 06:53
@alrz alrz force-pushed the list-patterns-syntax branch from 1fd452c to 3c2e686 Compare July 2, 2021 06:59
@333fred
Copy link
Copy Markdown
Member

333fred commented Jul 8, 2021

Done review pass (commit 38)

@alrz alrz requested a review from 333fred July 13, 2021 16:12
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.

LGTM (commit 39). @alrz since there are so many commits I'd prefer to squash this change, is that OK with you?

@alrz
Copy link
Copy Markdown
Member Author

alrz commented Jul 13, 2021

I'd prefer to squash this change

That's what we've been doing since forever.

@jcouv jcouv merged commit 9961685 into dotnet:features/list-patterns Jul 13, 2021
@jcouv
Copy link
Copy Markdown
Member

jcouv commented Jul 13, 2021

Merged/squashed. Yay! This unblocks a number of PRs :-)
Thanks @alrz @333fred

@alrz alrz changed the title List patterns: syntax changes List patterns: change syntax to use square brackets Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants