Smartcast for nullable variable properties

@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

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.

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

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

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.

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.

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

When they occur they can be worked around nicely.

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

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

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.

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.

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

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.

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

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

@arocnies Thanks for the hint, I will use it as well. Yes, I know about working smart cast on local variables (unless they are captured in some non-inline closures!).

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.

The point is that the Kotlin (especially JVM) is not designed as functional language/platform. Writing code in pure functional style may end up with tons of data being copied around. Truly functional languages, e.g. Haskell, have special implementation under the hood which prevents data copying wherever possible (among with other things which make its runtime different from imperative languages). So writing everything functional may not always be a good idea in an imperative language.
Anyway, thanks for the help! At least new idoms learned.

Gotcha :slight_smile:

Just to note that multi-assignment isn’t possible in Kotlin right now but I’ve seen it debated elsewhere. I’d be interested to see what other idea’s people have if they are also dealing with capturing lots of non-final variables.

I just don’t think the unsafe smart-casts are the right answer, IMHO.

(Maybe there’s no good alternative and it’s just a good code smell that helps coders know when to refactor).

Oh, wow… I’ve been writing a lot of Kotlin for a couple of years now and never used !! (maybe except in a few unit tests where I was just lazy to actually do assertions that would have the same effect of just causing the test to fail). If you’re using it in Kotlin, I invite you to try to learn new ways of writing code which make var basically non-existent in the code base, and specially nullable vars - trust me, it’s not hard, and I am definitely not talking about pure FP here. Mutability outside of local variables, in general, makes code extremely hard to reason about, as many other posters have explained above… but if you keep to your conviction that it’ fine to use nullable, mutable state all over your code, then I would kindly suggest you should stick with Java as what you’re asking for is a unsafe Java construct, the kind of bug-prone thing whose elimination is the whole reason for Kotlin to exist.

2 Likes

Another idea that would make the original code easier to write would be the if let construct from Swift and Rust:

var maxSize: Int? = null

fun doSomething() {
    val doRoll = if (let max = maxSize) {
        // max not null here
       size >= max
    } else false
}

Well it does

val doRoll = maxSize?.let { size >= it } ?: false
2 Likes