Merge pull request #2177 from JamesIry/master_SI-7159

SI-7159 Distinguish between assignability and subtyping in TypeKinds

7 files changed, 47 insertions, 38 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 5438fd8590..65cd912dda 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/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) || (from == NullReference && to == NothingReference) // TODO why would we have null where we expect nothing?
       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,17 +1048,18 @@ 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()
-        // TODO why do we have this case? It's saying if we have a throwable and a non-throwable is expected then we should emit a cast? Why would we get here?
-        case ThrowableReference if !(ThrowableClass.tpe <:< to.toType) => ctx.bb.emit(CHECK_CAST(to)) // downcast throwables
-        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()
+        case _ if from isAssignabledTo 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")
       }
     }
 
diff --git a/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala b/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala
index fb1ef311d2..49f2d9859d 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/ICodeCheckers.scala
@@ -351,7 +351,7 @@ abstract class ICodeCheckers {
       def typeError(k1: TypeKind, k2: TypeKind) {
         icodeError("\n  expected: " + k1 + "\n     found: " + k2)
       }
-      def isSubtype(k1: TypeKind, k2: TypeKind) = (k1 <:< k2) || {
+      def isSubtype(k1: TypeKind, k2: TypeKind) = (k1 isAssignabledTo k2) || {
         import platform.isMaybeBoxed
 
         (k1, k2) match {
diff --git a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
index 6a392449e0..1875c8c914 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/TypeKinds.scala
@@ -88,11 +88,20 @@ 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
 
+    /**
+     * this is directly assignable to other if no coercion or
+     * casting is needed to convert this to other. It's a distinct
+     * relationship from <:< because on the JVM, BOOL, BYTE, CHAR, 
+     * SHORT need no coercion to INT even though JVM arrays
+     * are covariant, ARRAY[SHORT] is not a subtype of ARRAY[INT]
+     */
+    final def isAssignabledTo(other: TypeKind): Boolean = other match {
+      case INT  => this.isIntSizedType
+      case _    => this <:< other
+    }
+    
     /** Is this type a category 2 type in JVM terms? (ie, is it LONG or DOUBLE?) */
     def isWideType: Boolean = false
 
@@ -111,6 +120,7 @@ trait TypeKinds { self: ICodes =>
     override def toString = {
       this.getClass.getName stripSuffix "$" dropWhile (_ != '$') drop 1
     }
+    def <:<(other: TypeKind): Boolean = this eq other
   }
 
   /**
@@ -280,7 +290,7 @@ trait TypeKinds { self: ICodes =>
     }
 
     /** Checks subtyping relationship. */
-    override def <:<(other: TypeKind) = isNothingType || (other match {
+    def <:<(other: TypeKind) = isNothingType || (other match {
       case REFERENCE(cls2)  => cls.tpe <:< cls2.tpe
       case ARRAY(_)         => cls == NullClass
       case _                => false
@@ -318,7 +328,7 @@ trait TypeKinds { self: ICodes =>
 
     /** Array subtyping is covariant, as in Java. Necessary for checking
      *  code that interacts with Java. */
-    override def <:<(other: TypeKind) = other match {
+    def <:<(other: TypeKind) = other match {
       case ARRAY(elem2)                         => elem <:< elem2
       case REFERENCE(AnyRefClass | ObjectClass) => true // TODO: platform dependent!
       case _                                    => false
@@ -336,7 +346,7 @@ trait TypeKinds { self: ICodes =>
     }
 
     /** Checks subtyping relationship. */
-    override def <:<(other: TypeKind) = other match {
+    def <:<(other: TypeKind) = other match {
       case BOXED(`kind`)                        => true
       case REFERENCE(AnyRefClass | ObjectClass) => true // TODO: platform dependent!
       case _                                    => false
@@ -349,6 +359,7 @@ trait TypeKinds { self: ICodes =>
   */
   case object ConcatClass extends TypeKind {
     override def toString = "ConcatClass"
+    def <:<(other: TypeKind): Boolean = this eq other
 
     /**
      * Approximate `lub`. The common type of two references is
@@ -359,9 +370,6 @@ trait TypeKinds { self: ICodes =>
       case REFERENCE(_) => AnyRefReference
       case _            => uncomparable(other)
     }
-
-    /** Checks subtyping relationship. */
-    override def <:<(other: TypeKind) = this eq other
   }
 
   ////////////////// Conversions //////////////////////////////
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
index 91cb1857ac..3830b389ba 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenASM.scala
+++ b/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)
 
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
index 6e99129ee5..703724f003 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/ICodeReader.scala
@@ -930,7 +930,7 @@ abstract class ICodeReader extends ClassfileParser {
 
       locals.get(idx) match {
         case Some(ls) =>
-          val l = ls find { loc => loc._2 <:< kind }
+          val l = ls find { loc => loc._2 isAssignabledTo kind }
           l match {
             case Some((loc, _)) => loc
             case None =>
diff --git a/test/files/run/t107.check b/test/files/run/t107.check
new file mode 100644
index 0000000000..d00491fd7e
--- /dev/null
+++ b/test/files/run/t107.check
@@ -0,0 +1 @@
+1
diff --git a/test/files/run/t107.scala b/test/files/run/t107.scala
new file mode 100644
index 0000000000..ab1b289882
--- /dev/null
+++ b/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)
+  }
+}
\ No newline at end of file