implicit conversion is not implicitly available to types of UncheckedNull

[megri]

minimized code

/*
Using compiler flag -Yexplicit-nulls, the following snippet fails compilation with
[error] 4 |    "hello, world!".split(" ").map(_.length)
[error]   |    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]   |value map is not a member of Array[String | UncheckedN
ull] | UncheckedNull
*/

"hello, world!".split(" ").map(_.length)

expectation

Extension methods/implicit conversions should be avaliable to unions of UncheckedNull

[odersky]

I had a look at this and found several fundamental problems with the way we do explicit nulls.
Here's a minimization of the problem:

  val x: Array[String | UncheckedNull] = ???
  x.map(_.length)

Without -Yexplicit-nulls, the last line compiles to

  refArrayOps[String](x).map(_.length)

With -Yexplicit-nulls the compiler tries this instead:

  genericArrayOps[String | Null](x).map(_.length)

and then it fails since _.length is not applicable to a value of type String | Null.

refArrayOps and genericArrayOps are defined in Predef like this:

  @inline implicit def refArrayOps[T <: AnyRef](xs: Array[T]): ArrayOps[T]    = new ArrayOps(xs)
  @inline implicit def genericArrayOps[T](xs: Array[T]): ArrayOps[T]          = new ArrayOps(xs)

1st Problem: Null cannot be a value type

The first thing that is wrong here is that refArrayOps is no longer applicable to String | UncheckedNull since that type is not a reference type. The compiler is forced to choose genericArrayOps instead. Even if it would succeed doing so (we'll get to that part), the difference
would be unacceptable since generericArrayOps operations are about an order of magnitude slower than refArrayOps operations.

We do not have the option of changing Predef, or any of the other libraries that make use of the concept of a reference type (most of them unknown to us). The results can range from type errors, to unexpected slow-downs, to changed behavior. So, I conclude that we unfortunately need to revise our choice that Null is a value type. It has to be a reference type instead.

I believe the most conservative option is to make Null a subtype of Object. Yes, that means that we cannot talk about the type of Objects without Null, but I don't think this is a big loss. There's little you can do with an Object anyway. And when you cast it to another type, the null gets stripped away in any case.

2nd Problem: UncheckedNull cannot be an alias of Null.

The 2nd problem is illustrated by the fact that the compiler inferred String | Null instead of String | UncheckedNull as the type argument for genericArrayOps. Inferring type parameters is quite a complicated business and alias types are slippery. The compiler is free to dealias for normalization purposes or to try some branch. This is pretty fundamental. The compiler will make a best effort to not dealias needlessly, but that's all. Even if we could tweak it to not dealias UncheckedNull to Null in this case, the next, slightly more complicated case where it would fail will be right around the corner.

So, we need to find another representation of UncheckedNull that is less readily replaced by Null.
UncheckedNull still has to be at the same time a supertype and a subtype of Null. We need

UncheckedNull <: Null

since we want to be able to assign a Java String result to a String | Null. We need

Null <: UncheckedNull

since we want to be able to pass a String | Null to a Java String parameter.

We could try an annotated type for UncheckedNull instead. If UncheckedNull = Null @HostNull, and HostNull was a refining annotation, it would not get stripped. But it would also mean that UncheckedNull is a true subtype of Null. To get back Null <: UncheckedNull, we'd have to add special rules to TypeComparer. This certainly does not look trivial, but I see no other way.

A more direct approach would be to make UncheckedNull its own class-type, and make it a subclass of Null. Then again, we need special rules to ensure that we can still have Null <: UncheckedNull.

3rd Problem: We need better test coverage

That such a simple code snippet can upset so much shows that we are only at the beginning of testing. I believe at the very least we need to be able to compile all our codebase with -Yexplicit-nulls. The next step would be to extend that to the community build. This will be a lot of work. We need more people to sign up for this.

[odersky]

/cc @olhotak

[odersky]

Another question: What is the least upper bound of Null and UncheckedNull? If they are mutual subtypes of each other, this is not well-defined. It could be either type. But since the two types have different behavior, that's a problem.

So, maybe it's better to make UncheckedNull a true subtype of Null, and turn the missing subtype relationships into implicit conversions. We should also investigate this option.

[olhotak]

When we started this project, we considered making UncheckedNull a distinct type from Null. This causes lots of problems in addition to the ones that @odersky mentions. If the two are distinct types, then Array[String|Null] and Array[String|UncheckedNull] also become distinct types. So the decision was that UncheckedNull would be Null as far as the type system is concerned, just with an annotation that would be stripped whenever the type system decides to strip it. This supports the initial motivating use case of chaining Java calls (s.trim.toUppercase), but there are many cases that it does not support. The two most common cases that we encountered are implicit conversions and generic types.

Since an UncheckedNull is unchecked by definition, it is expected to be converted at some point to a non-null type. That conversion is unsound. If such unsoundness is necessary for Java interop, we wanted it to be limited to the immediate boundary, to calls of Java-defined methods. Making UncheckedNull more magic than just Null with an annotation enables it to propagate away from the Java boundary, propagating the unsoundness throughout the code.

I would like to get rid of UncheckedNull altogether. The motivating use case s.trim.toUppercase is unsound. If we want the final code to be sound, eventually we either 1) have to add annotations to the Java methods or 2) rewrite it as s.trim.nn.toUppercase. The code is unsound, so eventually we need to make a fundamental choice, either accept unsoundness in Java interactions, or require modifications in code that interacts with Java. Code that interacts significantly with Java (expecially Scala wrappers for Java libraries) has hundreds of places where the Java code could return null, and we eventually have to make a choice between 1) accepting unsoundness or 2) going through the hundreds of places individually and deciding what the Scala code should do if a null shows up in each place. The migration to 2) is tedious, but it needs to be done if soundness is what we want.

UncheckedNull was intended to ease the migration to either annotated Java methods or s.trim.nn.toUppercase. To ease the migration, here is I think a better solution: within designated scopes (e.g. through a language import or something similar), we wrap every call to a Java-defined method with an unchecked .nn. For code that interacts heavily with Java, we can turn this on for easy migration. Then we can gradually go through the code and consider each call to a Java-defined method, turning off the insertion of unchecked .nn gradually as we do so.

On the 3rd problem, we have ported the Dotty community build (except stdlib) to explicit nulls and are writing up the experience. That writeup and the diffs would inform this discussion, but at the moment, everyone is away for the holidays so the writeup won't be ready until the new year.

/cc @abeln @noti0na1

[odersky]

Since an UncheckedNull is unchecked by definition, it is expected to be converted at some point to a non-null type. That conversion is unsound. If such unsoundness is necessary for Java interop, we wanted it to be limited to the immediate boundary, to calls of Java-defined methods. Making UncheckedNull more magic than just Null with an annotation enables it to propagate away from the Java boundary, propagating the unsoundness throughout the code.

That's a good observation. The example in the concrete issue is misleading here, since it does imply that the distance in x.map(_.length) is short where it is not. There's a lot going on before we can make this call to map! Still, it seems natural to want it to work. So, I am not sure about that aspect yet. If we keep UncheckedNull as an alias, we accept that it will be stripped away at points that look unpredictable and arbitrary to users. If we make it a true subtype of Null, we risk propagating unsoundness further than intended. Or like you say, maybe we should drop it altogether. I am not sure yet how that would work in practice.

[odersky]

One thing I noted.: If we keep UncheckedNull as an alias, we can make the map example work by adding an additional conversion for ArrayOps:

  refNullArrayOps[T <: AnyRef](xs: Array[T | UncheckedNull]): ArrayOps[T | UncheckedNull]

That will fix just a single class of uses, albeit an important one.

[smarter]

What if we reinterpret references to AnyRef coming from scala 2 as AnyRef | Null ? That should let Array[T | UncheckedNull] pass through refArrayOps.

[olhotak]

There are two issues mixed together in this example. The return type of split is Array[String | UncheckedNull] | UncheckedNull.

It's the outside UncheckedNull that prevents the implicit conversion to ArrayOps. One way to deal with this is to reverse the earlier decision to not try to search for implicit conversions on nullable types. Another way is the implicit unchecked .nn on the return value of every Java-defined method invocation, which would make the type of the split call Array[String | UncheckedNull].

Even with both of these solutions, the inside UncheckedNull is still there, so the map is still on an ArrayOps[String | UncheckedNull] and the function passed to it needs to deal with null values in the array. But this problem is separate from the issue of the implicit conversion to ArrayOps.

[abeln]

I think the "proper" way to fix the specific error reported here is to use Java annotations. With annotations, all the UncheckedNulls go away and everything works again. In order for this to work, we need two things:

• the compiler needs access to a Java jdk with nullability annotations (in our prototype this was just a large text file indicating which jdk methods return non-nullable values: https://github.com/abeln/dotty/blob/dotty-explicit-nulls-evaluation/explicit-nulls-meta.txt)
• we need to add support for recognizing Java-8 style annotations (which can appear anywhere within a type: e.g. Array[@NonNull String]). The code merged in Dotty only recognizes annotations at the "top level": e.g. @NonNull Array[String].

Regarding the more general question of "how unsound" UncheckedNull should be, I agree with @olhotak that, instead of adding additional cases for UncheckedNull, we should consider an "optimistic" mode where we assume all Java methods in a scope (be it via flag, import, or some other mechanism) return non-nullable values. This is supported by the results of our evaluation (see below).

Like Ondrej said, we just finished migrating most community build libraries to explicit nulls. The results can be found in the evaluation chapter of my master thesis (evaluation.pdf), but the gist of it is below:

We created multiple "run configurations", where certain explicit nulls features were enabled/disabled:

For example, checker has access to nullability annotations for the jdk (but not all of it), which we got from the checker framework. optimistic has a mechanism similar to what Ondrej described, where we assume every Java method returns non-nullable values (but only at the top level, so split would return Array[String|UncheckedNull]).

We then ran dotty on every (library, configuration) pair, and counted how many compiler errors were produced. The results are reported below in # errors / 1000 LOC. The idea is to use "# of compilation errors" as a proxy for how much effort it would be to migrate a specific library (there are many caveats to that statement, but it's as good of a proxy as I can think of).

By comparing the naive and java-null columns, we can estimate how "useful" UncheckedNull is (how many fewer errors are generated). The results show that UncheckedNull does not seem to be very useful in reducing the number of errors. By contrast, the optimistic mode does help quite a lot, so it seems like a desirable feature for Scala libraries that are heavy on Java interop.

[sjrd]

There are two issues mixed together in this example. The return type of split is Array[String | UncheckedNull] | UncheckedNull.

It's the outside UncheckedNull that prevents the implicit conversion to ArrayOps. One way to deal with this is to reverse the earlier decision to not try to search for implicit conversions on nullable types. Another way is the implicit unchecked .nn on the return value of every Java-defined method invocation, which would make the type of the split call Array[String | UncheckedNull].

I think this would be an appropriate thing to do, at least for conversions invoked when trying to resolve selections. Without that, the following doesn't work either:

val x: String = ...
val y = x.substring(3).length // ok
val y = x.substring(3).stripSuffix(".txt") // doesn't compile

How can we possibly explain to a newcomer that the former works but the latter doesn't? stripSuffix is an extension method on String, but it looks like a normal method selection, and selections are supposed to "see through" UncheckedNulls.

This and the original example (including the performance aspect) if, when searching for extension methods on an x: T | UncheckedNull, we first used stripUncheckedNull to actually look for extension methods of T.