-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add support for kinds with type restrictions (#203) #204
Add support for kinds with type restrictions (#203) #204
Conversation
0a95f3a
to
624fddd
Compare
@@ -177,40 +177,44 @@ object K0: | |||
inline given mkCoproductInstances[F[_], T](using gen: CoproductGeneric[T]): CoproductInstances[F, T] = | |||
ErasedCoproductInstances[K0.type, F[T], LiftP[F, gen.MirroredElemTypes]](gen): CoproductInstances[F, T] | |||
|
|||
object K1: | |||
type Kind[C, O[_]] = C { | |||
abstract class K1T[TypeRestriction]: |
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.
If we're doing this we should add a lower bound too. And I wouldn't call this TypeRestriction
but LB
and UB
or Lower
and Upper
, something like that.
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 can make that change!
Now that #206 has been merged I think this belongs in a separate library, or at least a separate module. In particular I think that adding bounds to the existing |
@milessabin since 206 was merged, my motivation to need to merge the change I was suggesting is much less. That being said, I think those are probably the 2 most common kinds with bounds, and might benefit a library user at some point. I am curious why you think type inference would be affected? In this case K1 and K11 are exactly as they are before, they are just extending from K1T/K11T, and locking in the bounds. |
Possibly I'm being over cautious here. It would be good to test the new definitions against kittens at least. Aside from that I think the irregularity bothers me. Why no similar treatment for Kind[C, O[_[_ <: TypeRestriction]]] rather than, Kind[C, O[_[t] <: TypeRestriction[t]]] for instance? In the absence of true kind polymorphism we have to stop somewhere. The current set of kinds is adequate for all of cats (@joroKr21 is that true? I think it is), which makes it a fairly salient place to rest. |
I doubt that.
Agreed, that would be a good smoke test.
Me too - if we add support for bounds, it should be on all current kinds.
Actually I think it should be: Kind[C, O[_] >: LB <: UB] So that
Yes, that's true. I wonder if it's possible somehow to make use of |
Agreed that both upper and lower bounds would be needed for symmetry, however that's missing the point I was making, which was that a bound on a type of kind There is literally no end to this, which I why I chose to draw the line where I did. @joroKr21 you're the primary maintainer now, so this is your call, but I think you'll be making your life difficult if you go down this road. |
I think it might work with type lambdas in Scala 3 but if I'm wrong about that then I agree it's not worth the hassle. We have to try first |
I think the type variable is essential ... Which raises another issue ... what about variance? Each variance annotation creates a new kind. |
Ah yeah it doesn't work as I expected: scala> class Foo[F[_] <: [x] =>> List[x]]
// defined class Foo
scala> new Foo[List]
-- Error: ----------------------------------------------------------------------
1 |new Foo[List]
| ^^^^
| Type argument List does not have the same kind as its bound [_$1] <: List
1 error found
scala> class Foo[F <: [x] =>> List[x]]
// defined class Foo
scala> new Foo[List]
val res0: Foo[List] = Foo@50df37eb |
No description provided.