Open
Conversation
`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.
Owner
|
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 👀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.