-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Fix #13455: Also consider sealed classes closed sums #13483
Conversation
The warnings on master are incorrect: we know that no instance of SealedClass match the corresponding cases.
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.
After commenting in #13455, I'm more certain that you can't change this and not change the children side too, though I don't have the concrete counter-example to prove it.
This commit renames isClosedSum to isDecomposable, and moves that function and the decompose function to where they are used locally. The logic in question only makes sense at the precise point where it's used (see the added comment), so having those two functions defined locally helps to avoid confusion.
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.
A small typo (iiuc) but looks good to me now, thanks for the attention, @OlivierBlanvillain!
Co-authored-by: Dale Wijnand <[email protected]>
Even better. 💯 |
No description provided.