-
Notifications
You must be signed in to change notification settings - Fork 458
Add better contracts using 2.2.20 #3725
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
base: main
Are you sure you want to change the base?
Conversation
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.
😍
returns(true) implies (this@Either is Right<B>) | ||
returns(false) implies (this@Either is Left<A>) |
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.
This is so amazing 😍
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.
Wasn't this possible already, or am I stupid lol? Maybe I tried it on 2.2.20 as well.
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.
Before 2.2.20, the compiler was only aware of the "raw type", so you sometimes would lose some type information. In some versions including the generic parameter would even confuse the compiler, since it would create a "new A
type parameter", instead of understanding it was a reference to the actual one -- this is the reason the current code only states is Left
.
But in the new version you can declare contracts about generic parameters, so this part can be readily improved.
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.
Oh that's incredible! I have some usecases for hacky DSL stuff that I'll be trying out soon then.
I thought that this was allowed when the type parameter was known because it was a subclass of a class with type parameters though. Like AFAIK this Left
wasn't actually a raw type, it was secretly a Left<A>
that the compiler accepts a shorthand for just like when you do when(either) { is Left -> ... }
. Maybe I'm misremembering though...
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.
This compiles fine on 2.0.21 (playground):
import kotlin.contracts.*
fun main() {
val foo: Either<Int, String> = Either.Right("hi")
if (!foo.isLeft()) {
println(foo.value)
}
}
sealed class Either<out A, out B> {
class Right<B>(val value: B): Either<Nothing, B>()
class Left<A>: Either<A, Nothing>()
fun isLeft(): Boolean {
contract {
returns(true) implies (this@Either is Left<A>)
returns(false) implies (this@Either is Right<B>)
}
return this is Left
}
}
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.
Is the idea here that is Left<A>
and is Left
used to cause differing behaviour pre 2.2.20, and now also cause differening behaviour, with is Left<A>
having the behaviour we desire?
I have no problem with this change; it just feels like is Left
and is Left<A>
should have the same behaviour now, and anything other than that is a compiler bug.
Or are we just doing is Left<A>
post 2.2.20 for the sake of code style?
Unfortunately adding these poisons the binary, so we cannot release them yet :(