Skip to content
Browse files

SI-7159 Prepare to remove erroneous INT <:< LONG in TypeKinds

In preparation for dealing with a problem in TypeKinds, this commit
does some cleanup of code related to doing coercions.

* Comments are added to clarify.
* A println when converting between BOOL and anything else is removed
and  the code is allowed to flow through to an assertion.
* Assertions are refactored to use string interpolation.
* A few pattern matches were reformulated to equivalent variants

In addition, a test is created for SI-107, the bug that necessitated
the special case in GenICode#adapt for LONG coercion
  • Loading branch information...
1 parent 208d6ad commit 04b147e4a0c226526d3192b68f8efa8953b531ea @JamesIry JamesIry committed
View
35 src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -1038,14 +1038,9 @@ abstract class GenICode extends SubComponent {
// A typical example is an overloaded type assigned after typer.
log(s"GenICode#adapt($from, $to, $ctx, $pos)")
- val conforms = (from <:< to)
def coerce(from: TypeKind, to: TypeKind) = ctx.bb.emit(CALL_PRIMITIVE(Conversion(from, to)), pos)
- def checkAssertions() {
- def msg = s"Can't convert from $from to $to in unit ${unit.source} at $pos"
- debugassert(from != UNIT, msg)
- assert(!from.isReferenceType && !to.isReferenceType, msg)
- }
- if (conforms) from match {
+
+ (from, to) match {
// The JVM doesn't have a Nothing equivalent, so it doesn't know that a method of type Nothing can't actually return. So for instance, with
// def f: String = ???
// we need
@@ -1053,15 +1048,23 @@ abstract class GenICode extends SubComponent {
// 3: invokevirtual #29; //Method scala/Predef$.$qmark$qmark$qmark:()Lscala/runtime/Nothing$;
// 6: athrow
// So this case tacks on the ahtrow which makes the JVM happy because class Nothing is declared as a subclass of Throwable
- case NothingReference => ctx.bb.emit(THROW(ThrowableClass)) ; ctx.bb.enterIgnoreMode()
- case _ =>
- // widen subrange types
- if (from.isIntSizedType && to == LONG)
- coerce(INT, LONG)
- }
- else to match {
- case UNIT => ctx.bb.emit(DROP(from), pos) // value discarding
- case _ => checkAssertions() ; coerce(from, to) // other primitive coercions
+ case (NothingReference, _) =>
+ ctx.bb.emit(THROW(ThrowableClass))
+ ctx.bb.enterIgnoreMode()
+ // this special case is needed because of a special case in TypeKinds that
+ // says that the int sized primitives are subtypes of LONG
+ // even though they aren't according to the JVM
+ case (_, LONG) if from.isIntSizedType =>
+ coerce(INT, LONG)
+ case _ if (from <:< to) =>
+ ()
+ case (_, UNIT) =>
+ ctx.bb.emit(DROP(from), pos)
+ // otherwise we'd better be doing a primtive -> primitive coercion or there's a problem
+ case _ if !from.isRefOrArrayType && !to.isRefOrArrayType =>
+ coerce(from, to)
+ case _ =>
+ assert(false, s"Can't convert from $from to $to in unit ${unit.source} at $pos")
}
}
View
17 src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
@@ -88,10 +88,19 @@ trait TypeKinds { self: ICodes =>
final def isNumericType: Boolean = isIntegralType | isRealType
/** Simple subtyping check */
- def <:<(other: TypeKind): Boolean = (this eq other) || (this match {
- case BOOL | BYTE | SHORT | CHAR => other == INT || other == LONG
- case _ => this eq other
- })
+ def <:<(other: TypeKind): Boolean = other match {
+ // On the JVM, BOOL, BYTE, CHAR, SHORT need no coercion to INT
+ // TODO it's pretty suspect to call this a subtyping relationship
+ // for instance JVM Arrays are covariant, but Array[Char] is not
+ // a subtype of Array[Int] on the JVM. However, when I attempted
+ // to remove it I got verify errors when compiling the library
+ // under -optimize
+ case INT => this.isIntSizedType
+ // this case is even more suspect than the previous because
+ // BOOL, BYTE, CHAR, SHORT, and INT need conversion to get to LONG
+ case LONG => this.isIntSizedType || this == LONG
+ case _ => this eq other
+ }
/** Is this type a category 2 type in JVM terms? (ie, is it LONG or DOUBLE?) */
def isWideType: Boolean = false
View
12 src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
@@ -2626,8 +2626,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
* @param to The type the value will be converted into.
*/
def emitT2T(from: TypeKind, to: TypeKind) {
- assert(isNonUnitValueTK(from), from)
- assert(isNonUnitValueTK(to), to)
+ assert(isNonUnitValueTK(from) && isNonUnitValueTK(to), s"Cannot emit primitive conversion from $from to $to")
def pickOne(opcs: Array[Int]) {
val chosen = (to: @unchecked) match {
@@ -2643,10 +2642,8 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
}
if(from == to) { return }
- if((from == BOOL) || (to == BOOL)) {
- // the only conversion involving BOOL that is allowed is (BOOL -> BOOL)
- throw new Error("inconvertible types : " + from.toString() + " -> " + to.toString())
- }
+ // the only conversion involving BOOL that is allowed is (BOOL -> BOOL)
+ assert(from != BOOL && to != BOOL, "inconvertible types : $from -> $to")
if(from.isIntSizedType) { // BYTE, CHAR, SHORT, and INT. (we're done with BOOL already)
@@ -2810,8 +2807,7 @@ abstract class GenASM extends SubComponent with BytecodeWriters with GenJVMASM {
case Conversion(src, dst) =>
debuglog("Converting from: " + src + " to: " + dst)
- if (dst == BOOL) { println("Illegal conversion at: " + clasz + " at: " + pos.source + ":" + pos.line) }
- else { emitT2T(src, dst) }
+ emitT2T(src, dst)
case ArrayLength(_) => emit(Opcodes.ARRAYLENGTH)
View
1 test/files/run/t107.check
@@ -0,0 +1 @@
+1
View
8 test/files/run/t107.scala
@@ -0,0 +1,8 @@
+object Test {
+ def main(args : Array[String]) : Unit = {
+ var hash : Long = 0
+ val bytes = Array(1.toByte, 2.toByte, 3.toByte)
+ hash += bytes(0)
+ Console.println(hash)
+ }
+}

0 comments on commit 04b147e

Please sign in to comment.
Something went wrong with that request. Please try again.