fix #1354: improve type test of union types

[liufengyun]

Perform following transform before erasure:

expr.isInstanceOf[A | B]  ~~>  expr.isInstanceOf[A] | expr.isInstanceOf[B]

[smarter]

I'm happy with fixing isInstanceOf[A | B], but I disagree with changing asInstanceOf[A | B]

• try/catch are heavy machinery, you could do something simpler using just isInstanceOf checks
• but even doing that seems like a worse idea than just getting asInstanceOf[erasedLub(A, B)]:
  1. When I do val x: A | B = ... it gets erased to val x: erasedLub(A, B) = ... so it makes sense for asInstanceOf to behave similarly
  2. The semantics of asInstanceOf have always been the semantics of the underlying platform: you can cast a List[Int] to a List[String] and we won't crash at runtime because this is allowed by the implementation, similarly you should be able to cast whatever you want to A | B and it shouldn't crash if it's allowed by the implementation (that is, if it's any subtype of the erased type of A | B)

[smarter]

Note that this difference in behavior of isInstanceOf and asInstanceOf is not unprecedented, this is already how singleton types behave in scalac:

val x = "foo"
val y = "bar" 
x.isInstanceOf[y.type] // false in scalac
x.asInstanceOf[y.type] // true in scalac

Note: I just tested this in dotty and x.isInstanceOf[y.type] returns true, I think that's a bug that should be fixed in a similar way to this PR.

[sjrd]

👍 on @smarter's comment.
Not to mention that @liufengyun's proposed encoding of asInstanceOf[A | B] is plain wrong in Scala.js, as failing asInstanceOfs are Undefined Behavior. Also, not all types have an isInstanceOf (traits extending js.Any), so the alternative encoding if (x.isInstanceOf[A]) x.asInstanceOf[A] else x.asInstanceOf[B] is equally wrong (when A is a trait extending js.Any).

asInstanceOf is just a way to tell the compiler: just let me do this. It shouldn't try to give any "strong" guarantee. It just has to perform enough so that validating bytecode/IR can be produced.

[liufengyun]

Thanks for your comment @smarter @sjrd , then I think it's best to keep asInstanceOf as before.

[liufengyun]

@smarter interested to review?

[odersky]

I agree with @smarter and @sjrd on asInstanceOf.

The current version LGTM.

[DarkDimius]

@smarter

Note: I just tested this in dotty and x.isInstanceOf[y.type] returns true, I think that's a bug that should be fixed in a similar way to this PR.

It's intentional.