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

Proposal: lateinit property intrinsics #73

Closed
wants to merge 6 commits into
base: master
from
@@ -0,0 +1,215 @@
# Lateinit property isInitialized intrinsic
Goal: provide a way to check whether a `lateinit` property was assigned.
## Motivation
Original issue: https://youtrack.jetbrains.com/issue/KT-9327.
A prominent use case is tests, for example JUnit. A lateinit property may or may not be initialized in `setUp`, however `tearDown` must perform the cleanup only if the property was initialized, or otherwise it would have to catch `UninitializedPropertyAccessException`.
``` kotlin
class Test {
lateinit var file: File
@Before fun setUp() {
file = createFile()
}
@After fun tearDown() {
if (... file has been initialized ...) {
file.delete()
}
}
}
```
## Description
We propose to add a new declaration to the standard library: an extension property `isInitialized`. It would be available _only on a property reference expression_ that references a lateinit property, accessible at the call site. It's inline and intrinsic, i.e. the corresponding bytecode is generated manually by the compiler at each call site, because it's not possible to implement it in Kotlin.
``` kotlin
package kotlin
import kotlin.internal.InlineOnly
import kotlin.internal.AccessibleLateinitPropertyLiteral
import kotlin.reflect.KProperty0
/**
* Returns `true` if this lateinit property has been assigned a value, and `false` otherwise.
*/
@SinceKotlin("1.2")
@InlineOnly
inline val @receiver:AccessibleLateinitPropertyLiteral KProperty0<*>.isInitialized: Boolean
get() = throw NotImplementedError("Implementation is intrinsic")
```
`AccessibleLateinitPropertyLiteral` is an internal annotation (accessible only in standard library sources) defined as follows:
``` kotlin
package kotlin.internal
/**
* The value of this parameter should be a property reference expression (`this::foo`), referencing a `lateinit` property,
* the backing field of which is accessible at the point where the corresponding argument is passed.
*/
@Target(AnnotationTarget.VALUE_PARAMETER)
@Retention(AnnotationRetention.BINARY)
@SinceKotlin("1.2")
internal annotation class AccessibleLateinitPropertyLiteral
```
To call `isInitialized`, simply pass a bound reference to the property in question as the receiver:
``` kotlin
if (this::environment.isInitialized) {
this::environment.configure()
}
```
## Resolution
`isInitialized` is a normal declaration that is resolved according to the standard Kotlin rules. To prevent it from being called on any `KProperty0` instances except symbolic references, we implement a new call checker (call checkers are run only after the resolution is complete and successful). This checker is going to check that an argument passed to a parameter annotated with `AccessibleLateinitPropertyLiteral` is indeed a property literal (after being deparenthesized) and its backing field is accessible at the call site.
``` kotlin
class Test(val name: String) {
lateinit var file: File
fun test() {
this::file.isInitialized // OK
(this::file).isInitialized // OK
val otherTest = Test()
otherTest::file.isInitialized // OK, backing field is accessible, even if it's another instance
this::name.isInitialized // Error, referenced property is not lateinit
val q: KProperty0<*> = getProperty(...)
q.isInitialized // Error, receiver must be a property literal
val p = this::file
p.isInitialized // Error, receiver must be a property literal
}
}
class Other {
fun test() {
Test()::file.isInitialized // Error, backing field is not accessible here
}
}
```
Note that the `AccessibleLateinitPropertyLiteral` annotation is needed mostly for the users who are looking at these declarations in standard library sources. We could hard-code the support of exactly `isInitialized` (and maybe `deinitialize` in the future) from the standard library into the call checker, however it would be difficult for a user to understand why it's not possible to pass any `KProperty0` instance, not just a reference.
## Code generation
Using property's getter to check `isAccessible` is not possible because it throws an `UninitializedPropertyAccessException` if the property is not initialized. So, the compiler must generate direct read access to the backing field's property. The requirement "must be a reference to a property, the backing field of which should be accessible" is thus needed because the compiler must know _statically_ which property is referenced, check that it's a lateinit property, and ensure that it's possible to generate access to the backing field.
The generated bytecode itself is very simple. Because `isInitialized` is an intrinsic, we're able to avoid the generation of the anonymous class for the property reference and use the `GETFIELD` instruction directly.
``` kotlin
class Test {
lateinit var file: File
fun test() {
if (this::file.isInitialized) { // ALOAD 0, GETFIELD Test.file, IFNULL ...
...
}
}
}
```
### Backing field accessibility
We'd like to allow calling `isInitialized` on as many properties and from as many different contexts as possible. However, without certain limitations we would have problems with source and binary compatibility.
The main limitation is that we do not allow calling `isInitialized` on a lateinit property declared in another class, lexically unrelated to the class or file of the call site. It's not that we don't know what bytecode to generate. If the property (and thus its backing field) is public, we could generate `GETFIELD` exactly in the same way as for the property in the containing class. However, doing so would make removing the `lateinit` modifier on a property, which is today merely an implementation detail, a source-breaking and binary-breaking change. We don't want to make the fact that the property is `lateinit` a part of its API, so we disallow usages from other classes.
``` kotlin
class Test {
lateinit var file: File
}
class Other {
fun test() {
Test()::file.isInitialized // Error!
}
}
```
Note that we could allow usages from other classes _in the same file_ but for simplicity, and to avoid errors when moving classes between files, we disallow such usages at the moment.
Note that we _do_ allow usages from nested/inner classes of the property's declaring class, as well as from any lambdas and local classes inside members of the declaring class.
``` kotlin
class Test {
lateinit var file: File
inner class Inner {
fun test(t: Test) {
this@Test::file.isInitialized // OK
run {
t::file.isInitialized // OK
}
}
}
}
```
In case the property is private and is accessed from a nested/local class or lambda, the JVM back-end must generate a synthetic accessor for the backing field:
``` kotlin
class Test {
private lateinit var file: File
fun test() {
run {
if (this::file.isInitialized) { // ALOAD 0, INVOKESTATIC Test.access$getFile, IFNULL ...
...
}
}
}
}
```
### Usages in inline functions
Usages in inline functions are risky because the generated bytecode references the backing field and thus exposes it to all clients who can see the inline function. If the property is made non-lateinit (which, as discussed earlier, should be a source & binary compatible change), already inlined bytecode would no longer work. Therefore we propose to disallow usages of `isInitialized` in inline functions for now.
``` kotlin
class Test {
lateinit var file: File
inline fun getFileName(): String? {
if (this::file.isInitialized) { // Error!
return file.name
}
return null
}
}
```
## Tooling support
Code completion in the IDE must only propose `isInitialized` if the call is going to be allowed by the compiler, i.e. on an accessible lateinit property reference.
## Known issues
* The solution is admittedly very ad-hoc. The `AccessibleLateinitPropertyLiteral` annotation serves only one particular purpose which is hardly generalizable to be useful to solve any other problems.
* Because of the weird nature of the proposed call checker, trivial refactorings like "Extract variable" or "Use .let" could break the code here in an unexpected way:
``` kotlin
if (this::file.isInitialized) ... // OK
// Extract variable
val property = this::file
if (property.isInitialized) ... // Error!
// Use .let
if (this::file.let { it.isInitialized && it.get() == ...}) ... // Error!
```
## Future advancements
* Similarly to `isInitialized`, we could provide the `deinitialize` intrinsic for resetting the lateinit property value back to `null`. There doesn't seem to be immediate value in providing it though, so we postpone this until more use cases arise.
ProTip! Use n and p to navigate between commits in a pull request.