Fix #17467: Limit isNullable widening to stable TermRefs; remove under explicit nulls.

[sjrd]

The Scala language specification has a peculiar clause about the nullness of singleton types of the form path.type. It says that Null <:< path.type if the underlying type U of path is nullable itself.

The previous implementation of that rule was overly broad, as it indiscrimately widened all types. This resulted in problematic subtyping relationships like Null <:< "foo".

We do not widen anymore. Instead, we specifically handle TermRefs of stable members, which are how dotc represents singleton types. We also have a rule for Null <:< null, which is necessary for pattern matching exhaustivity to keep working in the presence of nulls.

---

Under explicit nulls, remove the rule Null <:< x.type.

The specified rule that Null <:< x.type when the underlying type U of x is nullable is dubious to begin with. Under explicit nulls, it becomes decidedly out of place. We now disable that rule under -Yexplicit-nulls.

[odersky]

Nicely confined changes!

[noti0na1]

I agree this is definitely a more refined rule for singleton type!
BTW, is there any specific reason why

val c1: Null = null 
val c2: c1.type = null

is disallowed under explicit nulls?

[noti0na1]

summon[Null <:< b1.type]

[sjrd]

BTW, is there any specific reason why

val c1: Null = null 
val c2: c1.type = null

is disallowed under explicit nulls?

I added a comment with the test you suggested. The reason is that allowing this under explicit nulls would require a dedicated subtyping rule (both in spec and in implementation), and would provide marginal/debatable value. So IMO it is better to leave it out.

[noti0na1]

LGTM

[sjrd]

Rebased to be on top of #18284, since this was waiting for a minor release.

@Kordyjan Could you confirm that main is for 3.4.x now, and that therefore we can merge things that need a minor release?

[Kordyjan]

Yes. The Main is now 3.4. It can be safely merged.

[WojciechMazur]

We've found 1 project (slick/slick) which failed in Open CB after merging this change. The failing code can be minimized to:

class Foo:
  def foo(flag: Boolean): this.type = if(flag) this else null 

Failing with:

-- [E007] Type Mismatch Error: /Users/wmazur/projects/dotty/bisect/main.scala:4:49 
4 |  def foo(): this.type = if(flag) this else null 
  |                                            ^^^^
  |Found:    Null
  |Required: (Foo.this : Foo)

The snippet above makes no sense to me so I don't treat it as a regression.

[sjrd]

Indeed, that makes absolutely no sense, and is precisely the kind of soundness holes that this PR is meant to fix.

[WojciechMazur]

I've found 1 more project failing due this change simy4/coregex, which also should not be a regression:

type Matching[A >: Null <: String, Regex >: Null <: String with Singleton] <: A
def uuid: Matching[String, "[0-9a-f]"] = ??? // error Type argument ("[0-9a-f]" : String) does not conform to lower bound Null

The obious workaround it removing the null bound which allows to compile it:

type Matching[A <: String, Regex <: String with Singleton] <: A
def uuid: Matching[String, "[0-9a-f]"] = ???

[sjrd]

We've found 1 project (slick/slick) which failed in Open CB after merging this change. The failing code can be minimized to:

Actually, after thinking about this more, I think that particular case, with the this.type, is in fact an unintended regression. this.type is a singleton type in spec language, but not a TermRef in the implementation (it's a ThisType instead), so I think we need to handle ThisType in the new logic.