A New UnsafeNulls Language Feature for Explicit Nulls

[noti0na1]

This PR removes the UncheckedNull type, and adds a new language feature unsafeNulls to explicit nulls, which allows unsafe null operations in a certain scope. In addition, a relaxed overriding rule is implemented for overriding Java classes.

Description

In the original explicit nulls design, UncheckedNull was introduced to allow member selections on nullable objects from Java code (with type T | UncheckedNull). We could define UncheckedNull as a type alias of Null or a true subtype of Null; however, we find it is difficult to implement in both ways. See #7828 for more discussion.

We decided to create a new language feature, called unsafeNulls. Inside this "unsafe" scope, all T | Null objects can be used as T, and Null keeps being a subtype of Any.

Users can import scala.language.unsafeNulls to create such scope, or use -language:unsafeNulls to enable this feature globally. The following unsafe null operations will apply to all nullable types:

1. select member of T on T | Null object
2. call extension methods of T on T | Null
3. convert T1 to T2 if T1.stripAllNulls <:< T2.stripAllNulls or T1 is Null and T2 has null value after erasure
4. allow equality check between T and T | Null

The intention of this unsafeNulls is to give users a better migration path for explicit nulls. The UncheckedNull is only limited to unsafe member selections. We think this approach can do more than UncheckedNull and in a better way.

Projects for Scala2 or regular dotty can try this by adding -Yexplicit-nulls -language:unsafeNulls to the compile options. A small number of manual modifications are expected (for example, some code relies on the facts of Null <:< AnyRef). To migrate to full explicit nulls in the future, -language:unsafeNulls can be dropped and add import scala.language.unsafeNulls only when needed.

Examples

All the examples are under -Yexplicit-nulls flag.

def f(x: String): String = ???

import scala.language.unsafeNulls

val s: String | Null = ???
val a: String = s // unsafely convert String | Null to String

val b1 = s.trim() // call .trim() on String | Null unsafely
val b2 = b1.length()

f(s).trim() // pass String | Null as an argument of type String unsafely

val c: String = null // Null to String

val d1: Array[String] = ???
val d2: Array[String | Null] = d1 // unsafely convert Array[String] to Array[String | Null]
val d3: Array[String] = Array(null) // unsafe

Without the unsafeNulls, all these unsafe operations will not be compiled.

unsafeNulls also works for extension methods and implicit search.

import scala.language.unsafeNulls

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

given Conversion[String, Array[String]] = _ => ???

val y: String | Null = ???
val z: Array[String | Null] = y 

See more examples about unsafeNulls in tests/explicit-nulls/pos/unsafe-*.scala.

An example for relaxed overriding:

abstract class J {
  abstract void f(String x);
}

// both S1 and S2 are acceptable

class S1 extends J {
  override def f(x: String) = ???
}

class S2 extends J {
  override def f(x: String | Null) = ???
}

See more examples about overriding in tests/explicit-nulls/pos/override-*.

We are also considering a Java-compatible flag, which enables unsafeNulls to all Java method calls. This will be in a seperate PR.

[odersky]

test performance please

[dottybot]

performance test scheduled: 1 job(s) in queue, 1 running.

[dottybot]

performance test failed:

Please check http://lamppc37.epfl.ch:8000/pull-9884-10-08-14.45.out for more information

[odersky]

We need a performance test for this. I tried but the scodec conflicts need to be resolved first.

Overall, I think the approach is workable, but we should work on streamlining with the goals of increasing clarity and efficiency.

My main concern is that there are currently too many booleans that are tested in too many places. We should try to standardize on a minimal set of switches only and define them all in the same place (probably as Mode bits). Then, add a doc comment explaining what each boolean means and where it is used, and why it is different from the others.

[noti0na1]

@odersky I have rebased the branch and scodec. Will the performance test run if I comment "test performance please" here?

[odersky]

I did a first pass. I have not looked at everything yet. The general direction looks good IMO but right now I find the logic complicated to follow because there are too many mode switches represented as boolean parameters with defaults. So before going further, I have the following suggestion:

Try to formulate the logic without using any boolean default parameters. Work on streamlining things in the context instead. Default parameters are a too easy way to hide complicated logic. Once that is done, consider bringing back some selected default parameters, but only if that leads to performance improvements or more streamlined code (but only after all other alternatives to streamline code have been considered).

[odersky]

I also added a commit with some small fixes of typos and wordings.

[odersky]

Hi @noti0na1. What's the current state of this? Do you still want to work on improvements here?

I just had a look again, but I still find there are too many modes and default parameters floating around. Is there no way to bundle all of it in a mode bit in the context, say?

Please re-assign to me, when you think this is ready for review again

[noti0na1]

@odersky I have combined all the commits.

[odersky]

test performance please

[dottybot]

performance test scheduled: 2 job(s) in queue, 1 running.

[dottybot]

performance test failed:

Please check http://lamppc37.epfl.ch:8000/pull-9884-02-10-18.40.out for more information

[odersky]

This PR still has a lot of complications, which might not be necessary. To be able to merge this, it should be simplified ruthlessly. I mean not just a few cosmetic changes but every element of this PR should be put to the test whether it is really needed. I did that with #11375, and found that a lot of concepts seem not to be needed, after all. At least all the explicitNulls tests pass if they are removed. Among the removed constructs are:

• The UnsafeNullsSubType mode
• The double caches in Implicits
• The withOrNull in translateParameterized
• The special case in adapt
• A few other smaller things

So, maybe it's best to take #11375 as a basis, and only add some specific changes to this simpler base that are needed for correctness.

The reason I am so strict here is that I worry about future maintainability of the compiler codebase. Maintainers need to be able to verify and modify every single element of the codebase. Unnecessary code, and worse, unnecessary abstractions and conditions are
very bad for this since they destroy confidence that the code we deal with is there for a reason.

[odersky]

What about we try the following procedure to push this over the finish line?

• Start with Attempt to Simplify UnsafeNulls #11375 as the commonly agreed basis. (If possible, split it again in several commits that handle individual features, but this is not mandatory),
• Look at each remaining failure in the community build and minimize it into a test. Then, add the simplest fix with a comment in the code that refers to the failing test. That way, we can have an audit trail that tells us why the new distinctions and changes are required.

When in doubt, let's discuss each failure and proposed fix together.

[odersky]

Turns out the only failing CB test in #11375 is shapeless. This could be since shapeless has not been updated like it has here. @noti0na1 can you do the update a see what the status is?

[noti0na1]

@odersky Sorry for the late response. I will merge and look at these changes soon.

[noti0na1]

@odersky I have merged the changes from your PR. I think the introducing of SafeNulls really simplifies the complexity.

These are several tests for eligibleCache (searching inside and outside of unsafeNulls scope at the same time), and they all pass the test.

I will update the document recently.

[odersky]

@noti0na1 Should we merge this then?

[odersky]

Looks very good now! I think this is a great basis to experiment with and to possibly refine things later.