Null safety smart cast in conditional expression

The Kotlin compiler doesn’t recognize that this situation is null safe:

class Test(var func: (() -> Boolean)?) {
	init {
		// In either of these cases the `func()` call is null safe,
		// but the compiler doesn't currently smartcast.
		var example = func == null || func()

		if(func != null && func()) {
			// do something
		}
	}
}

This is (presumably) a simple enhancement, but I’m not sure where example cases like this belong. If this is the wrong place please point me where I should post it. Thanks.

Though smart cast works in func == null || func.invoke()

Though smart cast works in func == null || func.invoke()

Not as far as I can tell. I still get Smart cast to '() -> Boolean' is impossible, because 'func' is a mutable property that could have been changed by this time thrown at me. The thing is that this all occurs within an init block, so func is in fact guaranteed to be unmodified up until this point.

The issue is that func is declared as mutable. If you used val func, you wouldn’t have this issue. We can reason that func isn’t mutated between the check and call, but theoretically, another thread could mutate func at that time.
You can bypass this with if (func?.invoke() == true).

Not in this case since the code is in the init block of the class. Therefor no other thread should have a reference to the instance. Yes in some rare cases the constructor can leak the reference early but this is always a bug, since no class should be accessed before the constructor finished execution.
Maybe kotlin can be a bit less strict about nullability in constructors.

What if you start an asynchronous task earlier within the init block that reassigns func to be null?

In that case the compiler can figure that it is potentially unsafe. The argument is not that it is impossible to be unsafe. The argument is that the compiler should be able to determine that this case is definitely safe.

1 Like

Suppose that within init you pass this as an argument to a method on another object. Might that method then mutate func on this? We don’t know. Suppose we call another method on this. Is the compiler now going to inspect that entire method for asynchronous mutations? You’re asking the compiler to do significantly more work, and for what? In the above case, func?.invoke() == true already solves the problem in about as much code, and for other cases you can start the init block with val func = this.func.

Nobody here wants the compiler to figure out all of those cases. For those cases it would be fine if the compiler would just say it is potentially unsafe without further analysis. That is, if a reference to ‘this’ is leaked, then it is potentially unsafe, otherwise it is definitely safe.

We don’t need to further analyze the potentially unsafe cases. Even though they could potentially be safe, it is Ok to treat them as unsafe, just like it is now. Smart casting in the definitely safe case would arguably improve the majority of use cases.

The case provided in this thread could definitely be smart casted, without significantly increasing the compiler’s work.

Then what you’re asking for is a somewhat complicated compiler-side solution to a very narrow problem (has to be in an init block that has to lack any earlier delegation to other methods that may include this) that already has a solution (assign to a val, or even easier, use the built-in null-safe calls).

Additionally, if the compiler accepted that null-checking a var like this in this situation is okay, a novice programmer will learn that it is okay here, and then be confused when it doesn’t work later in a different context.

This is already the case. Hence this thread.

No, this thread is a programmer asking for an exception to the rule that vars are assumed to be mutable at all times. The var/val is an important distinction that any Kotlin programmer needs to learn, and the error message is helpful towards that goal. A novice programmer who encounters the rule should learn to make null-safe checks and assign to vals where immutability is important. We to not need to make it a var/val/var within an init block distinction.

I wholeheartedly disagree. Var variables are already allowed to be smart casted in some contexts (local variable vars). So the reason why it is not smart casted is not var vs val.

The reason is that the compiler is not smart enough to figure out that it is definitely safe to be smart casted.

Ah, you’re right, I was not aware of the local variable smart-casting for vars.

I never tested the following though, and now I am curious: when a local var iş captured by a (contract-less) lambda, does the compiler prevent smart casting, and what is the error message in that case? Unfortunately I am on vacation and cannot test this.

[SMARTCAST_IMPOSSIBLE] Smart cast to 'Int' is impossible, because 'x' is a local variable that is captured by a changing closure

Thank you very much. That’s very cool.

Yes, it’s a bug. But no, it’s not rare. Unfortunately. A typical scenario: a base class calls an overridable function in the constructor without even documenting it, a subclass overrides this function and then nobody knows where this can end up before even the base constructor finishes. For a real life example, have a look at JTable constructor. It indirectly calls tableChanged() which is often overridden for obvious reasons and then all kinds of bad things happen at construction, final fields being null and other such stuff.

Of course, in this case there is no base class, but I’d rather have this assumption that var is always mutable rather than some arcane set of rules to determine when it can be mutated or not.

1 Like