Merge pull request #13975 from noti0na1/explicit-nulls-more-relaxed-override

Fix override containing type param in explicit nulls

Author: noti0na1

compiler/src/dotty/tools/dotc/core/Contexts.scala

@@ -712,6 +712,9 @@ object Contexts {
 
       def withNotNullInfos(infos: List[NotNullInfo]): Context =
         if c.notNullInfos eq infos then c else c.fresh.setNotNullInfos(infos)
+
+      def relaxedOverrideContext: Context =
+        c.withModeBits(c.mode &~ Mode.SafeNulls | Mode.RelaxedOverriding)
   end ops
 
   // TODO: Fix issue when converting ModeChanges and FreshModeChanges to extension givens

compiler/src/dotty/tools/dotc/core/Mode.scala

@@ -124,4 +124,9 @@ object Mode {
    *  This mode forces expansion of inline calls in those positions even during typing.
    */
   val ForceInline: Mode = newMode(29, "ForceInline")
+
+  /** This mode is enabled when we check Java overriding in explicit nulls.
+   *  Type `Null` becomes a subtype of non-primitive value types in TypeComparer.
+   */
+  val RelaxedOverriding: Mode = newMode(30, "RelaxedOverriding")
 }

compiler/src/dotty/tools/dotc/core/TypeComparer.scala

@@ -766,14 +766,24 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
 
             isSubType(hi1, tp2, approx.addLow) || compareGADT || tryLiftedToThis1
           case _ =>
-            def isNullable(tp: Type): Boolean = tp.widenDealias match {
-              case tp: TypeRef => tp.symbol.isNullableClass
+            // `Mode.RelaxedOverriding` is only enabled when checking Java overriding
+            // in explicit nulls, and `Null` becomes a bottom type, which allows
+            // `T | Null` being a subtype of `T`.
+            // A type variable `T` from Java is translated to `T >: Nothing <: Any`.
+            // However, `null` can always be a value of `T` for Java side.
+            // So the best solution here is to let `Null` be a subtype of non-primitive
+            // value types temporarily.
+            def isNullable(tp: Type): Boolean = tp.widenDealias match
+              case tp: TypeRef =>
+                val tpSym = tp.symbol
+                ctx.mode.is(Mode.RelaxedOverriding) && !tpSym.isPrimitiveValueClass ||
+                tpSym.isNullableClass
               case tp: RefinedOrRecType => isNullable(tp.parent)
               case tp: AppliedType => isNullable(tp.tycon)
               case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2)
               case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2)
               case _ => false
-            }
+
             val sym1 = tp1.symbol
             (sym1 eq NothingClass) && tp2.isValueTypeOrLambda ||
             (sym1 eq NullClass) && isNullable(tp2)

compiler/src/dotty/tools/dotc/core/Types.scala

@@ -1112,8 +1112,8 @@ object Types {
      */
     def matches(that: Type)(using Context): Boolean = {
       record("matches")
-      withoutMode(Mode.SafeNulls)(
-        TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes))
+      val overrideCtx = if ctx.explicitNulls then ctx.relaxedOverrideContext else ctx
+      TypeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)(using overrideCtx)
     }
 
     /** This is the same as `matches` except that it also matches => T with T and

compiler/src/dotty/tools/dotc/transform/OverridingPairs.scala

@@ -5,6 +5,7 @@ package transform
 import core._
 import Flags._, Symbols._, Contexts._, Scopes._, Decorators._, Types.Type
 import NameKinds.DefaultGetterName
+import NullOpsDecorator._
 import collection.mutable
 import collection.immutable.BitSet
 import scala.annotation.tailrec
@@ -216,14 +217,12 @@ object OverridingPairs:
       )
     else
       // releaxed override check for explicit nulls if one of the symbols is Java defined,
-      // force `Null` being a subtype of reference types during override checking
-      val relaxedCtxForNulls =
-        if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined)) then
-          ctx.retractMode(Mode.SafeNulls)
-        else ctx
+      // force `Null` to be a subtype of non-primitive value types during override checking.
+      val overrideCtx = if ctx.explicitNulls && (member.is(JavaDefined) || other.is(JavaDefined))
+        then ctx.relaxedOverrideContext else ctx
       member.name.is(DefaultGetterName) // default getters are not checked for compatibility
       || memberTp.overrides(otherTp,
             member.matchNullaryLoosely || other.matchNullaryLoosely || fallBack
-          )(using relaxedCtxForNulls)
+          )(using overrideCtx)
 
 end OverridingPairs

compiler/src/dotty/tools/dotc/transform/ResolveSuper.scala

@@ -13,6 +13,7 @@ import Names._
 import StdNames._
 import NameOps._
 import NameKinds._
+import NullOpsDecorator._
 import ResolveSuper._
 import reporting.IllegalSuperAccessor
 
@@ -111,15 +112,12 @@ object ResolveSuper {
         val otherTp = other.asSeenFrom(base.typeRef).info
         val accTp = acc.asSeenFrom(base.typeRef).info
         // Since the super class can be Java defined,
-        // we use releaxed overriding check for explicit nulls if one of the symbols is Java defined.
-        // This forces `Null` being a subtype of reference types during override checking.
-        val relaxedCtxForNulls =
-        if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined)) then
-          ctx.retractMode(Mode.SafeNulls)
-        else ctx
-        if (!(otherTp.overrides(accTp, matchLoosely = true)(using relaxedCtxForNulls)))
+        // we use relaxed overriding check for explicit nulls if one of the symbols is Java defined.
+        // This forces `Null` to be a subtype of non-primitive value types during override checking.
+        val overrideCtx = if ctx.explicitNulls && (sym.is(JavaDefined) || acc.is(JavaDefined))
+          then ctx.relaxedOverrideContext else ctx
+        if !otherTp.overrides(accTp, matchLoosely = true)(using overrideCtx) then
           report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos)
-
       bcs = bcs.tail
     }
     assert(sym.exists, i"cannot rebind $acc, ${acc.targetName} $memberName")

tests/explicit-nulls/neg/opaque-nullable.scala

@@ -0,0 +1,47 @@
+// Unboxed option type using unions + null + opaque.
+// Relies on the fact that Null is not a subtype of AnyRef.
+// Test suggested by Sébastien Doeraene.
+
+object Nullables {
+  opaque type Nullable[+A <: AnyRef] = A | Null // disjoint by construction!
+
+  object Nullable:
+    def apply[A <: AnyRef](x: A | Null): Nullable[A] = x
+
+    def some[A <: AnyRef](x: A): Nullable[A] = x
+    def none: Nullable[Nothing] = null
+
+  extension [A <: AnyRef](x: Nullable[A])
+    def isEmpty: Boolean = x == null
+    def get: A | Null = x
+
+  extension [A <: AnyRef, B <: AnyRef](x: Nullable[A])
+    def flatMap(f: A => Nullable[B]): Nullable[B] =
+      if (x == null) null
+      else f(x)
+
+    def map(f: A => B): Nullable[B] = x.flatMap(f)
+
+  def test1 =
+    val s1: Nullable[String] = Nullable("hello")
+    val s2: Nullable[String] = "world"
+    val s3: Nullable[String] = Nullable.none
+    val s4: Nullable[String] = null
+
+    s1.isEmpty
+    s1.flatMap((x) => true)
+
+    assert(s2 != null)
+}
+
+def test2 =
+  import Nullables._
+
+  val s1: Nullable[String] = Nullable("hello")
+  val s2: Nullable[String] = Nullable.none
+  val s3: Nullable[String] = null // error: don't leak nullable union
+
+  s1.isEmpty
+  s1.flatMap((x) => Nullable(true))
+
+  assert(s2 == null) // error

tests/explicit-nulls/pos/opaque-nullable.scala

@@ -0,0 +1,18 @@
+// Testing relaxed overriding check for explicit nulls.
+// The relaxed check is only enabled if one of the members is Java defined.
+
+import java.util.Comparator
+
+class C1[T <: AnyRef] extends Ordering[T]:
+  override def compare(o1: T, o2: T): Int = 0
+
+// The following overriding is not allowed, because `compare`
+// has already been declared in Scala class `Ordering`.
+// class C2[T <: AnyRef] extends Ordering[T]:
+//   override def compare(o1: T | Null, o2: T | Null): Int = 0
+
+class D1[T <: AnyRef] extends Comparator[T]:
+  override def compare(o1: T, o2: T): Int = 0
+
+class D2[T <: AnyRef] extends Comparator[T]:
+  override def compare(o1: T | Null, o2: T | Null): Int = 0