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

Prepare for SIP-56 match types. #109

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

sjrd
Copy link

@sjrd sjrd commented Dec 8, 2023

Of the entire ecosystem of Scala 2 libraries, s2mc is the most affected by the new match types of SIP-56. Some of it was decidedly unsound and can be fixed by local rewriting, which we do in the first 2 commits.

Some tests are not fundamentally shown to be unsound but did rely on some implementation-defined behavior of the old match types, which is not supported anymore. It does not seem to me like the larger project needs those particular tests to pass to be meaningful. So we suggest to remove them, and focus on types that are actually provably disjoint.

`CodecBinding` per se is not a valid type constructor for match
type cases under SIP-56.

We manually expand the `Tuple.InverseMap` so that we get a more
precise decomposition, which allows to write a match type case
that is valid under the new rules, without changing the behavior.

This change allows to compile `protocol_core` but not yet its tests.
An abstract type like `Lock[X]` cannot be proven disjoint from
anything under SIP-56, since it *could* be instantiated to anything.
In particular, it could be instantiated to `type Lock[X] = Int`,
and then clearly even `Lock[Int]` and `Lock[String]` would not
be disjoint.

By making it a `sealed trait` instead, it is a concrete class type,
and so `Lock[Int]` and `Lock[String]` are actually disjoint.
No matter what we do, `Int` cannot be proven disjoint from `Any`
or from `1` (to take the first two tests that are removed). It
happened to work under the unspecified match types before SIP-56,
but there was evidence to guarantee its soundness.

It does not seem like the larger project needs those particular
tests to pass to be meaningful. So we suggest to remove them, and
focus on types that are actually provably disjoint.
@kory33
Copy link
Owner

kory33 commented Dec 8, 2023

Thank you for the PR!

I feel that I need to go through SIP-56 (I once did so but there seems to be some updates since then) to really understand the issue and your solution. Please allow me a few days to digest this 👀

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