From 608769ae3e41b7e5b01c5d5f165909d9ec908ad2 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Tue, 19 Apr 2022 15:57:30 +0200 Subject: [PATCH 1/3] Aligns `| Nothing` with `& Any` and `Nothing |` with `Any &` for value types. This also fixes `summon[ClassTag[Int | Nothing]]` as it is now equivalent to `summon[ClassTag[Int]]`. Fixes #14970 Fixes #14964 > :warning: This is a binary breaking change. > the Previous version generated the `def f: Int` and `def f: Object` variants of the method. > In the new one, we only generate the `def f: Int` version. > Unfortunately, the previous version called the boxed version of the interface that does not exist anymore. > > Is this something that we encounter in practice? --- .../dotty/tools/dotc/core/TypeErasure.scala | 10 +++++++- .../backend/jvm/DottyBytecodeTests.scala | 23 ++++++++++++++++++- tests/neg/i5823.scala | 8 ++++--- tests/run/i14964.scala | 5 ++++ tests/run/i14964b.check | 4 ++++ tests/run/i14964b.scala | 5 ++++ 6 files changed, 50 insertions(+), 5 deletions(-) create mode 100644 tests/run/i14964.scala create mode 100644 tests/run/i14964b.check create mode 100644 tests/run/i14964b.scala diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index 14e8d6d17577..da7b0f33e539 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -365,10 +365,18 @@ object TypeErasure { def erasedLub(tp1: Type, tp2: Type)(using Context): Type = { // After erasure, C | {Null, Nothing} is just C, if C is a reference type. // We need to short-circuit this case here because the regular lub logic below - // relies on the class hierarchy, which doesn't properly capture `Null`s subtyping + // relies on the class hierarchy, which doesn't properly capture `Null`/`Nothing`s subtyping // behaviour. if (tp1.isBottomTypeAfterErasure && tp2.derivesFrom(defn.ObjectClass)) return tp2 if (tp2.isBottomTypeAfterErasure && tp1.derivesFrom(defn.ObjectClass)) return tp1 + + // After erasure, A | Nothing is just A, if A is a value type. + // We need to short-circuit this case here because the regular lub logic below + // relies on the class hierarchy, which doesn't properly capture `Nothing`s subtyping + // behaviour. + if (tp1.isExactlyNothing && tp2.derivesFrom(defn.AnyValClass)) return valueErasure(tp2) + if (tp2.isExactlyNothing && tp1.derivesFrom(defn.AnyValClass)) return valueErasure(tp1) + tp1 match { case JavaArrayType(elem1) => import dotty.tools.dotc.transform.TypeUtils._ diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index a85c28a9f878..a8cf8d7ecc28 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -13,7 +13,7 @@ import scala.tools.asm.Opcodes import scala.jdk.CollectionConverters._ import Opcodes._ -class TestBCode extends DottyBytecodeTest { +class DottyBytecodeTests extends DottyBytecodeTest { import ASMConverters._ @Test def nullChecks = { val source = """ @@ -1234,6 +1234,27 @@ class TestBCode extends DottyBytecodeTest { Label(9), Op(IRETURN))) } } + + /** Check that erasure if `Int | Nothing` is `int` */ + @Test def i14970 = { + val source = + s"""class Foo { + | def foo: Int | Nothing = 1 + | def bar: Nothing | Int = 1 + |} + """.stripMargin + + checkBCode(source) { dir => + val clsIn = dir.lookupName("Foo.class", directory = false).input + val clsNode = loadClassNode(clsIn) + def testSig(methodName: String, expectedSignature: String) = { + val signature = clsNode.methods.asScala.filter(_.name == methodName).map(_.signature) + assertEquals(List(expectedSignature), signature) + } + testSig("foo", "()I") + testSig("bar", "()I") + } + } } object invocationReceiversTestCode { diff --git a/tests/neg/i5823.scala b/tests/neg/i5823.scala index 673cdfc65e86..28c27b0c747a 100644 --- a/tests/neg/i5823.scala +++ b/tests/neg/i5823.scala @@ -7,7 +7,7 @@ class A class B class Foo { - + // ok, because A and B are <: Object. def foo(a: A|Null): Unit = () def foo(b: B|Null): Unit = () @@ -26,13 +26,15 @@ class Foo { def foo2(a: A|Nothing): Unit = () def foo2(b: B|Nothing): Unit = () + // ok because erased to primitive types def bar2(a: Int|Nothing): Unit = () - def bar2(b: Boolean|Nothing): Unit = () // error: signatures match + def bar2(b: Boolean|Nothing): Unit = () // ok, T is erased to `String` and `Integer`, respectively def gen3[T <: String](s: T|Nothing): Unit = () def gen3[T <: Integer](i: T|Nothing): Unit = () + // ok because erased to primitive types def gen4[T <: Int](i: T|Nothing): Unit = () - def gen4[T <: Boolean](b: T|Nothing): Unit = () // error: signatures match + def gen4[T <: Boolean](b: T|Nothing): Unit = () } diff --git a/tests/run/i14964.scala b/tests/run/i14964.scala new file mode 100644 index 000000000000..33970caccf1e --- /dev/null +++ b/tests/run/i14964.scala @@ -0,0 +1,5 @@ +@main def Test: Unit = + val xs = (1, 2).toList + val a1 = xs.toArray + println((1, 2).toList.toArray) + val a2 = (1, 2).toList.toArray diff --git a/tests/run/i14964b.check b/tests/run/i14964b.check new file mode 100644 index 000000000000..f55e93967e38 --- /dev/null +++ b/tests/run/i14964b.check @@ -0,0 +1,4 @@ +Int +Int +Int +Int diff --git a/tests/run/i14964b.scala b/tests/run/i14964b.scala new file mode 100644 index 000000000000..cd0c6422fb80 --- /dev/null +++ b/tests/run/i14964b.scala @@ -0,0 +1,5 @@ +@main def Test: Unit = + println(summon[reflect.ClassTag[Int]]) // classOf[Int] + println(summon[reflect.ClassTag[Int | Int]]) // classOf[Int] + println(summon[reflect.ClassTag[Int | 1]]) // classOf[Int] + println(summon[reflect.ClassTag[Int | Nothing]]) // classOf[Object] From 49cef31af1ed2fa7efcb3554cba60bfebf5fcf0d Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 21 Apr 2022 13:45:53 +0200 Subject: [PATCH 2/3] Refactor T|Nothing logic --- .../dotty/tools/dotc/core/TypeErasure.scala | 22 ++++++------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index da7b0f33e539..29b40670e8e3 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -363,21 +363,13 @@ object TypeErasure { * which leads to more predictable bytecode and (?) faster dynamic dispatch. */ def erasedLub(tp1: Type, tp2: Type)(using Context): Type = { - // After erasure, C | {Null, Nothing} is just C, if C is a reference type. - // We need to short-circuit this case here because the regular lub logic below - // relies on the class hierarchy, which doesn't properly capture `Null`/`Nothing`s subtyping - // behaviour. - if (tp1.isBottomTypeAfterErasure && tp2.derivesFrom(defn.ObjectClass)) return tp2 - if (tp2.isBottomTypeAfterErasure && tp1.derivesFrom(defn.ObjectClass)) return tp1 - - // After erasure, A | Nothing is just A, if A is a value type. - // We need to short-circuit this case here because the regular lub logic below - // relies on the class hierarchy, which doesn't properly capture `Nothing`s subtyping - // behaviour. - if (tp1.isExactlyNothing && tp2.derivesFrom(defn.AnyValClass)) return valueErasure(tp2) - if (tp2.isExactlyNothing && tp1.derivesFrom(defn.AnyValClass)) return valueErasure(tp1) - - tp1 match { + // We need to short-circuit the following 2 case because the regular lub logic in the else relies on + // the class hierarchy, which doesn't properly capture `Nothing`/`Null` subtyping behaviour. + if tp1.isRef(defn.NothingClass) || (tp1.isRef(defn.NullClass) && tp2.derivesFrom(defn.ObjectClass)) then + tp2 // After erasure, Nothing | T is just T and Null | C is just C, if C is a reference type. + else if tp2.isRef(defn.NothingClass) || (tp2.isRef(defn.NullClass) && tp1.derivesFrom(defn.ObjectClass)) then + tp1 // After erasure, T | Nothing is just T and C | Null is just C, if C is a reference type. + else tp1 match { case JavaArrayType(elem1) => import dotty.tools.dotc.transform.TypeUtils._ tp2 match { From e7231b264f4a060bf5abda5112d3c577785865f2 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 21 Apr 2022 14:40:30 +0200 Subject: [PATCH 3/3] [DO NOT MERGE] Check if community build used |Nothing signature --- .../dotty/tools/dotc/core/TypeErasure.scala | 14 +++++-- .../backend/jvm/DottyBytecodeTests.scala | 38 +++++++++---------- tests/{run => neg}/i14964.scala | 2 + tests/{run => neg}/i14964b.scala | 1 + tests/neg/i5823.scala | 4 ++ 5 files changed, 36 insertions(+), 23 deletions(-) rename tests/{run => neg}/i14964.scala (81%) rename tests/{run => neg}/i14964b.scala (94%) diff --git a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala index 29b40670e8e3..efc376796403 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeErasure.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeErasure.scala @@ -365,10 +365,16 @@ object TypeErasure { def erasedLub(tp1: Type, tp2: Type)(using Context): Type = { // We need to short-circuit the following 2 case because the regular lub logic in the else relies on // the class hierarchy, which doesn't properly capture `Nothing`/`Null` subtyping behaviour. - if tp1.isRef(defn.NothingClass) || (tp1.isRef(defn.NullClass) && tp2.derivesFrom(defn.ObjectClass)) then - tp2 // After erasure, Nothing | T is just T and Null | C is just C, if C is a reference type. - else if tp2.isRef(defn.NothingClass) || (tp2.isRef(defn.NullClass) && tp1.derivesFrom(defn.ObjectClass)) then - tp1 // After erasure, T | Nothing is just T and C | Null is just C, if C is a reference type. + if (tp1.isRef(defn.NothingClass) || tp1.isRef(defn.NullClass)) && tp2.derivesFrom(defn.ObjectClass) then + tp2 // After erasure, {Nothing|Null} | C is just C, if C is a reference type. + else if (tp2.isRef(defn.NothingClass) || tp2.isRef(defn.NullClass)) && tp1.derivesFrom(defn.ObjectClass) then + tp1 // After erasure, C | {Nothing|Null} is just C, if C is a reference type. + else if tp1.isRef(defn.NothingClass) then + report.error(em"erasedLub($tp1, $tp2)") + tp2 // After erasure, Nothing | T is just T + else if tp2.isRef(defn.NothingClass) then + report.error(em"erasedLub($tp1, $tp2)") + tp1 // After erasure, T | Nothing is just T else tp1 match { case JavaArrayType(elem1) => import dotty.tools.dotc.transform.TypeUtils._ diff --git a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala index a8cf8d7ecc28..625899eef799 100644 --- a/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala +++ b/compiler/test/dotty/tools/backend/jvm/DottyBytecodeTests.scala @@ -1236,25 +1236,25 @@ class DottyBytecodeTests extends DottyBytecodeTest { } /** Check that erasure if `Int | Nothing` is `int` */ - @Test def i14970 = { - val source = - s"""class Foo { - | def foo: Int | Nothing = 1 - | def bar: Nothing | Int = 1 - |} - """.stripMargin - - checkBCode(source) { dir => - val clsIn = dir.lookupName("Foo.class", directory = false).input - val clsNode = loadClassNode(clsIn) - def testSig(methodName: String, expectedSignature: String) = { - val signature = clsNode.methods.asScala.filter(_.name == methodName).map(_.signature) - assertEquals(List(expectedSignature), signature) - } - testSig("foo", "()I") - testSig("bar", "()I") - } - } + // @Test def i14970 = { + // val source = + // s"""class Foo { + // | def foo: Int | Nothing = 1 + // | def bar: Nothing | Int = 1 + // |} + // """.stripMargin + + // checkBCode(source) { dir => + // val clsIn = dir.lookupName("Foo.class", directory = false).input + // val clsNode = loadClassNode(clsIn) + // def testSig(methodName: String, expectedSignature: String) = { + // val signature = clsNode.methods.asScala.filter(_.name == methodName).map(_.signature) + // assertEquals(List(expectedSignature), signature) + // } + // testSig("foo", "()I") + // testSig("bar", "()I") + // } + // } } object invocationReceiversTestCode { diff --git a/tests/run/i14964.scala b/tests/neg/i14964.scala similarity index 81% rename from tests/run/i14964.scala rename to tests/neg/i14964.scala index 33970caccf1e..dbecadf935e1 100644 --- a/tests/run/i14964.scala +++ b/tests/neg/i14964.scala @@ -1,3 +1,5 @@ +// nopos-error +// nopos-error @main def Test: Unit = val xs = (1, 2).toList val a1 = xs.toArray diff --git a/tests/run/i14964b.scala b/tests/neg/i14964b.scala similarity index 94% rename from tests/run/i14964b.scala rename to tests/neg/i14964b.scala index cd0c6422fb80..2e53f3648a2c 100644 --- a/tests/run/i14964b.scala +++ b/tests/neg/i14964b.scala @@ -1,3 +1,4 @@ +// nopos-error @main def Test: Unit = println(summon[reflect.ClassTag[Int]]) // classOf[Int] println(summon[reflect.ClassTag[Int | Int]]) // classOf[Int] diff --git a/tests/neg/i5823.scala b/tests/neg/i5823.scala index 28c27b0c747a..03dc9cbec622 100644 --- a/tests/neg/i5823.scala +++ b/tests/neg/i5823.scala @@ -1,3 +1,7 @@ +// nopos-error +// nopos-error +// nopos-error +// nopos-error // Test that `C|Null` is erased to `C` if `C` is // a reference type. // If `C` is a value type, then `C|Null = Object`.