More consistant results in pickling

Author: noti0na1

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

@@ -2141,7 +2141,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
    *  @note  We do not admit singleton types in or-types as lubs.
    */
   def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = /*>|>*/ trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true) /*<|<*/ {
-    if (tp1 eq tp2) tp1
+    if tp1 eq tp2 then tp1
     else if !tp1.exists || (tp2 eq WildcardType) then tp1
     else if !tp2.exists || (tp1 eq WildcardType) then tp2
     else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp1
@@ -2157,16 +2157,19 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
                 if (hi1 & hi2).isEmpty then return orType(tp1, tp2)
               case none =>
           case none =>
+
+        // Simplifying the super type is important to avoid
+        // inconsistant result in union type.
         val t1 = mergeIfSuper(tp1, tp2, canConstrain)
-        if (t1.exists) return t1
+        if t1.exists then return t1.simplified
 
         val t2 = mergeIfSuper(tp2, tp1, canConstrain)
-        if (t2.exists) return t2
+        if t2.exists then return t2.simplified
 
-        def widen(tp: Type) = if (widenInUnions) tp.widen else tp.widenIfUnstable
+        def widen(tp: Type) = if widenInUnions then tp.widen else tp.widenIfUnstable
         val tp1w = widen(tp1)
         val tp2w = widen(tp2)
-        if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft)
+        if (tp1 ne tp1w) || (tp2 ne tp2w) then lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft)
         else orType(tp1w, tp2w, isSoft = isSoft) // no need to check subtypes again
       }
       mergedLub(tp1.stripLazyRef, tp2.stripLazyRef)

compiler/src/dotty/tools/dotc/core/tasty/TreeUnpickler.scala

@@ -1194,10 +1194,6 @@ class TreeUnpickler(reader: TastyReader,
                 res.withAttachment(SuppressedApplyToNone, ())
               else res
 
-      def simplifyLub(tree: Tree): Tree =
-        tree.overwriteType(tree.tpe.simplified)
-        tree
-
       def readLengthTerm(): Tree = {
         val end = readEnd()
         val result =
@@ -1247,25 +1243,23 @@ class TreeUnpickler(reader: TastyReader,
               val tpt = ifBefore(end)(readTpt(), EmptyTree)
               Closure(Nil, meth, tpt)
             case MATCH =>
-              simplifyLub(
-                if (nextByte == IMPLICIT) {
-                  readByte()
-                  InlineMatch(EmptyTree, readCases(end))
-                }
-                else if (nextByte == INLINE) {
-                  readByte()
-                  InlineMatch(readTerm(), readCases(end))
-                }
-                else Match(readTerm(), readCases(end)))
+              if (nextByte == IMPLICIT) {
+                readByte()
+                InlineMatch(EmptyTree, readCases(end))
+              }
+              else if (nextByte == INLINE) {
+                readByte()
+                InlineMatch(readTerm(), readCases(end))
+              }
+              else Match(readTerm(), readCases(end))
             case RETURN =>
               val from = readSymRef()
               val expr = ifBefore(end)(readTerm(), EmptyTree)
               Return(expr, Ident(from.termRef))
             case WHILE =>
               WhileDo(readTerm(), readTerm())
             case TRY =>
-              simplifyLub(
-                Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree)))
+              Try(readTerm(), readCases(end), ifBefore(end)(readTerm(), EmptyTree))
             case SELECTouter =>
               val levels = readNat()
               readTerm().outerSelect(levels, SkolemType(readType()))

compiler/src/dotty/tools/dotc/typer/Nullables.scala

@@ -28,7 +28,7 @@ object Nullables:
     ctx.explicitNulls && !ctx.mode.is(Mode.SafeNulls)
 
   private def needNullifyHi(lo: Type, hi: Type)(using Context): Boolean =
-    ctx.explicitNulls
+    ctx.mode.is(Mode.SafeNulls)
     && lo.isExactlyNull // only nullify hi if lo is exactly Null type
     && hi.isValueType
     // We cannot check if hi is nullable, because it can cause cyclic reference.

compiler/test/dotty/tools/dotc/CompilationTests.scala

@@ -248,9 +248,9 @@ class CompilationTests {
       compileFilesInDir("tests/explicit-nulls/pos", explicitNullsOptions),
       compileFilesInDir("tests/explicit-nulls/pos-separate", explicitNullsOptions),
       compileFilesInDir("tests/explicit-nulls/pos-patmat", explicitNullsOptions and "-Xfatal-warnings"),
+      compileFilesInDir("tests/explicit-nulls/pos-pickling", explicitNullsOptions and "-Ytest-pickler" and "-Xprint-types"),
       compileFilesInDir("tests/explicit-nulls/unsafe-common", explicitNullsOptions and "-language:unsafeNulls"),
       compileFile("tests/explicit-nulls/pos-special/i14682.scala", explicitNullsOptions and "-Ysafe-init"),
-      compileFile("tests/explicit-nulls/pos-special/i14947.scala", explicitNullsOptions and "-Ytest-pickler" and "-Xprint-types"),
     )
   }.checkCompile()
 

tests/explicit-nulls/pos-special/i14947.scala renamed to tests/explicit-nulls/pos-pickling/i14947.scala

@@ -10,6 +10,7 @@ class C:
     if ??? then g else ""
 
   def f3 =
+    import scala.language.unsafeNulls
     (??? : Boolean) match
     case true => g
     case _ => ""

tests/pos/i15097.scala renamed to tests/explicit-nulls/pos-pickling/i15097.scala

@@ -0,0 +1,59 @@
+import scala.language.unsafeNulls
+
+def a(): String | Null = ???
+val b: String | Null = ???
+
+val i: Int = ???
+
+def f1 = i match {
+  case 0 => b
+  case _ => a()
+}
+
+def f2 = i match {
+  case 0 => a()
+  case _ => b
+}
+
+def f3 = i match {
+  case 0 => a()
+  case _ => "".trim
+}
+
+def f4 = i match {
+  case 0 => b
+  case _ => "".trim
+}
+
+def g1 = i match {
+  case 0 => a()
+  case 1 => ""
+  case _ => null
+}
+
+def g2 = i match {
+  case 0 => ""
+  case 1 => null
+  case _ => b
+}
+
+def g3 = i match {
+  case 0 => null
+  case 1 => b
+  case _ => ""
+}
+
+def h1(i: Int) = i match
+  case 0 => 0
+  case 1 => true
+  case 2 => i.toString
+  case _ => null
+
+// This test still fails.
+// Even without explicit nulls, the type of Match
+// is (0, true, "2"), which is wrong.
+// def h2(i: Int) = i match
+//   case 0 => 0
+//   case 1 => true
+//   case 2 => "2"
+//   case _ => null

tests/explicit-nulls/pos-pickling/match-case.scala

@@ -0,0 +1,35 @@
+import scala.language.unsafeNulls
+
+def associatedFile: String | Null = ???
+
+def source: String = {
+  val f = associatedFile
+  if (f != null) f else associatedFile
+}
+
+def defines(raw: String): List[String] = {
+  val ds: List[(Int, Int)] = ???
+  ds map { case (start, end) => raw.substring(start, end) }
+}
+
+abstract class DeconstructorCommon[T >: Null <: AnyRef] {
+  var field: T = null
+  def get: this.type = this
+  def isEmpty: Boolean = field eq null
+  def isDefined = !isEmpty
+  def unapply(s: T): this.type ={
+    field = s
+    this
+  }
+}
+
+def genBCode =
+  val bsmArgs: Array[Object | Null] | Null = null
+  val implMethod = bsmArgs(3).asInstanceOf[Integer].toInt
+  implMethod
+
+val arrayApply = "a".split(" ")(2)
+
+val globdir: String = if (??? : Boolean) then associatedFile.replaceAll("[\\\\/][^\\\\/]*$", "") else ""
+
+def newInstOfC(c: Class[?]) = c.getConstructor().newInstance()

tests/explicit-nulls/pos-pickling/other-pickling.scala

@@ -0,0 +1,27 @@
+import scala.language.unsafeNulls
+
+
+def s: String | Null = ???
+
+def tryString = try s catch {
+  case _: NullPointerException => null
+  case _ => ""
+}
+
+def tryString2 = try s catch {
+  case _: NullPointerException => ""
+  case _ => s
+}
+
+def loadClass(classLoader: ClassLoader, name: String): Class[?] =
+  try classLoader.loadClass(name)
+  catch {
+    case _ =>
+      throw new Exception()
+  }
+
+def loadClass2(classLoader: ClassLoader, name: String): Class[?] =
+  try classLoader.loadClass(name)
+  catch {
+    case _ => null
+  }