Smartcast for nullable variable properties

Language Design
1

After writing several projects using Kotlin I still cannot understand why this design decision has been made. I mean disallowing smartcast for nullable variable properties.

06
061520Ă—178 26.4 KB

I will try to explain why it is so bad.

  1. You need variable properties in your classes. This is true unless you are writing in pure functional language which is not our case.
  2. You need some of them to be nullable occasionally. One may say that you can use some kind of Optional for that. But this, firstly, is unnecessary overhead over just a nullable value which exists for the same reason. And the most important, secondly, does not have any principal difference with nullable type, and, if the compiler would be aware of it, it could say “Smart cast from Optional<T> to T is not possible because it might be changed since isPresent() was called”.
  3. Every time you access your variable in your class, you are sure that you can do it safely here, either by ensuring it is done in one thread or under some kind of synchronisation. All the code in the world which uses variables and does not have race bugs there, is such. I understand that under this design decision probably was some idea about helping to prevent race bugs for variable access, but it does not work. The compiler actually cannot detect if there is a race, and that’s the reason why it always prohibits smart casting of variable properties. And while it does not help to prevent them, it really hurts making all such code full of !! (wedge-writing, brainfuck-style, name it as you like), making it ugly and less readable. Most probably another reason (and the only “useful” effect) is that !! ensures that in case the value is null, Kotlin nice NPE is thrown instead of not-invented-here vanilla Java NPE. That’s, honestly, is absolutely no point for me personally, but the same check still can be done under the hood on each access to smart-casted nullable variable, just remove !! from the code in this case.

What do you think, people? Am I alone with this pain?

1 Like
2

The thing to do here is use let:

maxSize?.let{ 
    it > 500
}

That cast might not be possible because, as the tooltip says, the value might have changed. If it did the cast you’d have a ClassCastException in code which apparently doens’t do any casting.

3

This is just one example which BTW has else branch as well. I know, it can be combined also with Elvis operator. But if I need several values in one expression? Nested “let”'s? Using let(), also() and others is also decreasing code readability comparing with just a regular use of smart-casted values.

That cast might not be possible because, as the tooltip says, the value might have changed.

I understand, probably you did not get the point (have you read below the code example?). The cast is not possible because of controversial design decision which I am trying to discuss for possible change.

4

I was about to link this post until I realized you were the original poster of that topic as well.

This design limitation makes sense and should not change. Allowing non-final variables to be smart cast is unsafe. Yes, race conditions are one concern but it’s also the least of the concerns. The real issue is any side effect that would cause the value to change.

In your case, you should perform an unsafe cast of your object. This is what you’re wanting the compiler to do for you, unsafe smart-casts.

Of course, you could also change your code to be safe by capturing the non-final variable using a local variable or closure, but that’s not directly addressing your proposal for unsafe smart-casting.

1 Like
5

Allowing non-final variables to be smart cast is unsafe. Yes, race conditions are one concern but it’s also the least of the concerns. The real issue is any side effect that would cause the value to change.
In your case, you should perform an unsafe cast of your object .

It is formally unsafe. Like any imperative programming. The only formally safe way is pure functional programming, e.g. in Haskell. But 99.9% of world is not Haskell. So we need to live with that. As I said in the beginning, in practice such access is safe if you care about thread-safety and proper synchronisation. And the compiler cannot ensure you do, but instead of at least not hurting, it makes hell for accessing my mutable state. And you will have problems if you don’t care, even if you make unsafe cast or closure capture, it does not help.

Of course, you could also change your code to be safe by capturing the non-final variable using a local variable or closure

And how this capturing helps me, except making the code less readable? If I have several mutable properties, I cannot capture all of them at once atomically thus making this capture just an illusion of safety (among with less readable code). Again, the only way to be safe is ensuring thread-safety and proper synchronisation, where the Kotlin has nothing to do now.

6

@vagran I’ve read the rest of your post, and I’ve explained you why that doesn’t work, it’s not safe.

And there are lots of cases where that cast is provably wrong, without races:

if (x != null) {
   x = if (foo) 5 else null
   if (x == null) { /*...*/ 
   } else {
   }
}

Should the if be optimized away here?

And note that the assignment can be hidden in another method, maybe a virtual function call which can be overridden in a subclass.

1 Like
7

And note that the assignment can be hidden in another method, maybe a virtual function call which can be overridden in a subclass.

I understand this but my point is that the programmer is responsible for taking care about proper synchronisation (which he does anyway, the compiler does no magic to eliminate this responsibility from the programmer).

Should the if be optimized away here?

The answer is well-defined. Yes, it should if the x is not declared as volatile.

BTW Kotlin makes me impression of some not really good elaborated concurrency support. Among with the topic subject it is also having such important thing as volatile as annotation, synchronized as inline function (which also is my concern but minor, it is inconvenient in many cases, e.g. not able to make “continue” from “synchronzed” in a loop). Yes, the explanation is different backends, like JS, but the language suffered from this.

8

I think @al3c meant something like this

open class Foo {
    var x: Baz? = Baz()
    open fun doSomething() {  }

    fun someFunction() {
        if(x != null){
            doSomething()
            x.bar()  // NPE here if someFunction is called on Bar
        }
    }
}
class Bar: Foo()
    override doSomething() { x = null }
}

This is the reason why smart casting for var is not allowed. The only way to ensure that x is not changed is to analyze the control flow of every function called before x is accessed. This would just be a lot of computation which in most cases would not even lead to any result. open functions and calls into different libraries might change x at any time and there is no way of checking it. This would prohibit smart cast in 95% of all cases anyways. Add race conditions to that and there really is no point in allowing smart casts at this point.
Any sufficiently simple case that a compiler check would allow smart casting can easily be done using x?.let { ... }.

A possible change to kotlin I might support would be to add an unsafe block to the language which disables some checks inside of it.

3 Likes
9

A similar problem was discussed in this topic: Smart casts and nullability in single-threaded contexts.

It was a heated discussion, but with no result other than the issue in the tracker KT-20294, where all that was restated again, and a proof-of-concept fork of the compiler supporting @AssumeMultiThreadSafe annotation to mark var properties that can be smartcasted.

1 Like
10

Even if the code is guaranteed to run in a single thread, there’s still the possibilities for side effects:

// Foo defined elsewhere (not local variable) as non-final, nullable String.
if (foo != null && foo.length > 0 /* foo could be null here */) {
    // foo could be null here
    foo = "Hello world"
    // foo could be null here
    someOtherMethod() 
    // foo could be null here
}

Maybe variables that are local, don’t use delegation, and don’t have custom getters could be smart casted? Still, worst case scenario is to do a normal cast like you would in Java.

11

A similar problem was discussed in this topic: Smart casts and nullability in single-threaded contexts.

Yes, that’s about it. At least I am not alone with that pain. My statement is that any real-world project will be full of !!, nested “let”'s or other ugly workarounds just to overcome this language design flaw.

The usability of Kotlin is like 3x better than Java’s in all other aspects, but for some reason, mutable nullable fields are comparatively horrible to use (and if it wasn’t for lateinit , non-null fields would be just as bad)… It just seems like such a wart on an otherwise beautiful language…

Fully agree with @mslenc in that statement. It’s a pleasure to write in Kotlin after Java …unless you do it for real-world project which is full of mutable states, and you are responsible for access synchronisation. Compiler is your enemy there, this shouldn’t be so in a production-ready language. And as I said before, this situation is absolutely normal for imperative programming, problem is not in your code, it is in the compiler design. I hope that the language developers will understand this one day as well as the community (which I am pretty sure will happen while more and more people will try to use Kotlin for more complex projects), and then, I hope, there will be some solution.

Another point - actually this “problem” which the compiler is trying to fight with, is valid not only for nullable variables. Any mutable variable formally can be unexpectedly modified if the programmer is not taking care of it.

if (x == 42) {
    thisFunctionWillCorruptDataIfArgIsNot42(x)
}

There is no principal difference if we expect non-null or any other expected values. Unexpected change of mutable state is exclusively programmer headache, the compiler just should not make it worse.

12

The reports in this thread are greatly exaggerated. We are using Kotlin in a huge real world project, and problems with nullable mutable properties are not that common. When they occur they can be worked around nicely. For example sometimes thr logic within my function will allow to put this into the first line val x = this.x ?: return. There are other similar nice tricks.

2 Likes
13

When they occur they can be worked around nicely.

But it is not normal that such basic things should be “worked around”.

14

Then don’t consider them workarounds. The example I have shown makes your code easier to reason about. You can be sure what the local variable x holds within the functuon, without mental effort. It is a benefit.

1 Like
15

This example is simple, but it gonna be more complicated if you have several such variables and want to use them in expressions and change them, like you normally do, when you know that you can exclusively modify your state. In Kotlin your state is not longer yours. Compiler cares too much and limits your access to your state.

16

If think it is easy to handle these situations with a few tiny custom functions. For example:

class NullableProperties {
    var a: String? = null
    var b: String? = null
    var c: String? = null
}

fun <T: Any, U: Any, V: Any> ifAllNotNull(first: T?, second: U?, third: V?, block: (T, U, V) -> Unit) {
    if (first != null && second != null && third != null) {
        block.invoke(first, second, third)
    }
}

fun main(args: Array<String>) {
    val np = NullableProperties()
    ifAllNotNull(np.a, np.b, np.c) { a, b, c ->
        println("A: ${a.length}, B: ${b.length}, C: ${c.length}")
    }
    np.a = "a"
    np.b = "bb"
    np.c = "ccc"
    ifAllNotNull(np.a, np.b, np.c) { a, b, c ->
        println("A: ${a.length}, B: ${b.length}, C: ${c.length}")
    }
}

If I use the following criteria, I personally consider this an acceptable solution:

  • Is it boilerplate? A tiny bit.
  • Is it guaranteed to work properly? Yes.
  • Is it easy to understand what is happening? Yes.
17

I like the idea but shouldn’t ifAllNotNull be inlined? That way you can use local returns.

18

It is just an example. The idea is that you write specific functions for the specific situations that you encounter. If you need inlining, add it. If you need a return value, add it. Etc. I doubt there is a function that can be made to work for everybody in all situations.

19

Thanks for example. Good workarounds are valuable as well in this topic.

20

I think you may be on to something, but the idea of allowing unsafe smart-casts is not the solution. I’ll explain what I mean with unsafe smart-casts then suggest another idea.

Unsafe smart-casts

The request for unsafe smart-casts is similar to the request to drop the !! requirement when calling a null.
Both of these requests are based on a few ideas:

  1. The compiler and the programer know the operation is unsafe
  2. The programer somehow knows the unsafe scenario will not occur
  3. The compiler should be weaker (not enforcing type safety or null safety in this specific case)
  4. All workaround options are not good enough (because they add boilerplate)
  5. The issue is very common

We’ve talked about points #1 and #2 a good amount and I think everyone agrees there are possible cases where both of those are true.

For points #3, #4, and #5, here’s my opinions:

  1. The compiler should be weaker (not enforcing type safety or null safety)

This is a valid request. There are plenty of benefits to a weaker compiler that does not enforce safety. Groovy follows this mentality where Kotlin follows a stricter approach. Although this might look like a solution to dealing with non-final variables (to allow unsafe smart-casts) and a solution to the rampant use of !! (by not requiring !!), there’s a better solution.

  1. All workaround options are not good enough (because they add boilerplate)

The workarounds posted I would not consider “workarounds” but instead solutions. They’re “solutions” because not only do they resolve your problem, they improve your code by making it safe. Just like how the solution to rampant use of !! everywhere is to change the code to handle nulls properly, the solution to dealing with non-final variables is to change the code to capture them or use final variables. A separate issue resulting from these solutions may be the boilerplate, but that can be resolved other ways.

  1. This issue is very common and needs to be dealt a lot

Based on my experience, it’s rare to deal with a huge amount of non-final fields where I wouldn’t want to capture them as finals. Just like in Java, a great majority of your fields should be final. This is not a case of “only in a functional language is it possible”. Of course you could still be stuck in a project where mutable state is prevalent and you aren’t in a position to fix it. This issue isn’t as common as this topic would suggest.

Another Idea

Maybe what you really want is an easier way to capture local variables?
Normally, you would capture variables like this:

// Capturing in a local final variable
val x = this.x ?: "Hello World"
val x = this.x ?: error("Something went wrong")
val x = this.x!!

This would result in some boilerplate when there are multiple non-final variables that need to be captured.

val a = this.a ?: error("")
val b = this.b ?: error("")
val c = this.c ?: error("")
val d = this.d ?: error("")
...

Would adding multi-assignment fix the issue for you? This would improve your code by properly capturing the variables and may solve your issue of the difficulty of capture.

val (a, b, c, d) = (this.a, this.b, this.c, this.d)

Side note

Smart casts already work for local non-final variables

fun main(args: Array<String>) {
    someMethod()
}

//sampleStart
fun someMethod() {
    var localVar: String? = getStringOrNull()
    if (localVar != null) {
        // localVar is smartcast to type String here
        println(localVar.length)
    }
}
//sampleEnd

fun getStringOrNull() : String? {
    return "StringOrNull"
}
2 Likes