Skip to content

Fix for subtype pattern matching #13178

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.

Sign up for GitHub

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

Jump to bottom
Merged
dsyme merged 2 commits into from
May 23, 2022
Merged

Fix for subtype pattern matching #13178

dsyme merged 2 commits into from
May 23, 2022

Conversation

@dsyme
Copy link
Contributor

@dsyme dsyme commented May 23, 2022 •
edited
Loading

This fixes #13175

Added a systematic test matrix

@dsyme dsyme changed the title Tentative fix for pattern matching [WIP] fix for subtype pattern matching May 23, 2022
@dsyme dsyme changed the title [WIP] fix for subtype pattern matching Fix for subtype pattern matching May 23, 2022
@dsyme
Copy link
Contributor Author

dsyme commented May 23, 2022 •
edited
Loading

@vzarytovskii Could you take a careful look at this (and the subsequent 6 functions)

fsharp/src/Compiler/Checking/PatternMatchCompilation.fs

Line 436 in c464767

let computeWhatSuccessfulTypeTestImpliesAboutNullTest g tgtTy1 =
and see if there are obvious test cases/matrix that we should add? I think I need more eyes on the test matrix for this.

@dsyme
Copy link
Contributor Author

dsyme commented May 23, 2022

and see if there are obvious test cases/matrix that we should add? I think I need more eyes on the test matrix for this.

We can merge this and add the tests separately

@vzarytovskii
Copy link
Member

vzarytovskii commented May 23, 2022

and see if there are obvious test cases/matrix that we should add? I think I need more eyes on the test matrix for this.

We can merge this and add the tests separately

Agree, we should merge this one, while I'm looking at some more potential tests.
What shall we do with servicing? @KevinRansom @brettfo

@dsyme
Copy link
Contributor Author

dsyme commented May 23, 2022

What shall we do with servicing? @KevinRansom @brettfo

This unfortunately needs to be a priority for all servicable branches that have the problem...

@dsyme dsyme merged commit c95d6aa into dotnet:main May 23, 2022
vzarytovskii pushed a commit to vzarytovskii/fsharp that referenced this pull request May 24, 2022
@dsyme @vzarytovskii
Fix for subtype pattern matching (dotnet#13178)
c6e864e 
* tentative fix for pattern matching logic

* add test matrix
vzarytovskii added a commit that referenced this pull request May 24, 2022
@vzarytovskii @dsyme
Fix for subtype pattern matching (#13178) (#13190)
211fd76 
* tentative fix for pattern matching logic

* add test matrix

Co-authored-by: Don Syme <dsyme@users.noreply.github.com>
@KathleenDollard KathleenDollard mentioned this pull request May 24, 2022
Closed
@vzarytovskii vzarytovskii mentioned this pull request May 24, 2022
Closed
@vzarytovskii
Copy link
Member

vzarytovskii commented May 24, 2022

dev17.2 PR is here - #13190

@vzarytovskii vzarytovskii mentioned this pull request Aug 1, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vzarytovskii vzarytovskii vzarytovskii approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Regression in dotnet SDK 6.0.300: optimizing match with type checks

2 participants

@dsyme @vzarytovskii