Take number of type parameters into account when choosing an overload

smarter · smarter · commit f95153437ef9 · 2019-08-22T18:01:42.000+02:00
In Tasty, pickling a selection will pickle its signature using the
SIGNED tag to be able to distinguish overloads, this usually works but
it turns out that some overloads have the same signature (see
tests/pos/poly-signature.scala, but this also affects `Set#++` in the
2.13 collections), resulting in ambiguous overload errors when
unpickled.

This commit solves this problem by also storing the number of type
parameters in the signature. The obvious way to implement this would be
to add a field of type Int to the `Signature` case class, but that
wouldn't scale to a future where methods may have multiple type
parameter lists, so instead we change the existing `paramsSig:
List[TypeName]` field to take a list of `TypeName | Int`.
diff --git a/compiler/src/dotty/tools/dotc/core/Signature.scala b/compiler/src/dotty/tools/dotc/core/Signature.scala
@@ -1,27 +1,42 @@
 package dotty.tools.dotc
 package core
 
+import scala.annotation.tailrec
+
 import Names._, Types._, Contexts._, StdNames._, Decorators._
 import TypeErasure.sigName
-
-import scala.annotation.tailrec
+import Signature._
 
 /** The signature of a denotation.
- *  Overloaded denotations with the same name are distinguished by
- *  their signatures. A signature of a method (of type PolyType,MethodType, or ExprType) is
- *  composed of a list of signature names, one for each parameter type, plus a signature for
- *  the result type. Methods are uncurried before taking their signatures.
- *  The signature name of a type is the fully qualified name of the type symbol of the type's erasure.
+ *
+ *  Same-named denotations with different signatures are considered to be
+ *  overloads, see `SingleDenotation#matches` for more details.
+ *
+ *  A _method signature_ (a value of type `Signature`, excluding `NotAMethod`
+ *  and `OverloadedSignature`) is composed of a list of _parameter signatures_,
+ *  plus a _type signature_ for the final result type.
+ *
+ *  A _parameter signature_ (a value of type `ParamSig`) is either an integer,
+ *  representing the number of type parameters in a type parameter section, or
+ *  the _type signature_ of a term parameter.
+ *
+ *  A _type signature_ is the fully qualified name of the type symbol of the
+ *  type's erasure.
  *
  *  For instance a definition
  *
- *      def f(x: Int)(y: List[String]): String
+ *      def f[T, S](x: Int)(y: List[T]): S
  *
  *  would have signature
  *
  *      Signature(
- *        List("scala.Int".toTypeName, "scala.collection.immutable.List".toTypeName),
- *        "scala.String".toTypeName)
+ *        List(2, "scala.Int".toTypeName, "scala.collection.immutable.List".toTypeName),
+ *        "java.lang.Object".toTypeName)
+ *
+ *  Note that `paramsSig` has one entry for *a whole type parameter section* but
+ *  one entry *for each term parameter* (currently, methods in Dotty can only
+ *  have one type parameter section but this encoding leaves the door open for
+ *  supporting multiple sections).
  *
  *  The signatures of non-method types are always `NotAMethod`.
  *
@@ -31,19 +46,18 @@ import scala.annotation.tailrec
  *   - tpnme.WILDCARD       Arises from a Wildcard or error type
  *   - tpnme.Uninstantiated Arises from an uninstantiated type variable
  */
-case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
-  import Signature._
+case class Signature(paramsSig: List[ParamSig], resSig: TypeName) {
 
   /** Two names are consistent if they are the same or one of them is tpnme.Uninstantiated */
-  private def consistent(name1: TypeName, name2: TypeName) =
+  private def consistent(name1: ParamSig, name2: ParamSig) =
     name1 == name2 || name1 == tpnme.Uninstantiated || name2 == tpnme.Uninstantiated
 
   /** Does this signature coincide with that signature on their parameter parts?
-   *  This is the case if all parameter names are _consistent_, i.e. they are either
+   *  This is the case if all parameter signatures are _consistent_, i.e. they are either
    *  equal or on of them is tpnme.Uninstantiated.
    */
   final def consistentParams(that: Signature)(implicit ctx: Context): Boolean = {
-    @tailrec def loop(names1: List[TypeName], names2: List[TypeName]): Boolean =
+    @tailrec def loop(names1: List[ParamSig], names2: List[ParamSig]): Boolean =
       if (names1.isEmpty) names2.isEmpty
       else !names2.isEmpty && consistent(names1.head, names2.head) && loop(names1.tail, names2.tail)
     if (ctx.erasedTypes && (this == NotAMethod) != (that == NotAMethod))
@@ -56,22 +70,23 @@ case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
 
   /** `that` signature, but keeping all corresponding parts of `this` signature. */
   final def updateWith(that: Signature): Signature = {
-    def update(name1: TypeName, name2: TypeName): TypeName =
+    def update[T <: ParamSig](name1: T, name2: T): T =
       if (consistent(name1, name2)) name1 else name2
     if (this == that) this
     else if (!this.paramsSig.hasSameLengthAs(that.paramsSig)) that
     else {
       val mapped = Signature(
-          this.paramsSig.zipWithConserve(that.paramsSig)(update),
+          // DOTTY: we shouldn't have to explicitly pass a type argument to `update`
+          this.paramsSig.zipWithConserve(that.paramsSig)(update[ParamSig]),
           update(this.resSig, that.resSig))
       if (mapped == this) this else mapped
     }
   }
 
   /** The degree to which this signature matches `that`.
-   *  If parameter names are consistent and result types names match (i.e. they are the same
+   *  If parameter signatures are consistent and result types names match (i.e. they are the same
    *  or one is a wildcard), the result is `FullMatch`.
-   *  If only the parameter names are consistent, the result is `ParamMatch` before erasure and
+   *  If only the parameter signatures are consistent, the result ParamMatch` before erasure and
    *  `NoMatch` otherwise.
    *  If the parameters are inconsistent, the result is always `NoMatch`.
    */
@@ -94,8 +109,16 @@ case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
    *
    *  Like Signature#apply, the result is only cacheable if `isUnderDefined == false`.
    */
-  def prepend(params: List[Type], isJava: Boolean)(implicit ctx: Context): Signature =
-    Signature(params.map(p => sigName(p, isJava)) ++ paramsSig, resSig)
+  def prependTermParams(params: List[Type], isJava: Boolean)(implicit ctx: Context): Signature =
+    Signature(params.map(p => sigName(p, isJava)) ::: paramsSig, resSig)
+
+  /** Construct a signature by prepending the length of a type parameter section
+   *  to the parameter part of this signature.
+   *
+   *  Like Signature#apply, the result is only cacheable if `isUnderDefined == false`.
+   */
+  def prependTypeParams(typeParamSigsSectionLength: Int)(implicit ctx: Context): Signature =
+    Signature(typeParamSigsSectionLength :: paramsSig, resSig)
 
   /** A signature is under-defined if its paramsSig part contains at least one
    *  `tpnme.Uninstantiated`. Under-defined signatures arise when taking a signature
@@ -106,6 +129,10 @@ case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
 }
 
 object Signature {
+  /** A parameter signature, see the documentation of `Signature` for more information. */
+  type ParamSig = TypeName | Int
+    // Erasure means that our Ints will be boxed, but Integer#valueOf caches
+    // small values, so the performance hit should be minimal.
 
   enum MatchDegree {
     case NoMatch, ParamMatch, FullMatch
diff --git a/compiler/src/dotty/tools/dotc/core/Types.scala b/compiler/src/dotty/tools/dotc/core/Types.scala
@@ -3216,7 +3216,7 @@ object Types {
 
     def computeSignature(implicit ctx: Context): Signature = {
       val params = if (isErasedMethod) Nil else paramInfos
-      resultSignature.prepend(params, isJavaMethod)
+      resultSignature.prependTermParams(params, isJavaMethod)
     }
 
     protected def prefixString: String = companion.prefixString
@@ -3402,7 +3402,8 @@ object Types {
     assert(resType.isInstanceOf[TermType], this)
     assert(paramNames.nonEmpty)
 
-    def computeSignature(implicit ctx: Context): Signature = resultSignature
+    def computeSignature(implicit ctx: Context): Signature =
+      resultSignature.prependTypeParams(paramNames.length)
 
     override def isContextualMethod = resType.isContextualMethod
     override def isImplicitMethod = resType.isImplicitMethod
diff --git a/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala b/compiler/src/dotty/tools/dotc/core/tasty/NameBuffer.scala
@@ -4,7 +4,7 @@ package core
 package tasty
 
 import collection.mutable
-import Names.{Name, chrs, SimpleName, DerivedName}
+import Names.{Name, chrs, SimpleName, DerivedName, TypeName}
 import NameKinds._
 import Decorators._
 import TastyBuffer._
@@ -23,7 +23,13 @@ class NameBuffer extends TastyBuffer(10000) {
       case None =>
         name1 match {
           case SignedName(original, Signature(params, result)) =>
-            nameIndex(original); nameIndex(result); params.foreach(nameIndex)
+            nameIndex(original)
+            nameIndex(result)
+            params.foreach {
+              case param: TypeName =>
+                nameIndex(param)
+              case _ =>
+            }
           case AnyQualifiedName(prefix, name) =>
             nameIndex(prefix); nameIndex(name)
           case AnyUniqueName(original, separator, num) =>
@@ -50,6 +56,16 @@ class NameBuffer extends TastyBuffer(10000) {
   def writeNameRef(ref: NameRef): Unit = writeNat(ref.index)
   def writeNameRef(name: Name): Unit = writeNameRef(nameRefs(name.toTermName))
 
+  def writeParamSig(paramSig: Signature.ParamSig): Unit ={
+    val encodedValue = paramSig match {
+      case paramSig: TypeName =>
+        nameRefs(paramSig.toTermName).index
+      case paramSig: Int =>
+        -paramSig
+    }
+    writeInt(encodedValue)
+  }
+
   def pickleNameContents(name: Name): Unit = {
     val tag = name.toTermName.info.kind.tag
     writeByte(tag)
@@ -70,10 +86,10 @@ class NameBuffer extends TastyBuffer(10000) {
         }
       case AnyNumberedName(original, num) =>
         withLength { writeNameRef(original); writeNat(num) }
-      case SignedName(original, Signature(params, result)) =>
+      case SignedName(original, Signature(paramsSig, result)) =>
         withLength(
-          { writeNameRef(original); writeNameRef(result); params.foreach(writeNameRef) },
-          if ((params.length + 2) * maxIndexWidth <= maxNumInByte) 1 else 2)
+          { writeNameRef(original); writeNameRef(result); paramsSig.foreach(writeParamSig) },
+          if ((paramsSig.length + 2) * maxIndexWidth <= maxNumInByte) 1 else 2)
       case DerivedName(original, _) =>
         withLength { writeNameRef(original) }
     }
diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyFormat.scala
@@ -42,7 +42,10 @@ Macro-format:
                   INLINEACCESSOR    Length underlying_NameRef                               -- inline$A
                   OBJECTCLASS       Length underlying_NameRef                               -- A$  (name of the module class for module A)
 
-                  SIGNED            Length original_NameRef resultSig_NameRef paramSig_NameRef*   -- name + signature
+                  SIGNED            Length original_NameRef resultSig_NameRef ParamSig*     -- name + signature
+
+  ParamSig      = Int // If negative, the absolute value represents the length of a type parameter section
+                      // If positive, this is a NameRef for the fully qualified name of a term parameter.
 
   NameRef       = Nat                    // ordinal number of name in name table, starting from 1.
 
@@ -248,7 +251,7 @@ Standard Section: "Comments" Comment*
 object TastyFormat {
 
   final val header: Array[Int] = Array(0x5C, 0xA1, 0xAB, 0x1F)
-  val MajorVersion: Int = 16
+  val MajorVersion: Int = 17
   val MinorVersion: Int = 0
 
   /** Tags used to serialize names */
diff --git a/compiler/src/dotty/tools/dotc/core/tasty/TastyUnpickler.scala b/compiler/src/dotty/tools/dotc/core/tasty/TastyUnpickler.scala
@@ -36,6 +36,14 @@ class TastyUnpickler(reader: TastyReader) {
   private def readName(): TermName = nameAtRef(readNameRef())
   private def readString(): String = readName().toString
 
+  private def readParamSig(): Signature.ParamSig = {
+    val ref = readInt()
+    if (ref < 0)
+      ref.abs
+    else
+      nameAtRef(NameRef(ref)).toTypeName
+  }
+
   private def readNameContents(): TermName = {
     val tag = readByte()
     val length = readNat()
@@ -58,8 +66,9 @@ class TastyUnpickler(reader: TastyReader) {
       case SIGNED =>
         val original = readName()
         val result = readName().toTypeName
-        val params = until(end)(readName().toTypeName)
-        var sig = Signature(params, result)
+        // DOTTY: we shouldn't have to give an explicit type to paramsSig
+        val paramsSig: List[Signature.ParamSig] = until(end)(readParamSig())
+        val sig = Signature(paramsSig, result)
         SignedName(original, sig)
       case _ =>
         simpleNameKindOfTag(tag)(readName())
diff --git a/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala b/compiler/src/dotty/tools/dotc/reporting/diagnostic/messages.scala
@@ -2152,19 +2152,27 @@ object messages {
     val kind: String = "Duplicate Symbol"
     val msg: String = {
       def nameAnd = if (decl.name != previousDecl.name) " name and" else ""
-      val details = if (decl.isRealMethod && previousDecl.isRealMethod) {
-        // compare the signatures when both symbols represent methods
-        decl.signature.matchDegree(previousDecl.signature) match {
-          case Signature.MatchDegree.NoMatch =>
+      def details(implicit ctx: Context): String =
+        if (decl.isRealMethod && previousDecl.isRealMethod) {
+          // compare the signatures when both symbols represent methods
+          decl.signature.matchDegree(previousDecl.signature) match {
             // DOTTY problem: Need to qualify MatchDegree enum vals since otherwise exhaustivity fails.
             // To fix this, we need to export vals under singleton types.
-            "" // shouldn't be reachable
-          case Signature.MatchDegree.ParamMatch =>
-            "have matching parameter types."
-          case Signature.MatchDegree.FullMatch =>
-            i"have the same$nameAnd type after erasure."
+            case Signature.MatchDegree.NoMatch =>
+              // If the signatures don't match at all at the current phase, then
+              // they might match after erasure.
+              val elimErasedCtx = ctx.withPhaseNoEarlier(ctx.elimErasedValueTypePhase.next)
+              if (elimErasedCtx != ctx)
+                details(elimErasedCtx)
+              else
+                "" // shouldn't be reachable
+            case Signature.MatchDegree.ParamMatch =>
+              "have matching parameter types."
+            case Signature.MatchDegree.FullMatch =>
+              i"have the same$nameAnd type after erasure."
+          }
         }
-      } else ""
+        else ""
       def symLocation(sym: Symbol) = {
         val lineDesc =
           if (sym.span.exists && sym.span != sym.owner.span)
diff --git a/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala b/compiler/src/dotty/tools/dotc/transform/ExtensionMethods.scala
@@ -15,6 +15,7 @@ import SymDenotations._, Symbols._, StdNames._, Denotations._
 import TypeErasure.{ valueErasure, ErasedValueType }
 import NameKinds.{ExtMethName, UniqueExtMethName}
 import Decorators._
+import TypeUtils._
 
 /**
  * Perform Step 1 in the inline classes SIP: Creates extension methods for all
@@ -190,11 +191,13 @@ object ExtensionMethods {
       val companion = imeth.owner.companionModule
       val companionInfo = companion.info
       val candidates = companionInfo.decl(extensionName(imeth)).alternatives
-      val matching = candidates filter (c => FullParameterization.memberSignature(c.info) == imeth.signature)
+      val matching =
+        // See the documentation of `memberSignature` to understand why `.stripPoly.ensureMethodic` is needed here.
+        candidates filter (c => FullParameterization.memberSignature(c.info) == imeth.info.stripPoly.ensureMethodic.signature)
       assert(matching.nonEmpty,
        i"""no extension method found for:
           |
-          |  $imeth:${imeth.info.show} with signature ${imeth.signature} in ${companion.moduleClass}
+          |  $imeth:${imeth.info.show} with signature ${imeth.info.signature} in ${companion.moduleClass}
           |
           | Candidates:
           |
diff --git a/compiler/src/dotty/tools/dotc/transform/FullParameterization.scala b/compiler/src/dotty/tools/dotc/transform/FullParameterization.scala
@@ -247,7 +247,16 @@ trait FullParameterization {
 object FullParameterization {
 
   /** Assuming `info` is a result of a `fullyParameterizedType` call, the signature of the
-   *  original method type `X` such that `info = fullyParameterizedType(X, ...)`.
+   *  original method type `X` after stripping its leading type parameters section,
+   *  such that:
+   *    info.stripPoly.ensureMethodic = fullyParameterizedType(X, ...).stripPoly.ensureMethodic
+   *
+   *  NOTE: Keeping the polymorphic part of the signature would be more precise,
+   *  but we cannot distinguish which type parameters of `info` are also type
+   *  parameters of`X`. This could be fixed by using a specific NameKind for the
+   *  extra type parameters, but that wouldn't help for extension methods
+   *  unpickled from Scala 2 (because Scala 2 extmeths phase happens before
+   *  pickling, which is maybe something we should change for 2.14).
    */
   def memberSignature(info: Type)(implicit ctx: Context): Signature = info match {
     case info: PolyType =>
diff --git a/tests/pos/poly-signature.scala b/tests/pos/poly-signature.scala
@@ -0,0 +1,13 @@
+trait P[A] {
+  def foo[T](x: Int): A = ???
+}
+
+class C extends P[Int] {
+  def foo(x: Int): Int = x
+}
+
+object Test {
+  def test(p: C): Unit = {
+    p.foo(1)
+  }
+}
