add flexible types to deal with Java-defined signatures under -Yexplicit-nulls

[olhotak]

This is a continuation of #17369.

When dealing with reference types from Java, it's essential to address the implicit nullability of these types. The most accurate way to represent them in Scala is to use nullable types, though working with lots of nullable types
directly can be annoying. To streamline interactions with Java libraries, we introduce the concept of flexible types.

The flexible type, denoted by T?, functions as an abstract type with unique bounds: T | Null ... T, ensuring that T | Null <: T? <: T. The subtyping rule treats a reference type coming from Java as either nullable or non-nullable depending on the context. This concept draws inspiration from Kotlin's platform types. By relaxing null checks for such types, Scala aligns its safety guarantees with those of Java. Notably, flexible types are non-denotable, meaning users cannot explicitly write them in the code; only the compiler can construct or infer these types.

Consequently, a value with a flexible type can serve as both a nullable and non-nullable value. Additionally, both nullable and non-nullable values can be passed as parameters with flexible types during function calls. Invoking the member functions of a flexible type is allowed, but it can trigger a NullPointerException if the value is indeed null during runtime.

// Considering class J is from Java
class J {
  // Translates to def f(s: String?): Unit
  public void f(String s) {
  }

  // Translates to def g(): String?
  public String g() {
    return "";
  }
}

// Use J in Scala
def useJ(j: J) =
  val x1: String = ""
  val x2: String | Null = null
  j.f(x1) // Passing String to String?
  j.f(x2) // Passing String | Null to String?
  j.f(null) // Passing Null to String?

  // Assign String? to String
  val y1: String = j.g()
  // Assign String? to String | Null
  val y2: String | Null = j.g()

  // Calling member functions on flexible types
  j.g().trim().length()

[olhotak]

@noti0na1 I've finished cleaning this up and it's ready for you to review again.

Best reviewed all at once. Will need to be squashed before merging, but I wanted to maintain the history for review.

[noti0na1]

LGTM overall.

[noti0na1]

@odersky suggested to define FlexableType as a TypeRef with a TypeBounds info, because TypeComparer can handle bounds from TypeRef automatically.

However, I don't think this is a good idea:

• FlexableType should be a type operator which can be applied to any type, so it does not have a name or symbol,
• Creating a special designator and denotation just for this use case can be difficult.

[noti0na1]

It would be better if we can change the definition of hi of FlexableType(T) from T to T.stripNulls.

This can be useful if we are passing a nullable type to Java, for example, we may encounter FlexableType(String | Null) . With the current definition, FlexableType(String | Null) !<:< String.

cc @olhotak

[olhotak]

@odersky suggested to define FlexableType as a TypeRef with a TypeBounds info, because TypeComparer can handle bounds from TypeRef automatically.

However, I don't think this is a good idea:

• FlexableType should be a type operator which can be applied to any type, so it does not have a name or symbol,
• Creating a special designator and denotation just for this use case can be difficult.

In many places a TypeRef is expected to have a prefix and symbol. That seems to be the biggest obstacle.

However, a FlexibleType could be a proxy type with a TypeBounds as underlying, in the same way that a TypeRef has a TypeBounds as underlying. The benefit is removing various special cases for FlexibleType. The downside is the danger of missing some places where FlexibleType should be handled specially.

[olhotak]

It would be better if we can change the definition of hi of FlexableType(T) from T to T.stripNulls.

This can be useful if we are passing a nullable type to Java, for example, we may encounter FlexableType(String | Null) . With the current definition, FlexableType(String | Null) !<:< String.

cc @olhotak

Yes, we could. I thought it was unnecessary because we only apply FlexibleType to Java types, which don't have | Null, but you're right that we also apply it to type variables that could be instantiated with a nullble type.

Would this solve the currently failing interop-match-src test?

[odersky]

What I would try is to do something like `narrow` in Types.scala /** A prefix-less refined this or a termRef to a new skolem symbol * that has the given type as info. */ def narrow(using Context): TermRef = TermRef(NoPrefix, newSkolem(this)) Narrow creates a TermRef with a fresh skolem symbol. Likewise we could define a flexible type to be a fresh typeref with the two bounds. We should however be careful to re-use the same flexible type if we encounter the same bounds (which should be often). This can be done either by relying on the type hashing function and treating flexible types as cached types or "by-hand" by adding a one field flexible type cache to each TypeBounds type. TypeBoudns are hash consed, so this would do the trick.

…

On Thu, Sep 21, 2023 at 5:17 PM Ondřej Lhoták ***@***.***> wrote: It would be better if we can change the definition of hi of FlexableType(T) from T to T.stripNulls. This can be useful if we are passing a nullable type to Java, for example, we may encounter FlexableType(String | Null) . With the current definition, FlexableType(String | Null) !<:< String. cc @olhotak <https://github.com/olhotak> Yes, we could. I thought it was unnecessary because we only apply FlexibleType to Java types, which don't have | Null, but you're right that we also apply it to type variables that could be instantiated with a nullble type. Would this solve the currently failing interop-match-src test? — Reply to this email directly, view it on GitHub <#18112 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAGCKVSKJD27EFGTWEZEXBDX3RLCFANCNFSM6AAAAAAZ2GDIDA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Martin Odersky Professor, Programming Methods Group (LAMP) Faculty IC, EPFL Station 14, Lausanne, Switzerland

[noti0na1]

@odersky I have tried the TypeRef approach, but it doesn't work very well.

Although creating a simple flexiable type is easy and its subtyping works automatically, we still need to add special cases to many places. For example in TypeMap, a flexiable type does not behaviour like a regular TypeRef, we need to bypass the cases for NamedType and TypeBound, and map the unbderlying type. What's more, extracting the underlying types and creating new TypeRefs are expensive.

Therefore, I think it is better to just add a new type for flexiable types.

[noti0na1]

@olhotak
The bug is introduced at the change to JavaNullInterop in 58818e9.

outermostLevelAlreadyNullable = tp.classSymbol.is(JavaDefined) && !ctx.flexibleTypes

This causes adding extra flexible types to type arguments of Java-defined types.

public class Jtest<T> {
  public Jtest<T> j = this;
}

def test[T](x: Jtest[T]) =
  val y = x.j

The correct type for y should be FlexibleType(Jtest[T]) not FlexibleType(Jtest[FlexibleType(T)]).

The error in tests/flexible-types/pos/sam-parameter-javadefined/sam-test.scala is not fixed by this either.

[olhotak]

@olhotak The bug is introduced at the change to JavaNullInterop in 58818e9.

outermostLevelAlreadyNullable = tp.classSymbol.is(JavaDefined) && !ctx.flexibleTypes

This causes adding extra flexible types to type arguments of Java-defined types.

public class Jtest<T> {
  public Jtest<T> j = this;
}

def test[T](x: Jtest[T]) =
  val y = x.j

The correct type for y should be FlexibleType(Jtest[T]) not FlexibleType(Jtest[FlexibleType(T)]).

The error in tests/flexible-types/pos/sam-parameter-javadefined/sam-test.scala is not fixed by this either.

sam-test.scala has a call to the handle method of CompletableFuture, a JavaDefined class.

The Java signature of the handle method is:

<U> CompletableFuture<U> handle(BiFunction<? super T,Throwable,? extends U> fn)

If we don't make Throwable into FlexibleType(Throwable) in the above signature, then the function fn that is passed to the handle method expects a non-nullable Throwable and we can't pass null to it. Thus my fix was to make the type argument a flexible type as well, since it's part of the JavaDefined signature.

[noti0na1]

Fixed a bug related to widenUnion, probably due to the new mergerLub.

[Gedochao]

This has been decided to be included in the 3.5.0 release.

[odersky]

Can you please rebase over main instead of merging? With the merge in the history I am unable to review.

What I try to do is squash all commits of flexible types because with the individual commits its impossible to see what remains to be done. But a merge in the middle makes that impossible.

In fact I think it might be best if all flexible types commits were squashed into one. I don't see any increments that we want to handle separately.

[odersky]

Thanks for the squash. LGTM!

Let's just to a performance test to make sure we don't have slowdowns.

[odersky]

test performance please

[mbovel]

test performance please

[dottybot]

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

[dottybot]

Performance test finished successfully:

Visit https://dotty-bench.epfl.ch/18112/ to see the changes.

Benchmarks is based on merging with main (447cdf4)

[nicolasstucki]

There is a small thing missing.

  def isLegalTag(tag: Int): Boolean =
    firstSimpleTreeTag <= tag && tag <= SPLITCLAUSE ||
    firstNatTreeTag <= tag && tag <= RENAMED ||
    firstASTTreeTag <= tag && tag <= BOUNDED ||
    firstNatASTTreeTag <= tag && tag <= NAMEDARG ||
-   firstLengthTreeTag <= tag && tag <= MATCHCASEtype ||
+   firstLengthTreeTag <= tag && tag <= FLEXIBLEtype ||
    tag == HOLE

astTagToString also needs to be updated. Maybe other methods around there also need to be change.

[noti0na1]

@olhotak The added experimental function fromNullable causes cyclic references with the new updates of the experimental annotation.
We plan to move it to a separate PR to avoid the blocking of this PR.

[olhotak]

@olhotak The added experimental function fromNullable causes cyclic references with the new updates of the experimental annotation. We plan to move it to a separate PR to avoid the blocking of this PR.

OK, that's fine. It is needed to port existing code (e.g. in the community build) that currently uses Option.apply, but it can be independent of the main flexible types PR.