-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix #7554: Add TypeTest for sound pattern type test #7555
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
Conversation
516ca89
to
8a8d8ec
Compare
6a52c40
to
d962d0b
Compare
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.
I strongly maintain that we should use ClassTag
for this and not TypeTest
. Let's fix/generalize broken abstractions instead of inventing new ones. That's the only way to keep complexity down.
And I strongly maintain that |
As @sjrd stated the issue is not that that ClassTag
s are broken but only the type test functionality that was added. This redefines the type tests functionality in a separate TypeTest
class that has more a generalized API and stricter contract that fixes the shortcoming mentioned in #7554 and https://infoscience.epfl.ch/record/261291?ln=en .
41931cf
to
dcb296e
Compare
We still need this to make the |
Is there a legal open-access version of https://infoscience.epfl.ch/record/261291? |
It looks like the paper can be freely downloaded from https://dl.acm.org/doi/10.1145/3241653.3241658 |
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.
I am still deeply opposed to introduce yet another form of type tag. We have too many of those already! So, I am sorry, that's a no go. At least not until we have tried out and tested all the alternatives.
Let's try to make a subtype of ClassTag
that tests for prefixes. If that has a chance of working, it's much preferable to any alternative. I know some of you disagree on this, but I'll put my foot down on this one.
In #8993 I found that providing ClassTags were the type test is sound can lead unsoundess when using those ClassTags in arrays. |
1474c9a
to
3356dcf
Compare
464aaff
to
d35f356
Compare
If it preserves (or improves the precision of) the semantics of shapeless's shapeless has a gadget for using its @Test
def testTypeCase: Unit = {
import HList.ListCompat._
def typeCase[T: Typeable](t: Any): Option[T] = {
val T = TypeCase[T]
val `List[T]` = TypeCase[List[T]]
val `(String, T)` = TypeCase[(String, T)]
val `List[(String, T)]` = TypeCase[List[(String, T)]]
t match {
case T(t) => Some(t)
case `List[T]`(lt) => lt.headOption
case `(String, T)`(s, t) => typed[String](s) ; Some(t)
case `List[(String, T)]`((s, t) :: _) => typed[String](s); Some(t)
case `List[(String, T)]`(lst) => assertTrue(lst.isEmpty) ; None
case _ => None
}
}
assertEquals(Some(23), typeCase[Int](23: Any))
assertEquals(None, typeCase[String](23: Any))
assertEquals(Some(23), typeCase[Int](List(23): Any))
assertEquals(None, typeCase[String](List(23): Any))
assertEquals(Some(23), typeCase[Int](("foo", 23): Any))
assertEquals(None, typeCase[String](("foo", 23): Any))
assertEquals(Some(23), typeCase[Int](List(("foo", 23)): Any))
assertEquals(None, typeCase[String](List(("foo", 23)): Any))
} It would be great if Scala 3 supported pattern syntax similar to the above without having to resort to backtick puns. |
d35f356
to
142e6c0
Compare
142e6c0
to
39702c5
Compare
`TypeTest` is a replacemnt for the same functionallity performed by the `ClassTag.unaplly`. Using `ClassTag` instances happend to be unsound. `TypeTest` fixes that unsoundess and adds extra flexibility with the `S` type. `ClassTag` type tests will still be supported but a warining will be emitted after 3.0.
39702c5
to
0bb5301
Compare
Rebased |
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.
Code looks good, it's just the doc page that needs some fixes.
2b96e49
to
7b2cc30
Compare
7b2cc30
to
5dec63f
Compare
Co-authored-by: Jamie Thompson <[email protected]>
Comparison with other kinds of type representations https://gist.github.com/nicolasstucki/ae3eaae553f594066bf9ad0e817f9a63