Fixed issue #451 by updating structural method ...

Fixed issue #451 by updating structural method dispatch logic to work
with new arrays.

2 files changed, 139 insertions, 174 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/Definitions.scala b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
index cb9c681075..25c60c32b8 100644
--- a/src/compiler/scala/tools/nsc/symtab/Definitions.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Definitions.scala
@@ -125,6 +125,9 @@ trait Definitions {
       def SeqFactory = getMember(ScalaRunTimeModule, nme.Seq)
       def checkDefinedMethod = getMember(ScalaRunTimeModule, "checkDefined")
       def isArrayMethod = getMember(ScalaRunTimeModule, "isArray")
+      def arrayApplyMethod = getMember(ScalaRunTimeModule, "array_apply")
+      def arrayUpdateMethod = getMember(ScalaRunTimeModule, "array_update")
+      def arrayLengthMethod = getMember(ScalaRunTimeModule, "array_length")
 
     // classes with special meanings
     lazy val NotNullClass         = getClass("scala.NotNull")
@@ -185,6 +188,7 @@ trait Definitions {
     lazy val ArrayClass   = getClass("scala.Array")
       def Array_apply   = getMember(ArrayClass, nme.apply)
       def Array_update  = getMember(ArrayClass, nme.update)
+      def Array_length  = getMember(ArrayClass, nme.length)
     lazy val ArrayModule  = getModule("scala.Array")
       def ArrayModule_apply = getMember(ArrayModule, nme.apply)
 
diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
index d8aede6c37..dafec5e1c3 100644
--- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala
+++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
@@ -92,7 +92,7 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
        * '  getMethod("f", Array(classOf[b.type])).
        * '  invoke(a, Array(b))
        * plus all the necessary casting/boxing/etc. machinery required
-       * for type-compatibility (see fixResult and fixParams).
+       * for type-compatibility (see fixResult).
        *
        * USAGE CONTRACT:
        * There are a number of assumptions made on the way a dynamic apply
@@ -276,17 +276,6 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
 
         /* ### HANDLING METHODS NORMALLY COMPILED TO OPERATORS ### */
 
-        def mayRequirePrimitiveReplacement: Boolean = {
-
-          def isBoxed(sym: Symbol): Boolean =
-            ((sym isNonBottomSubClass BoxedNumberClass) ||
-              (!forMSIL && (sym isNonBottomSubClass BoxedCharacterClass)))
-
-          val sym = qual.tpe.typeSymbol
-          (sym == definitions.ObjectClass) || isBoxed(sym)
-
-        }
-
         val testForNumber: Tree     = (qual IS_OBJ BoxedNumberClass.tpe) OR (qual IS_OBJ BoxedCharacterClass.tpe)
         val testForBoolean: Tree    = (qual IS_OBJ BoxedBooleanClass.tpe)
         val testForNumberOrBoolean  = testForNumber OR testForBoolean
@@ -338,17 +327,6 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
           }
         }
 
-        def boxArray(t: Tree) = t
-/* was:
-          assert(!settings.newArrays.value)
-          val sym = currentOwner.newValue(ad.pos, mkTerm()) setInfo ObjectClass.tpe
-          BLOCK(
-            VAL(sym) === t,
-            IF (NULL ANY_== REF(sym)) THEN NULL ELSE gen.mkRuntimeCall(nme.boxArray, List(REF(sym)))
-          )
-        }
-*/
-
         /* ### BOXING PARAMS & UNBOXING RESULTS ### */
 
         /* Transforms the result of a reflective call (always an AnyRef) to
@@ -356,174 +334,157 @@ abstract class CleanUp extends Transform with ast.TreeDSL {
          * depends on the method's static return type.
          * - for units (void), the reflective call will return null: a new
          *   boxed unit is generated.
-         * - for arrays, the reflective call will return an unboxed array:
-         *   the resulting array is boxed.
          * - otherwise, the value is simply casted to the expected type. This
          *   is enough even for value (int et al.) values as the result of
          *   a dynamic call will box them as a side-effect. */
-        def fixResult(resType: Type)(tree: Tree): Tree = localTyper typed {
-          resType.typeSymbol match {
-            case UnitClass    => BLOCK(tree, REF(BoxedUnit_UNIT))
-            case ArrayClass   => boxArray(tree)
-            case ObjectClass  => tree
-            case _            => tree AS_ATTR resType
-          }
-        }
 
-        /* Transforms the parameters of a dynamic apply (always AnyRefs) to
-         * something compatible with reflective calls. The transformation depends
-         * on the method's static parameter types.
-         * - for (unboxed) arrays, the (non-null) value is tested for its erased
-         *   type. If it is a boxed array, the array is unboxed. If it is an
-         *   unboxed array, it is left alone - except that for varargs in structural
-         *   types to work properly, if the parameter is an Array and the parameter
-         *   type a Seq, it is routed through the boxed logic. */
-        def fixParams(params: List[Tree], paramTypes: List[Type]): List[Tree] = {
-          def doUnboxedArray(param: Tree, paramType: Type) = {
-            val sym = currentOwner.newValue(ad.pos, mkTerm()) setInfo ObjectClass.tpe
-            assert(paramType.typeArgs.length == 1)
-            val arrayType = paramType.typeArgs(0).normalize
-            lazy val unboxMethod = getMember(BoxedArrayClass, nme.unbox)
-
-            BLOCK(
-              VAL(sym) === param,
-              IF (NULL ANY_== REF(sym)) .
-                THEN (NULL) .
-              ELSE (
-                IF (REF(sym) IS_OBJ BoxedArrayClass.tpe) .
-                  THEN (((REF(sym) AS_ATTR BoxedArrayClass.tpe) DOT unboxMethod)(LIT(arrayType)))
-                ELSE
-                  REF(sym)
-              )
-            )
-          }
-          for ((param, paramType) <- params zip paramTypes) yield localTyper typed {
-            (param.tpe, paramType.typeSymbol) match {
-              case (_, ArrayClass)                        => doUnboxedArray(param, paramType)
-              case (TypeRef(_, ArrayClass, _), SeqClass)  => boxArray(param)  // ticket #1141
-              case _                                      => param
+        /* ### CALLING THE APPLY ### */
+        def callAsReflective(paramTypes: List[Type], resType: Type, structResType: Type): Tree = localTyper typed {
+          def fixResult(tree: Tree): Tree = localTyper typed {
+            structResType.typeSymbol match {
+              case UnitClass    => BLOCK(tree, REF(BoxedUnit_UNIT))
+              case ObjectClass  => tree
+              case _            => tree AS_ATTR structResType
             }
           }
-        }
-
-        /* ### CALLING THE APPLY -> one for operators (see above), one for normal methods ### */
-        def callAsOperator(paramTypes: List[Type], resType: Type): Tree = localTyper typed {
-          def default = callAsMethod(paramTypes, resType)
-          // This is more indirect than it should be (and I don't think it spots every
-          // case either) but it's the most direct way I could see to address ticket #1110
-          // given the information available from this vantage.
-          def useOperator =
-            (getPrimitiveReplacementForStructuralCall isDefinedAt ad.symbol.name) &&
-            ((resType :: paramTypes) forall (x => isValueClass(x.typeSymbol)))
-
-          if (useOperator) {
-            val (operator, test)  = getPrimitiveReplacementForStructuralCall(ad.symbol.name)
-            def args              = qual :: fixParams(params, paramTypes)
-
-            IF (test) THEN (REF(operator) APPLY args) ELSE default
+          val qualSym = qual.tpe.typeSymbol
+          val methSym = ad.symbol
+          def defaultCall = {
+            // reflective method call machinery
+            val invokeName  = MethodClass.tpe member nme.invoke_    // reflect.Method.invoke(...)
+            def cache       = REF(reflectiveMethodCache(ad.symbol.name.toString, paramTypes)) // cache Symbol
+            def lookup      = Apply(cache, List(qual GETCLASS))     // get Method object from cache
+            def args        = ArrayValue(TypeTree(ObjectClass.tpe), params)  // args for invocation
+            def invocation  = (lookup DOT invokeName)(qual, args)   // .invoke(qual, ...)
+
+            // exception catching machinery
+            val invokeExc   = currentOwner.newValue(ad.pos, mkTerm()) setInfo InvocationTargetExceptionClass.tpe
+            def catchVar    = Bind(invokeExc, Typed(Ident(nme.WILDCARD), TypeTree(InvocationTargetExceptionClass.tpe)))
+            def catchBody   = Throw(Apply(Select(Ident(invokeExc), nme.getCause), Nil))
+
+            // try { method.invoke } catch { case e: InvocationTargetExceptionClass => throw e.getCause() }
+            TRY (invocation) CATCH { CASE (catchVar) ==> catchBody } ENDTRY
           }
-          else default
-        }
-
-        def callAsMethod(paramTypes: List[Type], resType: Type): Tree = localTyper.typed {
-          // reflective method call machinery
-          val invokeName  = MethodClass.tpe member nme.invoke_    // reflect.Method.invoke(...)
-          def cache       = REF(reflectiveMethodCache(ad.symbol.name.toString, paramTypes)) // cache Symbol
-          def lookup      = Apply(cache, List(qual GETCLASS))     // get Method object from cache
-          def args        = ArrayValue(TypeTree(ObjectClass.tpe), fixParams(params, paramTypes))  // args for invocation
-          def invocation  = (lookup DOT invokeName)(qual, args)   // .invoke(qual, ...)
-
-          // exception catching machinery
-          val invokeExc   = currentOwner.newValue(ad.pos, mkTerm()) setInfo InvocationTargetExceptionClass.tpe
-          def catchVar    = Bind(invokeExc, Typed(Ident(nme.WILDCARD), TypeTree(InvocationTargetExceptionClass.tpe)))
-          def catchBody   = Throw(Apply(Select(Ident(invokeExc), nme.getCause), Nil))
-
-          // try { method.invoke } catch { case e: InvocationTargetExceptionClass => throw e.getCause() }
-          TRY (invocation) CATCH { CASE (catchVar) ==> catchBody } ENDTRY
+          def useValueOperator = {
+            def isBoxed(qualSym: Symbol): Boolean =
+              (qualSym isNonBottomSubClass BoxedNumberClass) ||
+              (!forMSIL && (qualSym isNonBottomSubClass BoxedCharacterClass))
+            ((qualSym == definitions.ObjectClass) || isBoxed(qualSym)) && // may be a boxed value class
+            (getPrimitiveReplacementForStructuralCall isDefinedAt methSym.name) &&
+            ((resType :: paramTypes) forall (x => isValueClass(x.typeSymbol))) // issue #1110
+          }
+          def useArrayOperator =
+            ((qualSym == definitions.ObjectClass) || (qualSym == definitions.ArrayClass)) &&
+            ((methSym.name == nme.length) || (methSym.name == nme.update) || (methSym.name == nme.apply))
+          val callCode = if (useValueOperator) {
+            val (operator, test)  = getPrimitiveReplacementForStructuralCall(methSym.name)
+            def args              = qual :: params
+            fixResult((IF (test) THEN (REF(operator) APPLY args) ELSE defaultCall))
+          }
+          else if (useArrayOperator) {
+            val args = qual :: params
+            val operatorCall = // what follows is incredibly ugly. this dirty fix should be deal with at the next cleanup of cleanup.
+              if (methSym.name == nme.length)
+                (REF(boxMethod(IntClass)) APPLY (REF(arrayLengthMethod) APPLY args))
+              else if (methSym.name == nme.update)
+                (REF(arrayUpdateMethod) APPLY List(args(0), (REF(unboxMethod(IntClass)) APPLY args(1)), args(2)))
+              else
+                (REF(arrayApplyMethod) APPLY List(args(0), (REF(unboxMethod(IntClass)) APPLY args(1))))
+            (IF (qual IS_OBJ arrayType(ObjectClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(ByteClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(ShortClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(IntClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(LongClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(FloatClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(DoubleClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(CharClass.tpe)) THEN operatorCall
+            ELSE (IF (qual IS_OBJ arrayType(BooleanClass.tpe)) THEN operatorCall
+            ELSE fixResult(defaultCall)
+            )))))))))
+          }
+          else fixResult(defaultCall)
+          localTyper.typed(callCode)
         }
 
         def getClass(q: Tree): Tree = (q DOT nme.getClass_)()
 
-      if (settings.refinementMethodDispatch.value == "invoke-dynamic") {
-/*        val guardCallSite: Tree = {
-          val cachedClass = addStaticVariableToClass("cachedClass", definitions.ClassClass.tpe, EmptyTree)
-          val tmpVar = currentOwner.newVariable(ad.pos, unit.fresh.newName(ad.pos, "x")).setInfo(definitions.AnyRefClass.tpe)
-          atPos(ad.pos)(Block(List(
-            ValDef(tmpVar, transform(qual))),
-            If(Apply(Select(gen.mkAttributedRef(cachedClass), nme.EQ), List(getClass(Ident(tmpVar)))),
-               Block(List(Assign(gen.mkAttributedRef(cachedClass), getClass(Ident(tmpVar)))),
-                     treeCopy.ApplyDynamic(ad, Ident(tmpVar), transformTrees(params))),
-               EmptyTree)))
-        }
-        //println(guardCallSite)
+        if (settings.refinementMethodDispatch.value == "invoke-dynamic") {
+/*          val guardCallSite: Tree = {
+            val cachedClass = addStaticVariableToClass("cachedClass", definitions.ClassClass.tpe, EmptyTree)
+            val tmpVar = currentOwner.newVariable(ad.pos, unit.fresh.newName(ad.pos, "x")).setInfo(definitions.AnyRefClass.tpe)
+            atPos(ad.pos)(Block(List(
+              ValDef(tmpVar, transform(qual))),
+              If(Apply(Select(gen.mkAttributedRef(cachedClass), nme.EQ), List(getClass(Ident(tmpVar)))),
+                 Block(List(Assign(gen.mkAttributedRef(cachedClass), getClass(Ident(tmpVar)))),
+                       treeCopy.ApplyDynamic(ad, Ident(tmpVar), transformTrees(params))),
+                 EmptyTree)))
+          }
+          //println(guardCallSite)
 */
-        localTyper.typed(treeCopy.ApplyDynamic(ad, transform(qual), transformTrees(params)))
-      } else {
-
-        /* ### BODY OF THE TRANSFORMATION -> remember we're in case ad@ApplyDynamic(qual, params) ### */
-
-        /* This creates the tree that does the reflective call (see general comment
-         * on the apply-dynamic tree for its format). This tree is simply composed
-         * of three succesive calls, first to getClass on the callee, then to
-         * getMethod on the classs, then to invoke on the method.
-         * - getMethod needs an array of classes for choosing one amongst many
-         *   overloaded versions of the method. This is provided by paramTypeClasses
-         *   and must be done on the static type as Scala's dispatching is static on
-         *   the parameters.
-         * - invoke needs an array of AnyRefs that are the method's arguments. The
-         *   erasure phase guarantees that any parameter passed to a dynamic apply
-         *   is compatible (through boxing). Boxed ints et al. is what invoke expects
-         *   when the applied method expects ints, hence no change needed there.
-         *   On the other hand, arrays must be dealt with as they must be entered
-         *   unboxed in the parameter array of invoke. fixParams is responsible for
-         *   that.
-         * - in the end, the result of invoke must be fixed, again to deal with arrays.
-         *   This is provided by fixResult. fixResult will cast the invocation's result
-         *   to the method's return type, which is generally ok, except when this type
-         *   is a value type (int et al.) in which case it must cast to the boxed version
-         *   because invoke only returns object and erasure made sure the result is
-         *   expected to be an AnyRef. */
-        val t: Tree = ad.symbol.tpe match {
-          case MethodType(mparams, resType) =>
-            assert(params.length == mparams.length)
-            typedPos {
-              val sym = currentOwner.newValue(ad.pos, mkTerm("qual")) setInfo qual0.tpe
-              qual = REF(sym)
-
-              def resTypeForFix = if (isValueClass(resType.typeSymbol)) boxedClass(resType.typeSymbol).tpe else resType
-              def call          = if (mayRequirePrimitiveReplacement) (callAsOperator _) else (callAsMethod _)
-              BLOCK(
-                VAL(sym) === qual0,
-                fixResult(resTypeForFix)(call(mparams map (_.tpe), resType))
-              )
-            }
+          localTyper.typed(treeCopy.ApplyDynamic(ad, transform(qual), transformTrees(params)))
         }
-
-        /* For testing purposes, the dynamic application's condition
-         * can be printed-out in great detail. Remove? */
-        if (settings.debug.value) {
-          def paramsToString(xs: Any*) = xs map (_.toString) mkString ", "
-          val mstr = ad.symbol.tpe match {
+        else {
+
+          /* ### BODY OF THE TRANSFORMATION -> remember we're in case ad@ApplyDynamic(qual, params) ### */
+
+          /* This creates the tree that does the reflective call (see general comment
+           * on the apply-dynamic tree for its format). This tree is simply composed
+           * of three succesive calls, first to getClass on the callee, then to
+           * getMethod on the classs, then to invoke on the method.
+           * - getMethod needs an array of classes for choosing one amongst many
+           *   overloaded versions of the method. This is provided by paramTypeClasses
+           *   and must be done on the static type as Scala's dispatching is static on
+           *   the parameters.
+           * - invoke needs an array of AnyRefs that are the method's arguments. The
+           *   erasure phase guarantees that any parameter passed to a dynamic apply
+           *   is compatible (through boxing). Boxed ints et al. is what invoke expects
+           *   when the applied method expects ints, hence no change needed there.
+           * - in the end, the result of invoke must be fixed, again to deal with arrays.
+           *   This is provided by fixResult. fixResult will cast the invocation's result
+           *   to the method's return type, which is generally ok, except when this type
+           *   is a value type (int et al.) in which case it must cast to the boxed version
+           *   because invoke only returns object and erasure made sure the result is
+           *   expected to be an AnyRef. */
+          val t: Tree = ad.symbol.tpe match {
             case MethodType(mparams, resType) =>
-              """|  with
-                 |  - declared parameter types: '%s'
-                 |  - passed argument types:    '%s'
-                 |  - result type:              '%s'""" .
-                stripMargin.format(
-                   paramsToString(mparams),
-                   paramsToString(params),
-                   resType.toString
+              assert(params.length == mparams.length)
+              typedPos {
+                val sym = currentOwner.newValue(ad.pos, mkTerm("qual")) setInfo qual0.tpe
+                qual = REF(sym)
+
+                def structResType = if (isValueClass(resType.typeSymbol)) boxedClass(resType.typeSymbol).tpe else resType
+                BLOCK(
+                  VAL(sym) === qual0,
+                  callAsReflective(mparams map (_.tpe), resType, structResType)
                 )
-            case _ => ""
+              }
+          }
+
+          /* For testing purposes, the dynamic application's condition
+           * can be printed-out in great detail. Remove? */
+          if (settings.debug.value) {
+            def paramsToString(xs: Any*) = xs map (_.toString) mkString ", "
+            val mstr = ad.symbol.tpe match {
+              case MethodType(mparams, resType) =>
+                """|  with
+                   |  - declared parameter types: '%s'
+                   |  - passed argument types:    '%s'
+                   |  - result type:              '%s'""" .
+                  stripMargin.format(
+                     paramsToString(mparams),
+                     paramsToString(params),
+                     resType.toString
+                  )
+              case _ => ""
+            }
+            Console.printf("""Dynamically application '%s.%s(%s)' %s - resulting code: '%s'""",
+                List(qual, ad.symbol.name, paramsToString(params), mstr, t) map (_.toString) : _*
+            )
           }
-          Console.printf("""Dynamically application '%s.%s(%s)' %s - resulting code: '%s'""",
-              List(qual, ad.symbol.name, paramsToString(params), mstr, t) map (_.toString) : _*
-          )
-        }
 
-        /* We return the dynamic call tree, after making sure no other
-         * clean-up transformation are to be applied on it. */
-        transform(t)
+          /* We return the dynamic call tree, after making sure no other
+           * clean-up transformation are to be applied on it. */
+          transform(t)
         }
         /* ### END OF DYNAMIC APPLY TRANSFORM ### */