closes #2741, #4079: pickling now ensures that ...

closes #2741, #4079: pickling now ensures that a local type param with
a non-local owner, which will thus get a localized owner, will only get
a class as its localized owner if its old owner was a class (otherwise,
NoSymbol) this ensures that asSeenFrom does not treat typerefs to this
symbol differently after pickling.

todo: should we pro-actively set the owner of these type params to
something else than the type alias that they originate from? see notes
in typeFunAnon

review by odersky

7 files changed, 73 insertions, 5 deletions

diff --git a/src/compiler/scala/tools/nsc/symtab/Types.scala b/src/compiler/scala/tools/nsc/symtab/Types.scala
index 7b382592e8..4dfde951bd 100644
--- a/src/compiler/scala/tools/nsc/symtab/Types.scala
+++ b/src/compiler/scala/tools/nsc/symtab/Types.scala
@@ -2754,9 +2754,10 @@ A type's typeSymbol should never be inspected directly.
     else tpe // it's okay to be forgiving here
 
   /** A creator for anonymous type functions, where the symbol for the type function still needs to be created
-   * TODO_NMT:
-   *  - can we avoid this whole synthetic owner business? harden ASF instead to deal with orphan type params
-   *  - orthogonally: try to recycle the class symbol in the common case type C[X] = Class[X]  (where appliedType(this, dummyArgs) =:= appliedType(sym.info, dummyArgs))
+   *
+   * TODO:
+   * type params of anonymous type functions, which currently can only arise from normalising type aliases, are owned by the type alias of which they are the eta-expansion
+   * higher-order subtyping expects eta-expansion of type constructors that arise from a class; here, the type params are owned by that class, but is that the right thing to do?
    */
   def typeFunAnon(tps: List[Symbol], body: Type): Type = typeFun(tps, body)
 
@@ -3362,7 +3363,15 @@ A type's typeSymbol should never be inspected directly.
             else if (pre1.isStable) singleType(pre1, sym)
             else pre1.memberType(sym).resultType //todo: this should be rolled into existential abstraction
           }
-        case TypeRef(prefix, sym, args) if (sym.isTypeParameter) =>
+        // AM: Martin, is this description accurate?
+        // walk the owner chain of `clazz` (the original argument to asSeenFrom) until we find the type param's owner (while rewriting pre as we crawl up the owner chain)
+        // once we're at the owner, extract the information that pre encodes about the type param,
+        // by minimally subsuming pre to the type instance of the class that owns the type param,
+        // the type we're looking for is the type instance's type argument at the position corresponding to the type parameter
+        // optimisation: skip this type parameter if it's not owned by a class, as those params are not influenced by the prefix through which they are seen
+        // (concretely: type params of anonymous type functions, which currently can only arise from normalising type aliases, are owned by the type alias of which they are the eta-expansion)
+        // (skolems also aren't affected: they are ruled out by the isTypeParameter check)
+        case TypeRef(prefix, sym, args) if (sym.isTypeParameter && sym.owner.isClass) =>
           def toInstance(pre: Type, clazz: Symbol): Type =
             if ((pre eq NoType) || (pre eq NoPrefix) || !clazz.isClass) mapOver(tp)
             //@M! see test pos/tcpoly_return_overriding.scala why mapOver is necessary
diff --git a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
index ea757b95b4..ba182f7058 100644
--- a/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
+++ b/src/compiler/scala/tools/nsc/symtab/classfile/Pickler.scala
@@ -65,6 +65,7 @@ abstract class Pickler extends SubComponent {
     private var entries   = new Array[AnyRef](256)
     private var ep        = 0
     private val index     = new LinkedHashMap[AnyRef, Int]
+    private lazy val nonClassRoot = root.ownersIterator.find(! _.isClass) getOrElse NoSymbol
 
     private def isRootSym(sym: Symbol) =
       sym.name.toTermName == rootName && sym.owner == rootOwner
@@ -74,7 +75,11 @@ abstract class Pickler extends SubComponent {
      *  Question: Should this be done for refinement class symbols as well?
      */
     private def localizedOwner(sym: Symbol) =
-      if (isLocal(sym) && !isRootSym(sym) && !isLocal(sym.owner)) root
+      if (isLocal(sym) && !isRootSym(sym) && !isLocal(sym.owner))
+        // don't use a class as the localized owner for type parameters that are not owned by a class: those are not instantiated by asSeenFrom
+        // however, they would suddenly be considered by asSeenFrom if their localized owner became a class (causing the crashes of #4079, #2741)
+        (if(sym.isTypeParameter && !sym.owner.isClass) nonClassRoot
+         else root)
       else sym.owner
 
     /** Is root in symbol.owner*, or should it be treated as a local symbol
diff --git a/test/files/neg/t4079.check b/test/files/neg/t4079.check
new file mode 100644
index 0000000000..f4c956c445
--- /dev/null
+++ b/test/files/neg/t4079.check
@@ -0,0 +1,4 @@
+t4079_2.scala:2: error: could not find implicit value for parameter f: Functor[List]
+  Cat.compose[List,Option].Functor
+                           ^
+one error found
diff --git a/test/files/neg/t4079/t4079_1.scala b/test/files/neg/t4079/t4079_1.scala
new file mode 100644
index 0000000000..cbae864788
--- /dev/null
+++ b/test/files/neg/t4079/t4079_1.scala
@@ -0,0 +1,33 @@
+trait Functor[F[_]] {
+  def map[A,B](fa: F[A], f: A => B): F[B]
+}
+
+trait ComposeT[F[_],G[_]] {
+  type Apply[A] = F[G[A]]
+}
+
+case class Compose[F[_],G[_]]() {
+  def Functor(implicit f: Functor[F], g: Functor[G]): Functor[ComposeT[F,G]#Apply] =
+    new Functor[ComposeT[F,G]#Apply] {
+      def map[A,B](c: ComposeT[F,G]#Apply[A], h: A => B) =
+        f.map(c, (x:G[A]) => g.map(x,h))
+    }
+}
+
+object Cat {
+  def compose[F[_],G[_]] = Compose[F,G]()
+}
+
+object Functors {
+  implicit val List = new Functor[List] {
+    def map[A,B](fa: List[A], f: A => B): List[B] = fa map f
+  }
+  implicit val Option = new Functor[Option] {
+    def map[A,B](fa: Option[A], f: A => B): Option[B] = fa map f
+  }
+}
+
+object Main {
+  import Functors._
+  val cf = Cat.compose[List,Option].Functor
+}
diff --git a/test/files/neg/t4079/t4079_2.scala b/test/files/neg/t4079/t4079_2.scala
new file mode 100644
index 0000000000..9069f0ab4e
--- /dev/null
+++ b/test/files/neg/t4079/t4079_2.scala
@@ -0,0 +1,3 @@
+object Test {
+  Cat.compose[List,Option].Functor
+}
diff --git a/test/files/pos/t2741/2741_1.scala b/test/files/pos/t2741/2741_1.scala
new file mode 100644
index 0000000000..d47ed3b6cb
--- /dev/null
+++ b/test/files/pos/t2741/2741_1.scala
@@ -0,0 +1,9 @@
+trait Partial {
+  type Apply[XYZ] = List[XYZ]
+}
+trait MA[M[_]]
+trait MAs {
+  val a: MA[Partial#Apply] = null // after compilation, the type is pickled as `MA[ [B] List[B] ]`
+}
+
+object Scalaz extends MAs
\ No newline at end of file
diff --git a/test/files/pos/t2741/2741_2.scala b/test/files/pos/t2741/2741_2.scala
new file mode 100644
index 0000000000..41f6a64260
--- /dev/null
+++ b/test/files/pos/t2741/2741_2.scala
@@ -0,0 +1,5 @@
+// object Test compiles jointly, but not separately.
+object Test {
+  import Scalaz._
+  Scalaz.a
+}
\ No newline at end of file