From 65363f931996b67fbd8d31e8974ac8b54a5c74b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 11 May 2023 16:19:10 +0200 Subject: [PATCH 1/2] Fix #17467: Limit isNullable widening to stable TermRefs. 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. --- .../dotty/tools/dotc/core/TypeComparer.scala | 10 ++- tests/neg/i17467.check | 64 +++++++++++++++++++ tests/neg/i17467.scala | 34 ++++++++++ tests/run/t6443b.scala | 5 +- 4 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 tests/neg/i17467.check create mode 100644 tests/neg/i17467.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 2a99e791c6c0..e6a45b100012 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -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. + 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 || diff --git a/tests/neg/i17467.check b/tests/neg/i17467.check new file mode 100644 index 000000000000..26ea170d7af5 --- /dev/null +++ b/tests/neg/i17467.check @@ -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` diff --git a/tests/neg/i17467.scala b/tests/neg/i17467.scala new file mode 100644 index 000000000000..f527b6981b41 --- /dev/null +++ b/tests/neg/i17467.scala @@ -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 diff --git a/tests/run/t6443b.scala b/tests/run/t6443b.scala index 9320b1dcfe2f..796fd9d95df4 100644 --- a/tests/run/t6443b.scala +++ b/tests/run/t6443b.scala @@ -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 { From 3945e96934f7a7e4c75ccf0f319bf03be669ba7a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Doeraene?= Date: Thu, 11 May 2023 16:27:00 +0200 Subject: [PATCH 2/2] 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`. --- .../dotty/tools/dotc/core/TypeComparer.scala | 2 +- tests/explicit-nulls/neg/i17467.check | 31 +++++++++++++++++++ tests/explicit-nulls/neg/i17467.scala | 16 ++++++++++ 3 files changed, 48 insertions(+), 1 deletion(-) create mode 100644 tests/explicit-nulls/neg/i17467.check create mode 100644 tests/explicit-nulls/neg/i17467.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index e6a45b100012..e763f6c7a303 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -954,7 +954,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // 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. - isNullable(tp.underlying) && tp.isStable + !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) diff --git a/tests/explicit-nulls/neg/i17467.check b/tests/explicit-nulls/neg/i17467.check new file mode 100644 index 000000000000..3b5e556ba36d --- /dev/null +++ b/tests/explicit-nulls/neg/i17467.check @@ -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` diff --git a/tests/explicit-nulls/neg/i17467.scala b/tests/explicit-nulls/neg/i17467.scala new file mode 100644 index 000000000000..2e96252f6759 --- /dev/null +++ b/tests/explicit-nulls/neg/i17467.scala @@ -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