Merge pull request #13647 from dotty-staging/explicit-nulls-override

Fix overriding in explicit nulls

Author: smarter

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

@@ -301,6 +301,11 @@ object Types {
 
     def isFromJavaObject(using Context): Boolean = typeSymbol eq defn.FromJavaObjectSymbol
 
+    def containsFromJavaObject(using Context): Boolean = this match
+      case tp: OrType => tp.tp1.containsFromJavaObject || tp.tp2.containsFromJavaObject
+      case tp: AndType => tp.tp1.containsFromJavaObject && tp.tp2.containsFromJavaObject
+      case _ => isFromJavaObject
+
     /** True iff `symd` is a denotation of a class type parameter and the reference
      *  `<pre> . <symd>` is an actual argument reference, i.e. `pre` is not the
      *  ThisType of `symd`'s owner, or a reference to `symd`'s owner.'
@@ -4933,8 +4938,23 @@ object Types {
     }
 
     def & (that: TypeBounds)(using Context): TypeBounds =
-      if ((this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi)) that
-      else if ((that.lo frozen_<:< this.lo) && (this.hi frozen_<:< that.hi)) this
+      // This will try to preserve the FromJavaObjects type in upper bounds.
+      // For example, (? <: FromJavaObjects | Null) & (? <: Any),
+      // we want to get (? <: FromJavaObjects | Null) intead of (? <: Any),
+      // because we may check the result <:< (? <: Object | Null) later.
+      if this.hi.containsFromJavaObject
+        && (this.hi frozen_<:< that.hi)
+        && (that.lo frozen_<:< this.lo) then
+        // FromJavaObject in tp1.hi guarantees tp2.hi <:< tp1.hi
+        // prefer tp1 if FromJavaObject is in its hi
+        this
+      else if that.hi.containsFromJavaObject
+        && (that.hi frozen_<:< this.hi)
+        && (this.lo frozen_<:< that.lo) then
+        // Similarly, prefer tp2 if FromJavaObject is in its hi
+        that
+      else if (this.lo frozen_<:< that.lo) && (that.hi frozen_<:< this.hi) then that
+      else if (that.lo frozen_<:< this.lo) && (this.hi frozen_<:< that.hi) then this
       else TypeBounds(this.lo | that.lo, this.hi & that.hi)
 
     def | (that: TypeBounds)(using Context): TypeBounds =

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

@@ -15,6 +15,7 @@ import Decorators._
 import Denotations._, SymDenotations._
 import TypeErasure.erasure
 import DenotTransformers._
+import NullOpsDecorator._
 
 object ElimRepeated {
   val name: String = "elimRepeated"
@@ -335,6 +336,9 @@ class ElimRepeated extends MiniPhase with InfoTransformer { thisPhase =>
     val array = tp.translateFromRepeated(toArray = true) // Array[? <: T]
     val element = array.elemType.hiBound // T
 
-    if element <:< defn.AnyRefType || element.typeSymbol.isPrimitiveValueClass then array
+
+    if element <:< defn.AnyRefType
+      || ctx.mode.is(Mode.SafeNulls) && element.stripNull <:< defn.AnyRefType
+      || element.typeSymbol.isPrimitiveValueClass then array
     else defn.ArrayOf(TypeBounds.upper(AndType(element, defn.AnyRefType))) // Array[? <: T & AnyRef]
 }

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

@@ -107,7 +107,14 @@ object ResolveSuper {
         // of the superaccessor's type, see i5433.scala for an example where this matters
         val otherTp = other.asSeenFrom(base.typeRef).info
         val accTp = acc.asSeenFrom(base.typeRef).info
-        if (!(otherTp.overrides(accTp, matchLoosely = true)))
+        // 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)))
           report.error(IllegalSuperAccessor(base, memberName, targetName, acc, accTp, other.symbol, otherTp), base.srcPos)
 
       bcs = bcs.tail

tests/explicit-nulls/pos/i13040/Impl.scala

@@ -0,0 +1,5 @@
+class Impl extends Intf:
+  override def test(x: Object | Null*): Unit = ???
+
+class Impl2 extends Intf:
+  override def test(x: Object*): Unit = ???

tests/explicit-nulls/pos/i13040/Intf.java

@@ -0,0 +1,3 @@
+interface Intf {
+    void test(Object... x);
+}

tests/explicit-nulls/pos/i13486.scala

@@ -0,0 +1,7 @@
+class MyPrintStream extends java.io.PrintStream(??? : java.io.OutputStream):
+  override def printf(format: String | Null, args: Array[? <: Object | Null])
+  : java.io.PrintStream | Null = ???
+
+class MyPrintStream2 extends java.io.PrintStream(??? : java.io.OutputStream):
+  override def printf(format: String, args: Array[? <: Object])
+  : java.io.PrintStream = ???

tests/explicit-nulls/pos/i13608.scala

@@ -0,0 +1,3 @@
+import scala.util.control.NoStackTrace
+
+case class ParseException(line: Int, character: Int, message: String) extends NoStackTrace