Merge pull request #1468 from paulp/issue/6482

SI-6482 preserve bounds on tparams of value class

4 files changed, 150 insertions, 65 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala
index 14e5e352f7..5aa7a804fd 100644
--- a/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExtensionMethods.scala
@@ -75,32 +75,41 @@ abstract class ExtensionMethods extends Transform with TypingTransformers {
     matching.head
   }
 
+  /** Recognize a MethodType which represents an extension method.
+   *
+   *  It may have a curried parameter list with the `$this` alone in the first
+   *  parameter list, in which case that parameter list is dropped.  Or, since
+   *  the curried lists disappear during uncurry, it may have a single parameter
+   *  list with `$this` as the first parameter, in which case that parameter is
+   *  removed from the list.
+   */
+  object ExtensionMethodType {
+    def unapply(tp: Type) = tp match {
+      case MethodType(thiz :: rest, restpe) if thiz.name == nme.SELF =>
+        Some( if (rest.isEmpty) restpe else MethodType(rest, restpe) )
+      case _ =>
+        None
+    }
+  }
+
   /** This method removes the `$this` argument from the parameter list a method.
-   *  
+   *
    *  A method may be a `PolyType`, in which case we tear out the `$this` and the class
-   *  type params from its nested `MethodType`.
-   *  It may be a `MethodType`, either with a curried parameter list in which the first argument
-   *  is a `$this` - we just return the rest of the list.
-   *  This means that the corresponding symbol was generated during `extmethods`.
-   *  
-   *  It may also be a `MethodType` in which the `$this` does not appear in a curried parameter list.
-   *  The curried lists disappear during `uncurry`, and the methods may be duplicated afterwards,
-   *  for instance, during `specialize`.
-   *  In this case, the first argument is `$this` and we just get rid of it.
+   *  type params from its nested `MethodType`.  Or it may be a MethodType, as
+   *  described at the ExtensionMethodType extractor.
    */
   private def normalize(stpe: Type, clazz: Symbol): Type = stpe match {
     case PolyType(tparams, restpe) =>
-      GenPolyType(tparams dropRight clazz.typeParams.length, normalize(restpe.substSym(tparams takeRight clazz.typeParams.length, clazz.typeParams), clazz))
-    case MethodType(List(thiz), restpe) if thiz.name == nme.SELF =>
-      restpe
-    case MethodType(tparams, restpe) =>
-      MethodType(tparams.drop(1), restpe)
+      // method type parameters, class type parameters
+      val (mtparams, ctparams) = tparams splitAt (tparams.length - clazz.typeParams.length)
+      GenPolyType(mtparams, normalize(restpe.substSym(ctparams, clazz.typeParams), clazz))
+    case ExtensionMethodType(etpe) =>
+      etpe
     case _ =>
       stpe
   }
 
   class Extender(unit: CompilationUnit) extends TypingTransformer(unit) {
-
     private val extensionDefs = mutable.Map[Symbol, mutable.ListBuffer[Tree]]()
 
     def checkNonCyclic(pos: Position, seen: Set[Symbol], clazz: Symbol): Unit =
@@ -111,27 +120,51 @@ abstract class ExtensionMethods extends Transform with TypingTransformers {
         if (unboxed.isDerivedValueClass) checkNonCyclic(pos, seen + clazz, unboxed)
       }
 
+   /** We will need to clone the info of the original method (which obtains clones
+    *  of the method type parameters), clone the type parameters of the value class,
+    *  and create a new polymethod with the union of all those type parameters, with
+    *  their infos adjusted to be consistent with their new home. Example:
+    *
+    *    class Foo[+A <: AnyRef](val xs: List[A]) extends AnyVal {
+    *      def baz[B >: A](x: B): List[B] = x :: xs
+    *      // baz has to be transformed into this extension method, where
+    *      // A is cloned from class Foo and  B is cloned from method baz:
+    *      // def extension$baz[B >: A <: Any, A >: Nothing <: AnyRef]($this: Foo[A])(x: B): List[B]
+    *    }
+    *
+    *  TODO: factor out the logic for consolidating type parameters from a class
+    *  and a method for re-use elsewhere, because nobody will get this right without
+    *  some higher level facilities.
+    */
     def extensionMethInfo(extensionMeth: Symbol, origInfo: Type, clazz: Symbol): Type = {
-      // No variance for method type parameters
-      var newTypeParams = cloneSymbolsAtOwner(clazz.typeParams, extensionMeth) map (_ resetFlag COVARIANT | CONTRAVARIANT)
-      val thisParamType = appliedType(clazz.typeConstructor, newTypeParams map (_.tpeHK))
-      val thisParam     = extensionMeth.newValueParameter(nme.SELF, extensionMeth.pos) setInfo thisParamType
-      def transform(clonedType: Type): Type = clonedType match {
-        case MethodType(params, restpe) =>
-          // I assume it was a bug that this was dropping params... [Martin]: No, it wasn't; it's curried.
-          MethodType(List(thisParam), clonedType)
-        case NullaryMethodType(restpe) =>
-          MethodType(List(thisParam), restpe)
-      }
-      val GenPolyType(tparams, restpe) = origInfo cloneInfo extensionMeth
-      GenPolyType(tparams ::: newTypeParams, transform(restpe) substSym (clazz.typeParams, newTypeParams))
-    }
+      val GenPolyType(tparamsFromMethod, methodResult) = origInfo cloneInfo extensionMeth
+      // Start with the class type parameters - clones will be method type parameters
+      // so must drop their variance.
+      val tparamsFromClass = cloneSymbolsAtOwner(clazz.typeParams, extensionMeth) map (_ resetFlag COVARIANT | CONTRAVARIANT)
+      def fix(tp: Type) = tp.substSym(clazz.typeParams, tparamsFromClass)
 
-    private def allParams(tpe: Type): List[Symbol] = tpe match {
-      case MethodType(params, res) => params ::: allParams(res)
-      case _ => List()
+      val thisParamType = appliedType(clazz, tparamsFromClass map (_.tpeHK): _*)
+      val thisParam     = extensionMeth.newValueParameter(nme.SELF, extensionMeth.pos) setInfo thisParamType
+      val resultType    = MethodType(List(thisParam), dropNullaryMethod(methodResult))
+
+      // We can't substitute symbols on the entire polytype because we
+      // need to modify the bounds of the cloned type parameters, but we
+      // don't want to substitute for the cloned type parameters themselves.
+      val tparams = tparamsFromMethod ::: tparamsFromClass
+      GenPolyType(tparams map (_ modifyInfo fix), fix(resultType))
+
+      // For reference, calling fix on the GenPolyType plays out like this:
+      // error: scala.reflect.internal.Types$TypeError: type arguments [B#7344,A#6966]
+      // do not conform to method extension$baz#16148's type parameter bounds
+      //
+      // And the difference is visible here.  See how B is bounded from below by A#16149
+      // in both cases, but in the failing case, the other type parameter has turned into
+      // a different A. (What is that A? It is a clone of the original A created in
+      // SubstMap during the call to substSym, but I am not clear on all the particulars.)
+      //
+      //  bad: [B#16154 >: A#16149, A#16155 <: AnyRef#2189]($this#16156: Foo#6965[A#16155])(x#16157: B#16154)List#2457[B#16154]
+      // good: [B#16151 >: A#16149, A#16149 <: AnyRef#2189]($this#16150: Foo#6965[A#16149])(x#16153: B#16151)List#2457[B#16151]
     }
-
     override def transform(tree: Tree): Tree = {
       tree match {
         case Template(_, _, _) =>
@@ -146,39 +179,58 @@ abstract class ExtensionMethods extends Transform with TypingTransformers {
             super.transform(tree)
           } else tree
         case DefDef(_, _, tparams, vparamss, _, rhs) if tree.symbol.isMethodWithExtension =>
-          val companion = currentOwner.companionModule
-          val origMeth = tree.symbol
-          val extensionName = extensionNames(origMeth).head
-          val extensionMeth = companion.moduleClass.newMethod(extensionName, origMeth.pos, origMeth.flags & ~OVERRIDE & ~PROTECTED | FINAL)
-            .setAnnotations(origMeth.annotations)
-          companion.info.decls.enter(extensionMeth)
-          val newInfo = extensionMethInfo(extensionMeth, origMeth.info, currentOwner)
+          val origMeth      = tree.symbol
+          val origThis      = currentOwner
+          val origTpeParams = tparams.map(_.symbol) ::: origThis.typeParams   // method type params ++ class type params
+          val origParams    = vparamss.flatten map (_.symbol)
+          val companion     = origThis.companionModule
+
+          def makeExtensionMethodSymbol = {
+            val extensionName = extensionNames(origMeth).head
+            val extensionMeth = (
+              companion.moduleClass.newMethod(extensionName, origMeth.pos, origMeth.flags & ~OVERRIDE & ~PROTECTED | FINAL)
+                setAnnotations origMeth.annotations
+            )
+            companion.info.decls.enter(extensionMeth)
+          }
+
+          val extensionMeth = makeExtensionMethodSymbol
+          val newInfo       = extensionMethInfo(extensionMeth, origMeth.info, origThis)
           extensionMeth setInfo newInfo
-          log("Value class %s spawns extension method.\n  Old: %s\n  New: %s".format(
-            currentOwner,
-            origMeth.defString,
-            extensionMeth.defString)) // extensionMeth.defStringSeenAs(origInfo
-
-          def thisParamRef = gen.mkAttributedIdent(extensionMeth.info.params.head setPos extensionMeth.pos)
-          val GenPolyType(extensionTpeParams, extensionMono) = extensionMeth.info
-          val origTpeParams = (tparams map (_.symbol)) ::: currentOwner.typeParams
-          val extensionBody = rhs
+
+          log(s"Value class $origThis spawns extension method.\n  Old: ${origMeth.defString}\n  New: ${extensionMeth.defString}")
+
+          val GenPolyType(extensionTpeParams, MethodType(thiz :: Nil, extensionMono)) = newInfo
+          val extensionParams = allParameters(extensionMono)
+          val extensionThis   = gen.mkAttributedIdent(thiz setPos extensionMeth.pos)
+
+          val extensionBody = (
+            rhs
               .substituteSymbols(origTpeParams, extensionTpeParams)
-              .substituteSymbols(vparamss.flatten map (_.symbol), allParams(extensionMono).tail)
-              .substituteThis(currentOwner, thisParamRef)
-              .changeOwner((origMeth, extensionMeth))
-          extensionDefs(companion) += atPos(tree.pos) { DefDef(extensionMeth, extensionBody) }
-          val extensionCallPrefix = Apply(
-              gen.mkTypeApply(gen.mkAttributedRef(companion), extensionMeth, origTpeParams map (_.tpeHK)),
-              List(This(currentOwner)))
-          val extensionCall = atOwner(origMeth) {
-            localTyper.typedPos(rhs.pos) {
-              (extensionCallPrefix /: vparamss) {
-                case (fn, params) => Apply(fn, params map (param => Ident(param.symbol)))
-              }
-            }
-          }
-          deriveDefDef(tree)(_ => extensionCall)
+              .substituteSymbols(origParams, extensionParams)
+              .substituteThis(origThis, extensionThis)
+              .changeOwner(origMeth -> extensionMeth)
+          )
+
+          // Record the extension method ( FIXME: because... ? )
+          extensionDefs(companion) += atPos(tree.pos)(DefDef(extensionMeth, extensionBody))
+
+          // These three lines are assembling Foo.bar$extension[T1, T2, ...]($this)
+          // which leaves the actual argument application for extensionCall.
+          val sel        = Select(gen.mkAttributedRef(companion), extensionMeth)
+          val targs      = origTpeParams map (_.tpeHK)
+          val callPrefix = gen.mkMethodCall(sel, targs, This(origThis) :: Nil)
+
+          // Apply all the argument lists.
+          deriveDefDef(tree)(_ =>
+            atOwner(origMeth)(
+              localTyper.typedPos(rhs.pos)(
+                (callPrefix /: vparamss) {
+                  case (fn, params) => Apply(fn, params map (param => Ident(param.symbol)))
+                }
+              )
+            )
+          )
         case _ =>
           super.transform(tree)
       }
diff --git a/src/compiler/scala/tools/nsc/typechecker/Infer.scala b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
index e18310cf84..61e4fb86a2 100644
--- a/src/compiler/scala/tools/nsc/typechecker/Infer.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/Infer.scala
@@ -410,8 +410,19 @@ trait Infer extends Checkable {
 
     /** Like weakly compatible but don't apply any implicit conversions yet.
      *  Used when comparing the result type of a method with its prototype.
+     *
      *  [Martin] I think Infer is also created by Erasure, with the default
      *  implementation of isCoercible
+     *  [Paulp] (Assuming the above must refer to my comment on isCoercible)
+     *  Nope, I examined every occurrence of Inferencer in trunk.  It
+     *  appears twice as a self-type, once at its definition, and once
+     *  where it is instantiated in Typers.  There are no others.
+     *
+         % ack -A0 -B0 --no-filename '\bInferencer\b' src
+             self: Inferencer =>
+             self: Inferencer =>
+           class Inferencer(context: Context) extends InferencerContextErrors with InferCheckable {
+             val infer = new Inferencer(context0) {
      */
     def isConservativelyCompatible(tp: Type, pt: Type): Boolean =
       context.withImplicitsDisabled(isWeaklyCompatible(tp, pt))
diff --git a/src/reflect/scala/reflect/internal/Definitions.scala b/src/reflect/scala/reflect/internal/Definitions.scala
index 16ec95dafa..35acc2bcce 100644
--- a/src/reflect/scala/reflect/internal/Definitions.scala
+++ b/src/reflect/scala/reflect/internal/Definitions.scala
@@ -670,6 +670,11 @@ trait Definitions extends api.StandardDefinitions {
       case _        => Nil
     }
 
+    def dropNullaryMethod(tp: Type) = tp match {
+      case NullaryMethodType(restpe) => restpe
+      case _                         => tp
+    }
+
     def unapplyUnwrap(tpe:Type) = tpe.finalResultType.normalize match {
       case RefinedType(p :: _, _) => p.normalize
       case tp                     => tp
@@ -863,6 +868,12 @@ trait Definitions extends api.StandardDefinitions {
         removeRedundantObjects(parents)
     }
 
+    /** Flatten curried parameter lists of a method type. */
+    def allParameters(tpe: Type): List[Symbol] = tpe match {
+      case MethodType(params, res) => params ::: allParameters(res)
+      case _                       => Nil
+    }
+
     def typeStringNoPackage(tp: Type) =
       "" + tp stripPrefix tp.typeSymbol.enclosingPackage.fullName + "."
 
diff --git a/test/files/pos/t6482.scala b/test/files/pos/t6482.scala
new file mode 100644
index 0000000000..24ea38e519
--- /dev/null
+++ b/test/files/pos/t6482.scala
@@ -0,0 +1,11 @@
+final class TraversableOnceOps[+A](val collection: TraversableOnce[A]) extends AnyVal {
+  def reduceLeftOption[B >: A](op: (B, A) => B): Option[B] =
+    if (collection.isEmpty) None else Some(collection.reduceLeft[B](op))
+}
+// error: type arguments [B] do not conform to method reduceLeft's type parameter bounds [B >: A]
+//            if (collection.isEmpty) None else Some(collection.reduceLeft[B](op))
+//                                                                        ^
+
+class Foo[+A <: AnyRef](val xs: List[A]) extends AnyVal {
+  def baz[B >: A](x: B): List[B] = x :: xs
+}