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

Explainer for list-patterns #57210

Closed
wants to merge 5 commits into from

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
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