Merge pull request #812 from dotty-staging/fix/#806-signature-matching

Fix #806 signature matching

Author: DarkDimius

src/dotty/tools/dotc/config/Config.scala

@@ -76,12 +76,6 @@ object Config {
    */
   final val checkProjections = false
 
-  /** When set, use new signature-based matching.
-   *  Advantage of doing so: It's supposed to be faster
-   *  Disadvantage: It might hide inconsistencies, so while debugging it's better to turn it off
-   */
-  final val newMatch = false
-
   /** The recursion depth for showing a summarized string */
   final val summarizeDepth = 2
 

src/dotty/tools/dotc/core/Denotations.scala

@@ -125,8 +125,9 @@ object Denotations {
 
     /** Resolve overloaded denotation to pick the one with the given signature
      *  when seen from prefix `site`.
+     *  @param relaxed  When true, consider only parameter signatures for a match.
      */
-    def atSignature(sig: Signature, site: Type = NoPrefix)(implicit ctx: Context): SingleDenotation
+    def atSignature(sig: Signature, site: Type = NoPrefix, relaxed: Boolean = false)(implicit ctx: Context): SingleDenotation
 
     /** The variant of this denotation that's current in the given context, or
      *  `NotDefinedHereDenotation` if this denotation does not exist at current phase, but
@@ -221,7 +222,7 @@ object Denotations {
      */
     def matchingDenotation(site: Type, targetType: Type)(implicit ctx: Context): SingleDenotation =
       if (isOverloaded)
-        atSignature(targetType.signature, site).matchingDenotation(site, targetType)
+        atSignature(targetType.signature, site, relaxed = true).matchingDenotation(site, targetType)
       else if (exists && !site.memberInfo(symbol).matchesLoosely(targetType))
         NoDenotation
       else
@@ -268,7 +269,7 @@ object Denotations {
           }
         case denot1: SingleDenotation =>
           if (denot1 eq denot2) denot1
-          else if (denot1.signature matches denot2.signature) {
+          else if (denot1.matches(denot2)) {
             val info1 = denot1.info
             val info2 = denot2.info
             val sym1 = denot1.symbol
@@ -282,14 +283,14 @@ object Denotations {
                 case Nil => true
               }
               sym1.derivesFrom(sym2) ||
-              !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses)
+                !sym2.derivesFrom(sym1) && precedesIn(pre.baseClasses)
             }
 
             /** Preference according to partial pre-order (isConcrete, precedes) */
             def preferSym(sym1: Symbol, sym2: Symbol) =
               sym1.eq(sym2) ||
-              sym1.isAsConcrete(sym2) &&
-              (!sym2.isAsConcrete(sym1) || precedes(sym1.owner, sym2.owner))
+                sym1.isAsConcrete(sym2) &&
+                (!sym2.isAsConcrete(sym1) || precedes(sym1.owner, sym2.owner))
 
             /** Sym preference provided types also override */
             def prefer(sym1: Symbol, sym2: Symbol, info1: Type, info2: Type) =
@@ -310,8 +311,7 @@ object Denotations {
                 new JointRefDenotation(sym, info1 & info2, denot1.validFor & denot2.validFor)
               }
             }
-          }
-          else NoDenotation
+          } else NoDenotation
       }
 
       if (this eq that) this
@@ -333,7 +333,7 @@ object Denotations {
     def | (that: Denotation, pre: Type)(implicit ctx: Context): Denotation = {
 
       def unionDenot(denot1: SingleDenotation, denot2: SingleDenotation): Denotation =
-        if (denot1.signature matches denot2.signature) {
+        if (denot1.matches(denot2)) {
           val sym1 = denot1.symbol
           val sym2 = denot2.symbol
           val info1 = denot1.info
@@ -396,8 +396,8 @@ object Denotations {
     final def validFor = denot1.validFor & denot2.validFor
     final def isType = false
     final def signature(implicit ctx: Context) = Signature.OverloadedSignature
-    def atSignature(sig: Signature, site: Type)(implicit ctx: Context): SingleDenotation =
-      denot1.atSignature(sig, site) orElse denot2.atSignature(sig, site)
+    def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): SingleDenotation =
+      denot1.atSignature(sig, site, relaxed) orElse denot2.atSignature(sig, site, relaxed)
     def currentIfExists(implicit ctx: Context): Denotation =
       derivedMultiDenotation(denot1.currentIfExists, denot2.currentIfExists)
     def current(implicit ctx: Context): Denotation =
@@ -467,9 +467,11 @@ object Denotations {
     def accessibleFrom(pre: Type, superAccess: Boolean)(implicit ctx: Context): Denotation =
       if (!symbol.exists || symbol.isAccessibleFrom(pre, superAccess)) this else NoDenotation
 
-    def atSignature(sig: Signature, site: Type)(implicit ctx: Context): SingleDenotation = {
+    def atSignature(sig: Signature, site: Type, relaxed: Boolean)(implicit ctx: Context): SingleDenotation = {
       val situated = if (site == NoPrefix) this else asSeenFrom(site)
-      if (sig matches situated.signature) this else NoDenotation
+      val matches = sig.matchDegree(situated.signature) >=
+        (if (relaxed) Signature.ParamMatch else Signature.FullMatch)
+      if (matches) this else NoDenotation
     }
 
     // ------ Forming types -------------------------------------------
@@ -778,12 +780,15 @@ object Denotations {
     final def last = this
     final def toDenot(pre: Type)(implicit ctx: Context): Denotation = this
     final def containsSym(sym: Symbol): Boolean = hasUniqueSym && (symbol eq sym)
-    final def containsSig(sig: Signature)(implicit ctx: Context) =
-      exists && (signature matches sig)
+    final def matches(other: SingleDenotation)(implicit ctx: Context): Boolean = {
+      val d = signature.matchDegree(other.signature)
+      d == Signature.FullMatch ||
+      d >= Signature.ParamMatch && info.matches(other.info)
+    }
     final def filterWithPredicate(p: SingleDenotation => Boolean): SingleDenotation =
       if (p(this)) this else NoDenotation
     final def filterDisjoint(denots: PreDenotation)(implicit ctx: Context): SingleDenotation =
-      if (denots.exists && denots.containsSig(signature)) NoDenotation else this
+      if (denots.exists && denots.matches(this)) NoDenotation else this
     def mapInherited(ownDenots: PreDenotation, prevDenots: PreDenotation, pre: Type)(implicit ctx: Context): SingleDenotation =
       if (hasUniqueSym && prevDenots.containsSym(symbol)) NoDenotation
       else if (isType) filterDisjoint(ownDenots).asSeenFrom(pre)
@@ -872,7 +877,7 @@ object Denotations {
     def containsSym(sym: Symbol): Boolean
 
     /** Group contains a denotation with given signature */
-    def containsSig(sig: Signature)(implicit ctx: Context): Boolean
+    def matches(other: SingleDenotation)(implicit ctx: Context): Boolean
 
     /** Keep only those denotations in this group which satisfy predicate `p`. */
     def filterWithPredicate(p: SingleDenotation => Boolean): PreDenotation
@@ -933,8 +938,8 @@ object Denotations {
       (denots1 toDenot pre) & (denots2 toDenot pre, pre)
     def containsSym(sym: Symbol) =
       (denots1 containsSym sym) || (denots2 containsSym sym)
-    def containsSig(sig: Signature)(implicit ctx: Context) =
-      (denots1 containsSig sig) || (denots2 containsSig sig)
+    def matches(other: SingleDenotation)(implicit ctx: Context): Boolean =
+      denots1.matches(other) || denots2.matches(other)
     def filterWithPredicate(p: SingleDenotation => Boolean): PreDenotation =
       derivedUnion(denots1 filterWithPredicate p, denots2 filterWithPredicate p)
     def filterDisjoint(denots: PreDenotation)(implicit ctx: Context): PreDenotation =

src/dotty/tools/dotc/core/Signature.scala

@@ -24,16 +24,27 @@ import TypeErasure.sigName
  *  The signatures of non-method types are always `NotAMethod`.
  */
 case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
+  import Signature._
 
   /** Does this signature coincide with that signature on their parameter parts? */
   final def sameParams(that: Signature): Boolean = this.paramsSig == that.paramsSig
 
-  /** The meaning of `matches` depends on the phase. If types are not erased,
-   *  it means `sameParams`. Once types are erased, it means `==`, comparing parameter as
-   *  well as result type parts.
+  /** The degree to which this signature matches `that`.
+   *  If both parameter and result type names match (i.e. they are the same
+   *  or one is a wildcard), the result is `FullMatch`.
+   *  If only the parameter names match, the result is `ParamMatch` before erasure and
+   *  `NoMatch` otherwise.
+   *  If the parameters do not match, the result is always `NoMatch`.
    */
-  final def matches(that: Signature)(implicit ctx: Context) =
-    if (ctx.erasedTypes) equals(that) else sameParams(that)
+  final def matchDegree(that: Signature)(implicit ctx: Context): MatchDegree =
+    if (sameParams(that))
+      if (resSig == that.resSig || isWildcard(resSig) || isWildcard(that.resSig)) FullMatch
+      else if (!ctx.erasedTypes) ParamMatch
+      else NoMatch
+    else NoMatch
+
+  /** name.toString == "" or name.toString == "_" */
+  private def isWildcard(name: TypeName) = name.isEmpty || name == tpnme.WILDCARD
 
   /** Construct a signature by prepending the signature names of the given `params`
    *  to the parameter part of this signature.
@@ -45,6 +56,11 @@ case class Signature(paramsSig: List[TypeName], resSig: TypeName) {
 
 object Signature {
 
+  type MatchDegree = Int
+  val NoMatch = 0
+  val ParamMatch = 1
+  val FullMatch = 2
+
   /** The signature of everything that's not a method, i.e. that has
    *  a type different from PolyType, MethodType, or ExprType.
    */

src/dotty/tools/dotc/core/TypeComparer.scala

@@ -358,8 +358,7 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
       def compareMethod = tp1 match {
         case tp1 @ MethodType(_, formals1) =>
           (tp1.signature sameParams tp2.signature) &&
-            (if (Config.newMatch) subsumeParams(formals1, formals2, tp1.isJava, tp2.isJava)
-            else matchingParams(formals1, formals2, tp1.isJava, tp2.isJava)) &&
+            matchingParams(formals1, formals2, tp1.isJava, tp2.isJava) &&
             tp1.isImplicit == tp2.isImplicit && // needed?
             isSubType(tp1.resultType, tp2.resultType.subst(tp2, tp1))
         case _ =>
@@ -705,21 +704,6 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
       formals2.isEmpty
   }
 
-  private def subsumeParams(formals1: List[Type], formals2: List[Type], isJava1: Boolean, isJava2: Boolean): Boolean = formals1 match {
-    case formal1 :: rest1 =>
-      formals2 match {
-        case formal2 :: rest2 =>
-          (isSubType(formal2, formal1)
-            || isJava1 && (formal2 isRef ObjectClass) && (formal1 isRef AnyClass)
-            || isJava2 && (formal1 isRef ObjectClass) && (formal2 isRef AnyClass)) &&
-          subsumeParams(rest1, rest2, isJava1, isJava2)
-        case nil =>
-          false
-      }
-    case nil =>
-      formals2.isEmpty
-  }
-
   /** Do poly types `poly1` and `poly2` have type parameters that
    *  have the same bounds (after renaming one set to the other)?
    */
@@ -948,13 +932,6 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
     case tp1 @ MethodType(names1, formals1) =>
       tp2 match {
         case tp2 @ MethodType(names2, formals2)
-        if Config.newMatch && tp1.signature.sameParams(tp2.signature) &&
-           tp1.isImplicit == tp2.isImplicit =>
-          tp1.derivedMethodType(
-              mergeNames(names1, names2, nme.syntheticParamName),
-              (formals1 zipWithConserve formals2)(_ | _),
-              tp1.resultType & tp2.resultType.subst(tp2, tp1))
-        case tp2 @ MethodType(names2, formals2)
         if matchingParams(formals1, formals2, tp1.isJava, tp2.isJava) &&
            tp1.isImplicit == tp2.isImplicit =>
           tp1.derivedMethodType(
@@ -1014,13 +991,6 @@ class TypeComparer(initctx: Context) extends DotClass with ConstraintHandling {
     case tp1 @ MethodType(names1, formals1) =>
       tp2 match {
         case tp2 @ MethodType(names2, formals2)
-        if Config.newMatch && tp1.signature.sameParams(tp2.signature) &&
-           tp1.isImplicit == tp2.isImplicit =>
-          tp1.derivedMethodType(
-              mergeNames(names1, names2, nme.syntheticParamName),
-              (formals1 zipWithConserve formals2)(_ & _),
-              tp1.resultType | tp2.resultType.subst(tp2, tp1))
-        case tp2 @ MethodType(names2, formals2)
         if matchingParams(formals1, formals2, tp1.isJava, tp2.isJava) &&
            tp1.isImplicit == tp2.isImplicit =>
           tp1.derivedMethodType(

src/dotty/tools/dotc/core/Types.scala

@@ -647,12 +647,9 @@ object Types {
      *  (*) when matching with a Java method, we also regard Any and Object as equivalent
      *      parameter types.
      */
-    def matches(that: Type)(implicit ctx: Context): Boolean =
-      if (Config.newMatch) this.signature matches that.signature
-      else track("matches") {
-        ctx.typeComparer.matchesType(
-          this, that, relaxed = !ctx.phase.erasedTypes)
-      }
+    def matches(that: Type)(implicit ctx: Context): Boolean = track("matches") {
+      ctx.typeComparer.matchesType(this, that, relaxed = !ctx.phase.erasedTypes)
+    }
 
     /** This is the same as `matches` except that it also matches => T with T and
      *  vice versa.
@@ -1383,6 +1380,7 @@ object Types {
 
       lastDenotation = denot
       lastSymbol = denot.symbol
+      checkedPeriod = Nowhere
     }
 
     private[dotc] def withSym(sym: Symbol, signature: Signature)(implicit ctx: Context): ThisType =

src/dotty/tools/dotc/typer/Checking.scala

@@ -312,7 +312,7 @@ trait Checking {
     def checkDecl(decl: Symbol): Unit = {
       for (other <- seen(decl.name)) {
         typr.println(i"conflict? $decl $other")
-        if (decl.signature matches other.signature) {
+        if (decl.matches(other)) {
           def doubleDefError(decl: Symbol, other: Symbol): Unit = {
             def ofType = if (decl.isType) "" else d": ${other.info}"
             def explanation =

src/dotty/tools/dotc/typer/RefChecks.scala

@@ -369,7 +369,7 @@ object RefChecks {
             clazz.info.nonPrivateMember(sym.name).hasAltWith { alt =>
               alt.symbol.is(JavaDefined, butNot = Deferred) &&
                 !sym.owner.derivesFrom(alt.symbol.owner) &&
-                alt.signature.matches(sym.signature)
+                alt.matches(sym)
             }
           }
 

src/dotty/tools/dotc/typer/TypeAssigner.scala

@@ -62,8 +62,10 @@ trait TypeAssigner {
             case info: ClassInfo if variance > 0 =>
               val parentType = info.instantiatedParents.reduceLeft(ctx.typeComparer.andType(_, _))
               def addRefinement(parent: Type, decl: Symbol) = {
-                val inherited = parentType.findMember(decl.name, info.cls.thisType, Private)
-                val inheritedInfo = inherited.atSignature(decl.info .signature).info
+                val inherited =
+                  parentType.findMember(decl.name, info.cls.thisType, Private)
+                    .suchThat(decl.matches(_))
+                val inheritedInfo = inherited.info
                 if (inheritedInfo.exists && decl.info <:< inheritedInfo && !(inheritedInfo <:< decl.info))
                   typr.echo(
                     i"add ref $parent $decl --> ",

test/dotc/tests.scala

@@ -104,7 +104,7 @@ class tests extends CompilerTest {
   @Test def neg_typedapply() = compileFile(negDir, "typedapply", xerrors = 4)
   @Test def neg_typedidents() = compileFile(negDir, "typedIdents", xerrors = 2)
   @Test def neg_assignments() = compileFile(negDir, "assignments", xerrors = 3)
-  @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 12)(allowDoubleBindings)
+  @Test def neg_typers() = compileFile(negDir, "typers", xerrors = 10)(allowDoubleBindings)
   @Test def neg_privates() = compileFile(negDir, "privates", xerrors = 2)
   @Test def neg_rootImports = compileFile(negDir, "rootImplicits", xerrors = 2)
   @Test def neg_templateParents() = compileFile(negDir, "templateParents", xerrors = 3)
@@ -115,7 +115,6 @@ class tests extends CompilerTest {
   @Test def neg_overrides = compileFile(negDir, "overrides", xerrors = 11)
   @Test def neg_i39 = compileFile(negDir, "i39", xerrors = 2)
   @Test def neg_i50_volatile = compileFile(negDir, "i50-volatile", xerrors = 6)
-  @Test def neg_t0273_doubledefs = compileFile(negDir, "t0273", xerrors = 1)
   @Test def neg_zoo = compileFile(negDir, "zoo", xerrors = 12)
 
   val negTailcallDir = negDir + "tailcall/"

tests/neg/typers.scala

@@ -22,11 +22,11 @@ object typers {
     val z: Int
     def z(): String      // error: double def
 
-    def f(x: Any) = ()   // error: double def
+    def f(x: Any) = ()   // OK!
     def f(x: AnyRef): AnyRef
 
     def g(x: Object): Unit
-    def g[T](x: T): T = x  // error: double def
+    def g[T](x: T): T = x  // OK!
   }