Skip to content

Explainer for list-patterns#57210

Closed
jcouv wants to merge 5 commits intodotnet:features/list-patternsfrom
jcouv:pattern-explainer
Closed

Explainer for list-patterns#57210
jcouv wants to merge 5 commits intodotnet:features/list-patternsfrom
jcouv:pattern-explainer

Conversation

@jcouv
Copy link
Member

@jcouv jcouv commented Oct 18, 2021

Still work-in-progress (one test hits assertion, some tests remaining to add)
Relates to test plan #51289

FYI @alrz

return null;
}

var indexes = new Dictionary<int, string>();
Copy link
Member

Choose a reason for hiding this comment

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

Did not know you were working on this. I took a different approach to handle trailing patterns. Updated #55327 just in case. We should def go with your change since this is closer to completion. :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I got a bit excited this weekend and gave it a try. Thanks
I'll probably steal some bits from your draft

{
switch (eval)
{
case BoundDagPropertyEvaluation e when e.IsLengthOrCount:
Copy link
Member

Choose a reason for hiding this comment

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

It's unlikely to have multiple length tests in the shortest path. I suspect this doesn't need to be in the loop.

indexes.Add(e.Index, subPattern);
}
break;
case BoundDagSliceEvaluation e:
Copy link
Member

Choose a reason for hiding this comment

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

This may produce multiple slice patterns in the list but since we won't construct a dag for invalid patterns that probably won't happen.

Copy link
Member Author

Choose a reason for hiding this comment

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

Right, we don't get here when multiple slices. Added test

Comment on lines 179 to +180
tryHandleRecursivePattern(ref unnamedEnumValue) ??
tryHandleListPattern(ref unnamedEnumValue) ??
Copy link
Member

Choose a reason for hiding this comment

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

Doing this after recursive patterns will make it unnecessary to worry about [] or [..] but we may choose to generate those instead.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that something I've been debating. Should we say { Length: 0 } or []?

@jcouv jcouv force-pushed the pattern-explainer branch from 7c98d01 to 89edb85 Compare October 18, 2021 21:06
@jcouv
Copy link
Member Author

jcouv commented Oct 19, 2021

From offline chat, closing in favor of #55327
The SlicePattern_Nullability_Exhaustiveness_NestedSlice test is particularly interesting. For example, should we consider null or { Length: not 4 } => 0, [_, .. [_, _], _] => 0 exhaustive? If not, how do we explain it?

@jcouv jcouv closed this Oct 19, 2021
alrz added a commit to alrz/roslyn that referenced this pull request Oct 19, 2021
Co-authored-by: Julien Couvreur <jcouv@users.noreply.github.com>
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.

2 participants