-
Notifications
You must be signed in to change notification settings - Fork 4k
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
List patterns: change syntax to use square brackets #54335
List patterns: change syntax to use square brackets #54335
Conversation
…t-patterns-03 # Conflicts: # src/Compilers/CSharp/Portable/Binder/Binder_Expressions.cs
case BoundKind.ListPattern: | ||
{ | ||
return ((BoundListPattern)node).Variable as LocalSymbol; | ||
} |
There was a problem hiding this comment.
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
?
There was a problem hiding this comment.
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.
case BoundKind.ListPattern: | ||
{ | ||
return ((BoundListPattern)node).Variable as LocalSymbol; | ||
} |
There was a problem hiding this comment.
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)
@@ -1318,6 +1318,22 @@ protected virtual void AssignImpl(BoundNode node, BoundExpression value, bool is | |||
break; | |||
} | |||
|
|||
case BoundKind.ListPattern: |
There was a problem hiding this comment.
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
@@ -91,6 +91,14 @@ private void NoteDeclaredPatternVariables(BoundPattern pattern) | |||
} | |||
} | |||
break; | |||
case BoundListPattern list: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 Region test added.
3e5c0ca
to
1fd452c
Compare
1fd452c
to
3c2e686
Compare
src/Compilers/CSharp/Portable/Parser/LanguageParser_Patterns.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Syntax/Parsing/PatternParsingTests_ListPatterns.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Syntax/Parsing/PatternParsingTests_ListPatterns.cs
Outdated
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Syntax/Parsing/PatternParsingTests_ListPatterns.cs
Show resolved
Hide resolved
src/Compilers/CSharp/Test/Syntax/Parsing/PatternParsingTests_ListPatterns.cs
Show resolved
Hide resolved
Done review pass (commit 38) |
There was a problem hiding this 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?
That's what we've been doing since forever. |
Spec: https://github.com/dotnet/csharplang/blob/main/proposals/list-patterns.md
Test plan: #51289