Only nullify tasty if Yflexify-tasty is set; Refine FlexibleType nullification rules

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

@@ -1991,13 +1991,11 @@ class Definitions {
    *   - the upper bound of a TypeParamRef in the current constraint
    */
   def asContextFunctionType(tp: Type)(using Context): Type =
-    tp.stripTypeVar.dealias match
+    tp.stripNull().stripTypeVar.dealias match
       case tp1: TypeParamRef if ctx.typerState.constraint.contains(tp1) =>
         asContextFunctionType(TypeComparer.bounds(tp1).hiBound)
       case tp1 @ PolyFunctionOf(mt: MethodType) if mt.isContextualMethod =>
         tp1
-      case tp1: FlexibleType =>
-        asContextFunctionType(tp1.underlying)
       case tp1 =>
         if tp1.typeSymbol.name.isContextFunction && isFunctionNType(tp1) then tp1
         else NoType

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

@@ -2,19 +2,25 @@ package dotty.tools.dotc
 package core
 
 import Contexts.*
-import Flags.JavaDefined
+import Flags.*
 import StdNames.nme
 import Symbols.*
 import Types.*
-import dotty.tools.dotc.reporting.*
-import dotty.tools.dotc.core.Decorators.i
+import Decorators.i
+import reporting.*
 
-/** This module defines methods to interpret types of Java symbols, which are implicitly nullable in Java,
- *  as Scala types, which are explicitly nullable.
+/** This module defines methods to interpret types originating from sources without explicit nulls
+ *  (Java, and Scala code compiled without `-Yexplicit-nulls`) as Scala types with explicit nulls.
+ *  In those sources, reference types are implicitly nullable; here we make that nullability explicit.
+ *
+ *  e.g. given a Java method: `String foo(String arg) { return arg; }`
+ *
+ *  After calling `nullifyMember`, Scala will see the method as:
+ *  `def foo(arg: String | Null): String | Null`
  *
  *  The transformation is (conceptually) a function `n` that adheres to the following rules:
  *    (1) n(T)              = T | Null              if T is a reference type
- *    (2) n(T)              = T                       if T is a value type
+ *    (2) n(T)              = T                     if T is a value type
  *    (3) n(C[T])           = C[T] | Null           if C is Java-defined
  *    (4) n(C[T])           = C[n(T)] | Null        if C is Scala-defined
  *    (5) n(A|B)            = n(A) | n(B) | Null
@@ -29,142 +35,187 @@ import dotty.tools.dotc.core.Decorators.i
  *       e.g. calling `get` on a `java.util.List[String]` already returns `String|Null` and not `String`, so
  *       we don't need to write `java.util.List[String | Null]`.
  *     - if `C` is Scala-defined, however, then we want `n(C[T]) = C[n(T)] | Null`. This is because
- *       `C` won't be nullified, so we need to indicate that its type argument is nullable.
+ *       Scala-defined classes are not implicitly nullified inside their bodies, so we need to indicate that
+ *       their type arguments are nullable when the defining source did not use explicit nulls.
+ *
+ *  Why not use subtyping to nullify “exactly”?
+ *  -------------------------------------------------
+ *  The symbols we nullify here are often still under construction (e.g. during classfile loading or unpickling),
+ *  so we don't always have precise or stable type information available. Using full subtyping checks to determine
+ *  which parts are reference types would either force types prematurely or risk cyclic initializations. Therefore,
+ *  we use a conservative approach that targets concrete reference types  without depending on precise subtype
+ *  information.
+ *
+ *  Scope and limitations
+ *  -------------------------------------------------
+ *  The transformation is applied to types attached to members coming from Java and from Scala code compiled without
+ *  explicit nulls. The implementation is intentionally conservative and does not attempt to cover the full spectrum
+ *  of Scala types. In particular, we do not nullify type parameters or some complex type forms (e.g., match types,
+ *  or refined types) beyond straightforward mapping; in such cases we typically recurse only into obviously safe
+ *  positions or leave the type unchanged.
  *
- *   Notice that since the transformation is only applied to types attached to Java symbols, it doesn't need
- *   to handle the full spectrum of Scala types. Additionally, some kinds of symbols like constructors and
- *   enum instances get special treatment.
+ *  Additionally, some kinds of symbols like constructors and enum instances get special treatment.
  */
-object ImplicitNullInterop {
+object ImplicitNullInterop:
 
-  /** Transforms the type `tp` of Java member `sym` to be explicitly nullable.
-   *  `tp` is needed because the type inside `sym` might not be set when this method is called.
-   *
-   *  e.g. given a Java method
-   *  String foo(String arg) { return arg; }
-   *
-   *  After calling `nullifyMember`, Scala will see the method as
-   *
-   *  def foo(arg: String | Null): String | Null
-   *
-   *  If unsafeNulls is enabled, we can select on the return of `foo`:
-   *
-   *  val len = foo("hello").length
-   *
-   *  But the selection can throw an NPE if the returned value is `null`.
+  /** Transforms the type `tp` of a member `sym` that originates from a source without explicit nulls.
+   *  `tp` is passed explicitly because the type stored in `sym` might not yet be set when this is called.
    */
-  def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = trace(i"nullifyMember ${sym}, ${tp}"){
+  def nullifyMember(sym: Symbol, tp: Type, isEnumValueDef: Boolean)(using Context): Type = trace(i"nullifyMember ${sym}, ${tp}"):
     assert(ctx.explicitNulls)
 
-    // Some special cases when nullifying the type
-    if isEnumValueDef || sym.name == nme.TYPE_ // Don't nullify the `TYPE` field in every class and Java enum instances
-    || sym.is(Flags.ModuleVal) // Don't nullify Modules
-    then
-      tp
-    else if sym.name == nme.toString_ || sym.isConstructor || hasNotNullAnnot(sym) then
-      // Don't nullify the return type of the `toString` method.
-      // Don't nullify the return type of constructors.
-      // Don't nullify the return type of methods with a not-null annotation.
-      nullifyExceptReturnType(tp)
-    else
-      // Otherwise, nullify everything
-      nullifyType(tp)
-  }
+    // Skip `TYPE`, enum values, and modules
+    if isEnumValueDef
+      || sym.name == nme.TYPE_
+      || sym.name == nme.getClass_
+      || sym.name == nme.toString_
+      || sym.is(Flags.ModuleVal) then
+      return tp
 
-  private def hasNotNullAnnot(sym: Symbol)(using Context): Boolean =
-    ctx.definitions.NotNullAnnots.exists(nna => sym.unforcedAnnotation(nna).isDefined)
+    // Don't nullify result type for `toString`, constructors, and @NotNull methods
+    val skipResultType = sym.isConstructor || hasNotNullAnnot(sym)
+    // Don't nullify Given/implicit parameters
+    val skipCurrentLevel = sym.isOneOf(GivenOrImplicitVal)
 
-  /** If tp is a MethodType, the parameters and the inside of return type are nullified,
-   *  but the result return type is not nullable.
-   *  If tp is a type of a field, the inside of the type is nullified,
-   *  but the result type is not nullable.
-   */
-  private def nullifyExceptReturnType(tp: Type)(using Context): Type =
-    new ImplicitNullMap(outermostLevelAlreadyNullable = true)(tp)
+    val map = new ImplicitNullMap(
+      javaDefined = sym.is(JavaDefined),
+      skipResultType = skipResultType,
+      skipCurrentLevel = skipCurrentLevel)
+    map(tp)
 
-  /** Nullifies a type by adding `| Null` in the relevant places. */
-  private def nullifyType(tp: Type)(using Context): Type =
-    new ImplicitNullMap(outermostLevelAlreadyNullable = false)(tp)
+  private def hasNotNullAnnot(sym: Symbol)(using Context): Boolean =
+    ctx.definitions.NotNullAnnots.exists(nna => sym.unforcedAnnotation(nna).isDefined)
 
-  /** A type map that implements the nullification function on types. Given a Java-sourced type or an
-   *  implicitly null type, this adds `| Null` in the right places to make the nulls explicit.
+  /** A type map that implements the nullification function on types. Given a Java-sourced type or a type
+   *  coming from Scala code compiled without explicit nulls, this adds `| Null` or `FlexibleType` in the
+   *  right places to make nullability explicit in a conservative way (without forcing incomplete symbols).
    *
-   *  @param outermostLevelAlreadyNullable whether this type is already nullable at the outermost level.
-   *                                       For example, `Array[String] | Null` is already nullable at the
-   *                                       outermost level, but `Array[String | Null]` isn't.
-   *                                       If this parameter is set to true, then the types of fields, and the return
-   *                                       types of methods will not be nullified.
-   *                                       This is useful for e.g. constructors, and also so that `A & B` is nullified
-   *                                       to `(A & B) | Null`, instead of `(A | Null & B | Null) | Null`.
+   *  @param javaDefined        whether the type is from Java source, we always nullify type param refs from Java
+   *  @param skipResultType     do not nullify the method result type at the outermost level (e.g. for `toString`,
+   *                            constructors, or methods annotated as not-null)
+   *  @param skipCurrentLevel   do not nullify at the current level (used for implicit/Given parameters, varargs, etc.)
    */
-  private class ImplicitNullMap(var outermostLevelAlreadyNullable: Boolean)(using Context) extends TypeMap {
+  private class ImplicitNullMap(
+      val javaDefined: Boolean,
+      var skipResultType: Boolean = false,
+      var skipCurrentLevel: Boolean = false
+    )(using Context) extends TypeMap:
+
     def nullify(tp: Type): Type = if ctx.flexibleTypes then FlexibleType(tp) else OrNull(tp)
 
-    /** Should we nullify `tp` at the outermost level? */
+    /** Should we nullify `tp` at the outermost level?
+     *  The symbols are still under construction, so we don't have precise information.
+     *  We purposely do not rely on precise subtyping checks here (e.g., asking whether `tp <:< AnyRef`),
+     *  because doing so could force incomplete symbols or trigger cycles. Instead, we conservatively
+     *  nullify only when we can recognize a concrete reference type or type parameters from Java.
+     */
     def needsNull(tp: Type): Boolean =
-      if outermostLevelAlreadyNullable then false
-      else tp match
-        case tp: TypeRef if !tp.hasSimpleKind
+      if skipCurrentLevel || !tp.hasSimpleKind then false
+      else tp.dealias match
+        case tp: TypeRef =>
           // We don't modify value types because they're non-nullable even in Java.
-          || tp.symbol.isValueClass
-          // We don't modify unit types.
-          || tp.isRef(defn.UnitClass)
-          // We don't modify `Any` because it's already nullable.
-          || tp.isRef(defn.AnyClass) => false
-        case _ => true
+          val isValueOrSpecialClass =
+            tp.symbol.isValueClass
+            || tp.isRef(defn.NullClass)
+            || tp.isRef(defn.NothingClass)
+            || tp.isRef(defn.UnitClass)
+            || tp.isRef(defn.SingletonClass)
+            || tp.isRef(defn.AnyKindClass)
+            || tp.isRef(defn.AnyClass)
+          !isValueOrSpecialClass && (javaDefined || tp.symbol.isNullableClassAfterErasure)
+        case tp: TypeParamRef =>
+          javaDefined
+        case _ => false
 
-    // We don't nullify Java varargs at the top level.
+    // We don't nullify varargs (repeated parameters) at the top level.
     // Example: if `setNames` is a Java method with signature `void setNames(String... names)`,
     // then its Scala signature will be `def setNames(names: (String|Null)*): Unit`.
     // This is because `setNames(null)` passes as argument a single-element array containing the value `null`,
     // and not a `null` array.
     def tyconNeedsNull(tp: Type): Boolean =
-      if outermostLevelAlreadyNullable then false
+      if skipCurrentLevel then false
       else tp match
         case tp: TypeRef
           if !ctx.flexibleTypes && tp.isRef(defn.RepeatedParamClass) => false
         case _ => true
 
-    override def apply(tp: Type): Type = tp match {
-      case tp: TypeRef if needsNull(tp) => nullify(tp)
+    override def apply(tp: Type): Type = tp match
+      case tp: TypeRef if needsNull(tp) =>
+        nullify(tp)
+      case tp: TypeParamRef if needsNull(tp) =>
+        nullify(tp)
       case appTp @ AppliedType(tycon, targs) =>
-        val oldOutermostNullable = outermostLevelAlreadyNullable
-        // We don't make the outmost levels of type arguments nullable if tycon is Java-defined.
-        // This is because Java classes are _all_ nullified, so both `java.util.List[String]` and
-        // `java.util.List[String|Null]` contain nullable elements.
-        outermostLevelAlreadyNullable = tp.classSymbol.is(JavaDefined)
-        val targs2 = targs map this
-        outermostLevelAlreadyNullable = oldOutermostNullable
+        val savedSkipCurrentLevel = skipCurrentLevel
+
+        // If Java-defined tycon, don't nullify outer level of type args (Java classes are fully nullified)
+        skipCurrentLevel = tp.classSymbol.is(JavaDefined)
+        val targs2 = targs.map(this)
+
+        skipCurrentLevel = savedSkipCurrentLevel
         val appTp2 = derivedAppliedType(appTp, tycon, targs2)
-        if tyconNeedsNull(tycon) then nullify(appTp2) else appTp2
+        if tyconNeedsNull(tycon) && tp.hasSimpleKind then nullify(appTp2) else appTp2
       case ptp: PolyType =>
         derivedLambdaType(ptp)(ptp.paramInfos, this(ptp.resType))
       case mtp: MethodType =>
-        val oldOutermostNullable = outermostLevelAlreadyNullable
-        outermostLevelAlreadyNullable = false
-        val paramInfos2 = mtp.paramInfos map this
-        outermostLevelAlreadyNullable = oldOutermostNullable
-        derivedLambdaType(mtp)(paramInfos2, this(mtp.resType))
-      case tp: TypeAlias => mapOver(tp)
-      case tp: TypeBounds => mapOver(tp)
-      case tp: AndType =>
-        // nullify(A & B) = (nullify(A) & nullify(B)) | Null, but take care not to add
-        // duplicate `Null`s at the outermost level inside `A` and `B`.
-        outermostLevelAlreadyNullable = true
-        nullify(derivedAndType(tp, this(tp.tp1), this(tp.tp2)))
-      case tp: TypeParamRef if needsNull(tp) => nullify(tp)
-      // In all other cases, return the type unchanged.
-      // In particular, if the type is a ConstantType, then we don't nullify it because it is the
-      // type of a final non-nullable field.
-      case tp: ExprType => mapOver(tp)
-      case tp: AnnotatedType => mapOver(tp)
-      case tp: OrType =>
-        outermostLevelAlreadyNullable = true
-        nullify(derivedOrType(tp, this(tp.tp1), this(tp.tp2)))
+        val savedSkipCurrentLevel = skipCurrentLevel
+
+        // Don't nullify param types for implicit/using sections
+        skipCurrentLevel = mtp.isImplicitMethod
+        val paramInfos2 = mtp.paramInfos.map(this)
+
+        skipCurrentLevel = skipResultType
+        val resType2 = this(mtp.resType)
+
+        skipCurrentLevel = savedSkipCurrentLevel
+        derivedLambdaType(mtp)(paramInfos2, resType2)
+      case tp: TypeAlias =>
+        mapOver(tp)
+      case tp: TypeBounds =>
+        mapOver(tp)
+      case tp: AndOrType =>
+        // For unions/intersections we recurse into both sides.
+        // If both sides are nullalble, we only add `| Null` once.
+        // This keeps the result minimal and avoids duplicating `| Null`
+        // on both sides and at the outer level.
+        (this(tp.tp1), this(tp.tp2)) match
+          case (FlexibleType(_, t1), FlexibleType(_, t2)) if ctx.flexibleTypes =>
+            FlexibleType(derivedAndOrType(tp, t1, t2))
+          case (OrNull(t1), OrNull(t2)) =>
+            OrNull(derivedAndOrType(tp, t1, t2))
+          case (t1, t2) =>
+            derivedAndOrType(tp, t1, t2)
+      case tp: ExprType =>
+        mapOver(tp)
+      case tp: AnnotatedType =>
+        // We don't nullify the annotation part.
+        derivedAnnotatedType(tp, this(tp.underlying), tp.annot)
       case tp: RefinedType =>
-        outermostLevelAlreadyNullable = true
-        nullify(mapOver(tp))
-      case _ => tp
-    }
-  }
-}
+        val savedSkipCurrentLevel = skipCurrentLevel
+        val savedSkipResultType = skipResultType
+
+        skipCurrentLevel = true
+        val parent2 = this(tp.parent)
+
+        skipCurrentLevel = false
+        skipResultType = false
+        val refinedInfo2 = this(tp.refinedInfo)
+
+        skipCurrentLevel = savedSkipCurrentLevel
+        skipResultType = savedSkipResultType
+
+        parent2 match
+          case FlexibleType(_, parent2a) if ctx.flexibleTypes =>
+            FlexibleType(derivedRefinedType(tp, parent2a, refinedInfo2))
+          case OrNull(parent2a) =>
+            OrNull(derivedRefinedType(tp, parent2a, refinedInfo2))
+          case _ =>
+            derivedRefinedType(tp, parent2, refinedInfo2)
+      case _ =>
+        // In all other cases, return the type unchanged.
+        // In particular, if the type is a ConstantType, then we don't nullify it because it is the
+        // type of a final non-nullable field. We also deliberately do not attempt to nullify
+        // complex computed types such as match types here; those remain as-is to avoid forcing
+        // incomplete information during symbol construction.
+        tp
+    end apply
+  end ImplicitNullMap

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

@@ -6253,6 +6253,8 @@ object Types extends TypeUtils {
       tp.derivedAndType(tp1, tp2)
     protected def derivedOrType(tp: OrType, tp1: Type, tp2: Type): Type =
       tp.derivedOrType(tp1, tp2)
+    protected def derivedAndOrType(tp: AndOrType, tp1: Type, tp2: Type): Type =
+      tp.derivedAndOrType(tp1, tp2)
     protected def derivedMatchType(tp: MatchType, bound: Type, scrutinee: Type, cases: List[Type]): Type =
       tp.derivedMatchType(bound, scrutinee, cases)
     protected def derivedAnnotatedType(tp: AnnotatedType, underlying: Type, annot: Annotation): Type =

tests/explicit-nulls/flexible-unpickle/Flexible_2.scala

@@ -1,6 +1,6 @@
 import unsafeNulls.Foo.*
 import unsafeNulls.Unsafe_1
-import unsafeNulls.{A, B, C, F, G, H, I, J, L, M, S, T, U, expects}
+import unsafeNulls.{A, B, C, F, G, H, I, J, L, M, N, S, T, U, expects}
 import scala.reflect.Selectable.reflectiveSelectable
 import scala.quoted.*
 
@@ -100,6 +100,11 @@ def Flexible_2() =
 
   val m: String = M.test(null)
 
+  // i23911
+  val n1: List[Map[String, Int]] = ???
+  val n2 = new N[List]()
+  val n3 = n2.accept[Any](n1)
+
   // i23845
   transparent inline def typeName[A]: String = ${typeNameMacro[A]}
 
@@ -109,3 +114,4 @@ def Flexible_2() =
     implicit val givenS: S[A] = ???
     expects(alphaTypeNameMacro[A])
   }
+

tests/explicit-nulls/flexible-unpickle/Unsafe_1.scala

@@ -73,6 +73,11 @@ object M {
   def test(input: => String): String = "foo " + input
 }
 
+
+class N[F[_]] {
+  def accept[A](arg: F[A]): Nothing = ???
+}
+
 class S[X]
 object S {  def show[X] = "dummyStr" }
 class T