Skip to content

Commit

Permalink
Fix scala#17467: Limit isNullable widening to stable TermRefs; remove…
Browse files Browse the repository at this point in the history 
… under explicit nulls. (scala#17470)

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 `TermRef`s 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`.
  • Loading branch information
@Kordyjan
Kordyjan authored Aug 4, 2023
2 parents 6a6ee08 + 3945e96 commit 73967d5
Show file tree
Hide file tree
Showing 6 changed files with 158 additions and 2 deletions.
10 changes: 9 additions & 1 deletion compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -943,16 +943,24 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
// However, `null` can always be a value of `T` for Java side.
// So the best solution here is to let `Null` be a subtype of non-primitive
// value types temporarily.
def isNullable(tp: Type): Boolean = tp.widenDealias match
def isNullable(tp: Type): Boolean = tp.dealias match
case tp: TypeRef =>
val tpSym = tp.symbol
ctx.mode.is(Mode.RelaxedOverriding) && !tpSym.isPrimitiveValueClass ||
tpSym.isNullableClass
case tp: TermRef =>
// https://scala-lang.org/files/archive/spec/2.13/03-types.html#singleton-types
// A singleton type is of the form p.type. Where p is a path pointing to a value which conforms to
// scala.AnyRef [Scala 3: which scala.Null conforms to], the type denotes the set of values consisting
// of null and the value denoted by p (i.e., the value v for which v eq p). [Otherwise,] the type
// denotes the set consisting of only the value denoted by p.
!ctx.explicitNulls && isNullable(tp.underlying) && tp.isStable
case tp: RefinedOrRecType => isNullable(tp.parent)
case tp: AppliedType => isNullable(tp.tycon)
case AndType(tp1, tp2) => isNullable(tp1) && isNullable(tp2)
case OrType(tp1, tp2) => isNullable(tp1) || isNullable(tp2)
case AnnotatedType(tp1, _) => isNullable(tp1)
case ConstantType(c) => c.tag == Constants.NullTag
case _ => false
val sym1 = tp1.symbol
(sym1 eq NothingClass) && tp2.isValueTypeOrLambda ||
Expand Down
31 changes: 31 additions & 0 deletions tests/explicit-nulls/neg/i17467.check
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
-- [E007] Type Mismatch Error: tests/explicit-nulls/neg/i17467.scala:4:22 ----------------------------------------------
4 | val a2: a1.type = null // error
| ^^^^
| Found: Null
| Required: (a1 : String)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (a1 : String)
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/explicit-nulls/neg/i17467.scala:7:22 ----------------------------------------------
7 | val b2: b1.type = null // error
| ^^^^
| Found: Null
| Required: (b1 : String | Null)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (b1 : String | Null)
|
| longer explanation available when compiling with `-explain`
-- [E172] Type Error: tests/explicit-nulls/neg/i17467.scala:8:28 -------------------------------------------------------
8 | summon[Null <:< b1.type] // error
| ^
| Cannot prove that Null <:< (b1 : String | Null).
-- [E007] Type Mismatch Error: tests/explicit-nulls/neg/i17467.scala:14:22 ---------------------------------------------
14 | val c2: c1.type = null // error
| ^^^^
| Found: Null
| Required: (c1 : Null)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (c1 : Null)
|
| longer explanation available when compiling with `-explain`
16 changes: 16 additions & 0 deletions tests/explicit-nulls/neg/i17467.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
object Test:
def test(): Unit =
val a1: String = "foo"
val a2: a1.type = null // error

val b1: String | Null = "foo"
val b2: b1.type = null // error
summon[Null <:< b1.type] // error

/* The following would be sound, but it would require a specific subtyping
* rule (and implementation code) for debatable value. So it is an error.
*/
val c1: Null = null
val c2: c1.type = null // error
end test
end Test
64 changes: 64 additions & 0 deletions tests/neg/i17467.check
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:6:20 -------------------------------------------------------------
6 | val b1: "foo" = null // error
| ^^^^
| Found: Null
| Required: ("foo" : String)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than ("foo" : String)
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:9:22 -------------------------------------------------------------
9 | val c2: c1.type = null // error
| ^^^^
| Found: Null
| Required: (c1 : ("foo" : String))
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (c1 : ("foo" : String))
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:17:22 ------------------------------------------------------------
17 | val e2: e1.type = null // error
| ^^^^
| Found: Null
| Required: (e1 : MyNonNullable)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (e1 : MyNonNullable)
|
| longer explanation available when compiling with `-explain`
-- [E172] Type Error: tests/neg/i17467.scala:19:26 ---------------------------------------------------------------------
19 | summon[Null <:< "foo"] // error
| ^
| Cannot prove that Null <:< ("foo" : String).
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:21:23 ------------------------------------------------------------
21 | val f1: Mod.type = null // error
| ^^^^
| Found: Null
| Required: Test.Mod.type
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than Test.Mod.type
|
| longer explanation available when compiling with `-explain`
-- [E083] Type Error: tests/neg/i17467.scala:24:12 ---------------------------------------------------------------------
24 | val g2: g1.type = null // error // error
| ^^^^^^^
| (g1 : AnyRef) is not a valid singleton type, since it is not an immutable path
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:24:22 ------------------------------------------------------------
24 | val g2: g1.type = null // error // error
| ^^^^
| Found: Null
| Required: (g1 : AnyRef)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (g1 : AnyRef)
|
| longer explanation available when compiling with `-explain`
-- [E007] Type Mismatch Error: tests/neg/i17467.scala:33:24 ------------------------------------------------------------
33 | def me: this.type = null // error
| ^^^^
| Found: Null
| Required: (Bar.this : Test.Bar)
| Note that implicit conversions were not tried because the result of an implicit conversion
| must be more specific than (Bar.this : Test.Bar)
|
| longer explanation available when compiling with `-explain`
34 changes: 34 additions & 0 deletions tests/neg/i17467.scala
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
object Test:
def test(): Unit =
val a1: String = "foo"
val a2: a1.type = null // OK

val b1: "foo" = null // error

val c1: "foo" = "foo"
val c2: c1.type = null // error

type MyNullable = String
val d1: MyNullable = "foo"
val d2: d1.type = null // OK

type MyNonNullable = Int
val e1: MyNonNullable = 5
val e2: e1.type = null // error

summon[Null <:< "foo"] // error

val f1: Mod.type = null // error

var g1: AnyRef = "foo"
val g2: g1.type = null // error // error

val h1: Null = null
val h2: h1.type = null
end test

object Mod

class Bar:
def me: this.type = null // error
end Test
5 changes: 4 additions & 1 deletion tests/run/t6443b.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@ trait A {
type D >: Null <: C
def foo(d: D)(d2: d.type): Unit
trait C {
def bar: Unit = foo(null)(null)
def bar: Unit = {
val nul = null
foo(nul)(nul)
}
}
}
object B extends A {
Expand Down

0 comments on commit 73967d5

Please sign in to comment.