Skip to content

Unable to select member of nullable user-defined class #11968

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
noti0na1 opened this issue Apr 1, 2021 · 4 comments · Fixed by #11979
Closed

Unable to select member of nullable user-defined class #11968

noti0na1 opened this issue Apr 1, 2021 · 4 comments · Fixed by #11979

Comments

@noti0na1
Copy link
Member

noti0na1 commented Apr 1, 2021

scalac Stest.scala
class C {
    def get(): Int = 0
}

def g = {
    val s: String | Null = ???
    val l = s.length // ok
    val c: C | Null = ???
    c.get() // error: value get is not a member of C | Null
}

Output

-- [E008] Not Found Error: Stest.scala:13:6 ------------------------------------
13 |    c.get()
   |    ^^^^^
   |    value get is not a member of C | Null

The nullable C is expected to have similar behaviour as nullable String.

I think the issue is at TypeOps.orDominator. String | Null after join becomes String. But C | Null after join becomes Object.

This code can be compiled using explicit-nulls and unsafeNulls (scalac -Yexplicit-nulls -language:unsafeNulls), since unsafeNulls will ignore Null type when searching members, then there is no need to join the OrType (see Types.goOr).

(String | Null).join can return a correct result, because the type of String after widenExpr is different, approximateOr uses subtyping and lub to decide the result.

// approximateOr 
if ((tp1w ne tp1) || (tp2w ne tp2)) {
  val isSingle1 = tp1.isInstanceOf[SingletonType]
  val isSingle2 = tp2.isInstanceOf[SingletonType]
  return {
    if (tp2w eq tp2) orDominator(tp1w | tp2)                  // 2.1
    else if (tp1w eq tp1) orDominator(tp1 | tp2w)             // 2.1
    else if (tp1w frozen_<:< tp2w) orDominator(tp1w | tp2)    // 2.2
    else if (tp2w frozen_<:< tp1w) orDominator(tp1 | tp2w)    // 2.2
    else if (isSingle1 && !isSingle2) orDominator(tp1w | tp2) // 2.3
    else if (isSingle2 && !isSingle1) orDominator(tp1 | tp2w) // 2.3
    else if (tp1 frozen_<:< tp2w) tp2w                        // 2.4
    else if (tp2 frozen_<:< tp1w) tp1w                        // 2.5
    else orDominator(tp1w | tp2)                              // 2.6
  }
}

However, the type of C after widenExpr is the same, approximateOr uses baseClasses of C and Null to decide. I think we need to handle bottom classes for commonBaseClasses more carefully to fix the issue.

// Step 3: Intersect base classes of both sides
val commonBaseClasses = tp.mapReduceOr(_.baseClasses)(intersect)
val doms = dominators(commonBaseClasses, Nil)
def baseTp(cls: ClassSymbol): Type =
  tp.baseType(cls).mapReduceOr(identity)(mergeRefinedOrApplied)
doms.map(baseTp).reduceLeft(AndType.apply)

I don't understand the reason to use baseClasses here. Why don't we using subtyping compare in step 3 as well?

@noti0na1
Copy link
Member Author

noti0na1 commented Apr 3, 2021

I have a solution for the commonBaseClasses, but some compilation tests will fail.

def commonBaseClasses(tp: Type): List[ClassSymbol] = {
  val unsafeNulls = !ctx.mode.is(Mode.SafeNulls)
  var hasNull = false

  def loop(tp: Type): List[ClassSymbol] = tp match {
    case tp: OrType =>
      val cs1 = loop(tp.tp1)
      val cs2 = loop(tp.tp2)
      if cs1.isEmpty then cs2
      else if cs2.isEmpty then cs1
      else intersect(cs1, cs2)
    case _ =>
      if tp.isNothingType then List()
      else if unsafeNulls && tp.isNullType then
        hasNull = true
        List()
      else tp.baseClasses
  }

  val cs = loop(tp)
  if hasNull then
    if cs.isEmpty then defn.NullType.baseClasses
    else if !cs.contains(defn.ObjectClass) then
      intersect(cs, defn.NullType.baseClasses)
    else cs
  else if cs.isEmpty then defn.NothingType.baseClasses
  else cs
}

@odersky
Copy link
Contributor

odersky commented Apr 3, 2021

I think there is indeed a subtype test missing, but it should come before we enter orDominator (or at the start of it). There was an implicit assumption that arguments in an OrType are incompatible with each other. This is true for OrTypes arising from lubs but not true for OrTypes given explicitly.

@odersky
Copy link
Contributor

odersky commented Apr 3, 2021

There was another problem in the base type computation that did not do the right thing for unions with bottom types.

@odersky
Copy link
Contributor

odersky commented Apr 3, 2021

I don't understand the reason to use baseClasses here. Why don't we using subtyping compare in step 3 as well?

I think it's mostly efficiency, and also avoiding cycles by trying to force too much.

@Kordyjan Kordyjan added this to the 3.0.1 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants