Add a phase to Dotty that hides Dotty bottom types from JVM. #1002

DarkDimius · 2015-12-16T15:05:36Z

It is very convenient to have bottom types in type system. Unfortunately
JVM does not have bottom types and we need to insert casts that would make
bytecode pass verification.

DarkDimius · 2015-12-16T23:13:39Z

/rebuild

DarkDimius · 2015-12-17T12:20:42Z

@odersky please review.

smarter · 2015-12-18T15:07:53Z

src/dotty/tools/dotc/transform/Erasure.scala

@@ -263,6 +263,12 @@ object Erasure extends TypeTestsCasts{
          else
            cast(tree, pt)
      }
+
+
+    /** Is `tpe` a type which is a bottom type for Dotty, but not a bottom type for JVM */


There is no bottom type on the JVM so I don't really understand the naming of this function, how is it different from Definitions#isBottomType?

smarter · 2015-12-18T15:28:24Z

Nothing is supposed to have no value, so I don't know if allowing null.asInstanceOf[Nothing] to run without any error is a good thing or not, maybe it doesn't matter.

odersky · 2015-12-18T15:45:40Z

src/dotty/tools/dotc/transform/Erasure.scala

+    /** Is `tpe` a type which is a bottom type for Dotty, but not a bottom type for JVM */
+    def isNonJVMBottomType(tpe: Type)(implicit ctx: Context): Boolean = {
+      tpe.derivesFrom(ctx.definitions.NothingClass) || tpe.derivesFrom(ctx.definitions.NullClass)
+    }
  }


Indeed, why not use Definitions#isBottomType?

Definitions#isBottomType(tp) is defined to be tp.symbol \in {NothingClass, NullClass}
It does not work for types that do not return a usable symbol, such as And\Or types.

Should I update Definitions#isBottomType to account for Null | Nothing and Int & Nothing?

No, I think it's OK to leave the two separate. Maybe rename the second to
derivesFromBottomType
to make clear that refinements and & / | count as well?

On Fri, Dec 18, 2015 at 6:32 PM, Dmitry Petrashko [email protected]
wrote:

In src/dotty/tools/dotc/transform/Erasure.scala
#1002 (comment):

@@ -263,6 +263,12 @@ object Erasure extends TypeTestsCasts{
else
cast(tree, pt)
}
+
+

/** Is tpe a type which is a bottom type for Dotty, but not a bottom type for JVM */

def isNonJVMBottomType(tpe: Type)(implicit ctx: Context): Boolean = {

tpe.derivesFrom(ctx.definitions.NothingClass) || tpe.derivesFrom(ctx.definitions.NullClass)

}
}

Should I update Definitions#isBottomType to account for Null | Nothing
and Int & Nothing?

—
Reply to this email directly or view it on GitHub
https://github.com/lampepfl/dotty/pull/1002/files#r48048942.

Martin Odersky
EPFL

DarkDimius · 2015-12-20T16:19:30Z

@smarter

Nothing is supposed to have no value, so I don't know if allowing null.asInstanceOf[Nothing] to run without any error is a good thing or not, maybe it doesn't matter.

I do not have a strong opinion on this either. So I've implemented be compatible with scalac mode.

class A {
 val s: Nothing  = null.asInstanceOf[Nothing]
}

scalac emmits:

public class A {
    private final Nothing. s = null;

    public Nothing. s() {
        return this.s;
    }
}

PS: the . in Nothing. is not a printing error. Nothing is replaced by Nothing$ in a late transformation done by GenBCode.

smarter · 2015-12-20T16:26:45Z

Being compatible with scalac seems OK here, but note that the following fails at runtime in scalac but works with your patch:

object X {
  def main(args: Array[String]): Unit = {
    val x: Int = null.asInstanceOf[Nothing]
  }
}

smarter · 2015-12-20T16:29:24Z

And now that I think about it, this might be a good think because in the following:

object X {
  def main(args: Array[String]): Unit = {
    val x: Int = null.asInstanceOf
  }
}

The type parameter of asInstanceOf will be instantiated to Nothing, so it crashes at runtime in scalac but works in dotty with your patch.

odersky · 2015-12-21T11:54:55Z

src/dotty/tools/dotc/transform/Erasure.scala

+    /** Is `tpe` a type which is a bottom type for Dotty, but not a bottom type for JVM */
+    def isNonJVMBottomType(tpe: Type)(implicit ctx: Context): Boolean = {
+      tpe.derivesFrom(ctx.definitions.NothingClass) || tpe.derivesFrom(ctx.definitions.NullClass)
+    }


I now did a similar change in definitions (f780a37).
So it's best if you cherry-pick the commit and use defn.isBottomType.

It is very convenient to have bottom types in type system. Unfortunately JVM does not have bottom types and we need to insert casts that would make bytecode pass verification.

DarkDimius · 2016-01-04T10:41:28Z

I've addressed most comments except the (null.asInstanceOf[Nothing]): Int) one.
I do not have a clear understanding of how this works in scalac and why is it different from the other case.
If you someone is interested to get to the bottom of this, It could be done in a different PR. This one fixes a compiler crash so I'd like to get this in.

sjrd · 2016-01-04T10:44:40Z

FTR, in Scala.js we recently decided to go against what scalac/JVM does for null.asInstanceOf[Nothing]. Now it always throws immediately (as a ClassCastException/undefined behavior). See the PR scala-js/scala-js#2126 fixing issue scala-js/scala-js#2125

smarter · 2016-01-04T10:51:56Z

The following comments have not been addressed:

DarkDimius · 2016-01-06T15:20:51Z

java.lang.OutOfMemoryError: GC overhead limit exceeded

/rebuild

odersky · 2016-07-27T22:18:01Z

Let's either rebase and try again or close.

smarter reviewed Dec 18, 2015
View reviewed changes

odersky reviewed Dec 18, 2015
View reviewed changes

odersky reviewed Dec 21, 2015
View reviewed changes

DarkDimius self-assigned this Jan 3, 2016

DarkDimius added 3 commits January 4, 2016 11:28

Add a phase to Dotty that hides Dotty bottom types from JVM.

1f9fdaf

It is very convenient to have bottom types in type system. Unfortunately JVM does not have bottom types and we need to insert casts that would make bytecode pass verification.

Text that scala#828 is fixed.

4da5dfa

Fix casting bottom types to non-value types.

4168a25

DarkDimius force-pushed the fix-#828 branch 2 times, most recently from 45278e1 to 64e17d0 Compare January 4, 2016 10:40

DarkDimius removed their assignment Jan 4, 2016

Rebase and address reviewer comments.

142268a

DarkDimius force-pushed the fix-#828 branch from 64e17d0 to 142268a Compare January 4, 2016 13:10

Play with Or-types of bottom types.

15eaa10

felixmulder mentioned this pull request Oct 6, 2016

Remove bottom types that are not relevant for JVM #1559

Closed

felixmulder closed this Oct 6, 2016

nicolasstucki mentioned this pull request Oct 17, 2017

null asInstance of Nothing not always throwing #3340

Closed

nicolasstucki mentioned this pull request Feb 8, 2019

Fix #3340: Always throw when casting to scala.Nothing #5874

Merged

Add a phase to Dotty that hides Dotty bottom types from JVM. #1002

Add a phase to Dotty that hides Dotty bottom types from JVM. #1002

Uh oh!

Conversation

DarkDimius commented Dec 16, 2015

Uh oh!

DarkDimius commented Dec 16, 2015

Uh oh!

DarkDimius commented Dec 17, 2015

Uh oh!

smarter Dec 18, 2015

Choose a reason for hiding this comment

Uh oh!

smarter commented Dec 18, 2015

Uh oh!

odersky Dec 18, 2015

Choose a reason for hiding this comment

Uh oh!

DarkDimius Dec 18, 2015

Choose a reason for hiding this comment

Uh oh!

DarkDimius Dec 18, 2015

Choose a reason for hiding this comment

Uh oh!

odersky Dec 18, 2015

Choose a reason for hiding this comment

Uh oh!

DarkDimius Dec 20, 2015

Choose a reason for hiding this comment

Uh oh!

DarkDimius commented Dec 20, 2015

Uh oh!

smarter commented Dec 20, 2015

Uh oh!

smarter commented Dec 20, 2015

Uh oh!

odersky Dec 21, 2015

Choose a reason for hiding this comment

Uh oh!

DarkDimius commented Jan 4, 2016

Uh oh!

sjrd commented Jan 4, 2016

Uh oh!

smarter commented Jan 4, 2016

Uh oh!

DarkDimius commented Jan 6, 2016

Uh oh!

odersky commented Jul 27, 2016

Uh oh!

Uh oh!