There was a massive 30+ line cut and paste betw...

There was a massive 30+ line cut and paste between isPlausiblyCompatible
and normSubType. Since I already painstakingly optimized the former, it
was incredibly depressing to discover an exact copy of the "before" code
pasted into a method later in the same file. I can't begin to convey how
much unnecessary difficulty this sort of thing brings for us. Friends,
romans, countrymen, put down your ctrl-Vs. Review by anyone who might be
tempted to perform similar cutting and pasting in the future.

1 files changed, 79 insertions, 79 deletions

diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
index 19706e119c..20136d49b9 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
@@ -307,90 +307,86 @@ trait Infer {
         }
       }
 
-    def isPlausiblyCompatible(tp: Type, pt: Type): Boolean = tp match {
-      case PolyType(_, restpe) =>
-        isPlausiblyCompatible(restpe, pt)
-      case ExistentialType(tparams, qtpe) =>
-        isPlausiblyCompatible(qtpe, pt)
-      case mt @ MethodType(params, restpe) =>
-        if (mt.isImplicit) isPlausiblyCompatible(restpe, pt)
-        else pt match {
-          case TypeRef(pre, sym, args) =>
-            if (sym.isAliasType) {
-              isPlausiblyCompatible(tp, pt.dealias)
-            } else if (sym.isAbstractType) {
-              isPlausiblyCompatible(tp, pt.bounds.lo)
-            } else {
-              val len = args.length - 1
-              hasLength(params, len) &&
-              sym == FunctionClass(len) && {
-                val ps = mt.paramTypes.iterator
-                val as = args.iterator
-                while (ps.hasNext && as.hasNext) {
-                  if (!isPlausiblySubType(as.next, ps.next))
-                    return false
-                }
-                ps.isEmpty && as.hasNext && {
-                  val lastArg = as.next
-                  as.isEmpty && isPlausiblySubType(restpe, lastArg)
-                }
-              }
+    /** Capturing the overlap between isPlausiblyCompatible and normSubType.
+     *  This is a faithful translation of the code which was there, but it
+     *  seems likely the methods are intended to be even more similar than
+     *  they are: perhaps someone more familiar with the intentional distinctions
+     *  can examine the now much smaller concrete implementations below.
+     */
+    abstract class CompatibilityChecker {
+      def resultTypeCheck(restpe: Type, arg: Type): Boolean
+      def argumentCheck(arg: Type, param: Type): Boolean
+      def lastChanceCheck(tp: Type, pt: Type): Boolean
+
+      final def mtcheck(tp: MethodType, pt: TypeRef): Boolean = {
+        val MethodType(params, restpe) = tp
+        val TypeRef(pre, sym, args) = pt
+
+        if (sym.isAliasType) apply(tp, pt.dealias)
+        else if (sym.isAbstractType) apply(tp, pt.bounds.lo)
+        else {
+          val len = args.length - 1
+          hasLength(params, len) &&
+          sym == FunctionClass(len) && {
+            val ps = params.iterator
+            val as = args.iterator
+            while (ps.hasNext && as.hasNext) {
+              if (!argumentCheck(as.next, ps.next.tpe))
+                return false
             }
-          case _ =>
-            false
+            ps.isEmpty && as.hasNext && {
+              val lastArg = as.next
+              as.isEmpty && resultTypeCheck(restpe, lastArg)
+            }
+          }
         }
-      case _ =>
-        isPlausiblySubType(tp, pt)
+      }
+
+      def apply(tp: Type, pt: Type): Boolean = tp match {
+        case mt @ MethodType(_, restpe) =>
+          if (mt.isImplicit)
+            apply(restpe, pt)
+          else pt match {
+            case tr: TypeRef  => mtcheck(mt, tr)
+            case _            => lastChanceCheck(tp, pt)
+          }
+        case PolyType(_, restpe)        => apply(restpe, pt)
+        case ExistentialType(_, qtpe)   => apply(qtpe, pt)
+        case _                          => argumentCheck(tp, pt)
+      }
     }
 
-    private def isPlausiblySubType(tp1: Type, tp2: Type): Boolean = tp1 match {
-      case TypeRef(_, sym1, _) =>
-        if (sym1.isAliasType) isPlausiblySubType(tp1.dealias, tp2)
-        else if (!sym1.isClass) true
-        else tp2 match {
-          case TypeRef(_, sym2, _) =>
-            if (sym2.isAliasType) isPlausiblySubType(tp1, tp2.dealias)
-            else !sym2.isClass || (sym1 isSubClass sym2) || isNumericSubClass(sym1, sym2)
-          case _ =>
-            true
-        }
-      case _ =>
-        true
+    object isPlausiblyCompatible extends CompatibilityChecker {
+      def resultTypeCheck(restpe: Type, arg: Type) = isPlausiblySubType(restpe, arg)
+      def argumentCheck(arg: Type, param: Type)    = isPlausiblySubType(arg, param)
+      def lastChanceCheck(tp: Type, pt: Type)      = false
+    }
+    object normSubType extends CompatibilityChecker {
+      def resultTypeCheck(restpe: Type, arg: Type) = normSubType(restpe, arg)
+      def argumentCheck(arg: Type, param: Type)    = arg <:< param
+      def lastChanceCheck(tp: Type, pt: Type)      = tp <:< pt
+
+      override def apply(tp: Type, pt: Type): Boolean = tp match {
+        case PolyType(tparams, restpe) => tparams.isEmpty && normSubType(restpe, pt)
+        case ExistentialType(_, _)     => normalize(tp) <:< pt
+        case _                         => super.apply(tp, pt)
+      }
     }
 
-    final def normSubType(tp: Type, pt: Type): Boolean = tp match {
-      case mt @ MethodType(params, restpe) =>
-        if (mt.isImplicit) normSubType(restpe, pt)
-        else  pt match {
-          case TypeRef(pre, sym, args) =>
-            if (sym.isAliasType) {
-              normSubType(tp, pt.dealias)
-            } else if (sym.isAbstractType) {
-              normSubType(tp, pt.bounds.lo)
-            } else {
-              val l = args.length - 1
-              l == params.length &&
-              sym == FunctionClass(l) && {
-                var curargs = args
-                var curparams = params
-                while (curparams.nonEmpty) {
-                  if (!(curargs.head <:< curparams.head.tpe))
-                    return false
-                  curargs = curargs.tail
-                  curparams = curparams.tail
-                }
-                normSubType(restpe, curargs.head)
-              }
-            }
-          case _ =>
-            tp <:< pt
-        }
-      case PolyType(List(), restpe) => // nullary method type
-        normSubType(restpe, pt)
-      case ExistentialType(tparams, qtpe) =>
-        normalize(tp) <:< pt
-      case _ =>
-        tp <:< pt
+    /** This expresses more cleanly in the negative: there's a linear path
+     *  to a final true or false.
+     */
+    private def isPlausiblySubType(tp1: Type, tp2: Type) = !isImpossibleSubType(tp1, tp2)
+    private def isImpossibleSubType(tp1: Type, tp2: Type) = {
+      (tp1.dealias, tp2.dealias) match {
+        case (TypeRef(_, sym1, _), TypeRef(_, sym2, _)) =>
+           sym1.isClass &&
+           sym2.isClass &&
+          !(sym1 isSubClass sym2) &&
+          !(sym1 isNumericSubClass sym2)
+        case _ =>
+          false
+      }
     }
 
     def isCompatible(tp: Type, pt: Type): Boolean = {
@@ -412,6 +408,10 @@ trait Infer {
     def isConservativelyCompatible(tp: Type, pt: Type): Boolean =
       context.withImplicitsDisabled(isWeaklyCompatible(tp, pt))
 
+    /** This is overridden in the Typer.infer with some logic, but since
+     *  that's the only place in the compiler an Inferencer is ever created,
+     *  I suggest this should either be abstract or have the implementation.
+     */
     def isCoercible(tp: Type, pt: Type): Boolean = false
 
     /* -- Type instantiation------------------------------------------------ */
@@ -610,7 +610,7 @@ trait Infer {
                 ", argtpes = "+argtpes+
                 ", pt = "+pt+
                 ", tvars = "+tvars+" "+(tvars map (_.constr)))
-      if (formals.length != argtpes.length) {
+      if (!sameLength(formals, argtpes)) {
         throw new NoInstance("parameter lists differ in length")
       }