Skip to content

Fix #806 signature matching #812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 30, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions src/dotty/tools/dotc/config/Config.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
43 changes: 24 additions & 19 deletions src/dotty/tools/dotc/core/Denotations.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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) =
Expand All @@ -310,8 +311,7 @@ object Denotations {
new JointRefDenotation(sym, info1 & info2, denot1.validFor & denot2.validFor)
}
}
}
else NoDenotation
} else NoDenotation
}

if (this eq that) this
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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 -------------------------------------------
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 =
Expand Down
26 changes: 21 additions & 5 deletions src/dotty/tools/dotc/core/Signature.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
*/
Expand Down
32 changes: 1 addition & 31 deletions src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 _ =>
Expand Down Expand Up @@ -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)?
*/
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
10 changes: 4 additions & 6 deletions src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
2 changes: 1 addition & 1 deletion src/dotty/tools/dotc/typer/RefChecks.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down
6 changes: 4 additions & 2 deletions src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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 --> ",
Expand Down
3 changes: 1 addition & 2 deletions test/dotc/tests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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/"
Expand Down
4 changes: 2 additions & 2 deletions tests/neg/typers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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!
}


Expand Down
File renamed without changes.