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

Update list-patterns.md #5364

Merged
merged 2 commits into from
Oct 29, 2021
Merged

Update list-patterns.md #5364

merged 2 commits into from
Oct 29, 2021

Conversation

alrz
Copy link
Member

@alrz alrz commented Oct 29, 2021

Relates to the PR dotnet/roslyn#57457

@alrz alrz requested a review from a team as a code owner October 29, 2021 20:10
The following assumptions are made on the members being used:

- `Length` or `Count` properties are assumed to always return a non-negative value, if and only if the type is *indexable*. For instance, the pattern `{ Length: -1 }` can never match an array.
- The `Slice` method or the range indexer is assumed to be well-behaved, that is, the return value is never null and that it is a proper subslice of the containing list.
Copy link
Member Author

@alrz alrz Oct 29, 2021

Choose a reason for hiding this comment

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

Note that this only affects the codegen and nullability exhaustiveness.
We never produce an error if an unannotated type is matched with null.
Additionally, if the type is annotated (which is not what we'd expect), a slice variable remains annotated.

@alrz
Copy link
Member Author

alrz commented Oct 29, 2021

@jcouv @333fred

proposals/list-patterns.md Outdated Show resolved Hide resolved
Copy link
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 1)

@jcouv
Copy link
Member

jcouv commented Oct 29, 2021

Thanks for the follow-through (remembering to document the assumptions) :-)

@alrz
Copy link
Member Author

alrz commented Oct 29, 2021

(remembering to document the assumptions)

Almost.

@jcouv jcouv self-assigned this Oct 29, 2021
@jcouv jcouv merged commit cb5871f into dotnet:main Oct 29, 2021
@jcouv
Copy link
Member

jcouv commented Oct 29, 2021

Oops. I may have merged too soon.
Did we actually assume that Slice is not-null? That's not what I recall.

@jcouv
Copy link
Member

jcouv commented Oct 29, 2021

Notes say: "If a Slice method is annotated, we will treat it as potentially returning null for the purposes of subsumption and exhaustiveness."
https://github.com/dotnet/csharplang/blob/main/meetings/2021/LDM-2021-10-20.md#open-questions-in-list-patterns

@alrz
Copy link
Member Author

alrz commented Oct 29, 2021

Did we actually assume that Slice is not-null?

No, that's not what's been discussed in LDM.
I just realized this directly affects subsumption checking for nested slices so I figured we need to revisit.

@alrz
Copy link
Member Author

alrz commented Oct 29, 2021

Details to discuss: #5364 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants