From 158ccd7df3655a2a02065df5ebc2c8e0a28eb5f7 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Sun, 16 Dec 2018 14:38:11 +0100 Subject: [PATCH 1/3] Fix #5592: Fix & of method type refinements In `distributeAnd` we need to combine refinements in the same way denotation infos are combined, with special treatement of method and poly types. --- .../dotty/tools/dotc/core/Denotations.scala | 254 +++++++++--------- .../dotty/tools/dotc/core/TypeComparer.scala | 8 +- tests/neg/i5592.scala | 29 ++ 3 files changed, 165 insertions(+), 126 deletions(-) create mode 100644 tests/neg/i5592.scala diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 15eb8c1e65a9..c05035341f3a 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -334,17 +334,6 @@ object Denotations { else asSingleDenotation } - /** Handle merge conflict by throwing a `MergeError` exception */ - private def mergeConflict(tp1: Type, tp2: Type, that: Denotation)(implicit ctx: Context): Type = - throw new MergeError(this.symbol, that.symbol, tp1, tp2, NoPrefix) - - /** Merge parameter names of lambda types. If names in corresponding positions match, keep them, - * otherwise generate new synthetic names. - */ - private def mergeParamNames(tp1: LambdaType, tp2: LambdaType): List[tp1.ThisName] = - (for ((name1, name2, idx) <- (tp1.paramNames, tp2.paramNames, tp1.paramNames.indices).zipped) - yield if (name1 == name2) name1 else tp1.companion.syntheticParamName(idx)).toList - /** Form a denotation by conjoining with denotation `that`. * * NoDenotations are dropped. MultiDenotations are handled by merging @@ -374,72 +363,6 @@ object Denotations { * If SingleDenotations with different signatures are joined, return NoDenotation. */ def & (that: Denotation, pre: Type, safeIntersection: Boolean = false)(implicit ctx: Context): Denotation = { - - /** Normally, `tp1 & tp2`. Special cases for matching methods and classes, with - * the possibility of raising a merge error. - */ - def infoMeet(tp1: Type, tp2: Type): Type = { - if (tp1 eq tp2) tp1 - else tp1 match { - case tp1: TypeBounds => - tp2 match { - case tp2: TypeBounds => if (safeIntersection) tp1 safe_& tp2 else tp1 & tp2 - case tp2: ClassInfo if tp1 contains tp2 => tp2 - case _ => mergeConflict(tp1, tp2, that) - } - case tp1: ClassInfo => - tp2 match { - case tp2: ClassInfo if tp1.cls eq tp2.cls => tp1.derivedClassInfo(tp1.prefix & tp2.prefix) - case tp2: TypeBounds if tp2 contains tp1 => tp1 - case _ => mergeConflict(tp1, tp2, that) - } - - // Two remedial strategies: - // - // 1. Prefer method types over poly types. This is necessary to handle - // overloaded definitions like the following - // - // def ++ [B >: A](xs: C[B]): D[B] - // def ++ (xs: C[A]): D[A] - // - // (Code like this is found in the collection strawman) - // - // 2. In the case of two method types or two polytypes with matching - // parameters and implicit status, merge corresponding parameter - // and result types. - case tp1: MethodType => - tp2 match { - case tp2: PolyType => - tp1 - case tp2: MethodType if ctx.typeComparer.matchingMethodParams(tp1, tp2) && - tp1.isImplicitMethod == tp2.isImplicitMethod => - tp1.derivedLambdaType( - mergeParamNames(tp1, tp2), - tp1.paramInfos, - infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1))) - case _ => - mergeConflict(tp1, tp2, that) - } - case tp1: PolyType => - tp2 match { - case tp2: MethodType => - tp2 - case tp2: PolyType if ctx.typeComparer.matchingPolyParams(tp1, tp2) => - tp1.derivedLambdaType( - mergeParamNames(tp1, tp2), - tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => - infoMeet(p1, p2.subst(tp2, tp1)).bounds - }, - infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1))) - case _ => - mergeConflict(tp1, tp2, that) - } - - case _ => - tp1 & tp2 - } - } - /** Try to merge denot1 and denot2 without adding a new signature. */ def mergeDenot(denot1: Denotation, denot2: SingleDenotation): Denotation = denot1 match { case denot1 @ MultiDenotation(denot11, denot12) => @@ -535,7 +458,7 @@ object Denotations { if (preferSym(sym2, sym1)) sym2 else sym1 val jointInfo = - try infoMeet(info1, info2) + try infoMeet(info1, info2, sym1, sym2, safeIntersection) catch { case ex: MergeError => // TODO: this picks one type over the other whereas it might be better @@ -571,51 +494,6 @@ object Denotations { */ def | (that: Denotation, pre: Type)(implicit ctx: Context): Denotation = { - /** Normally, `tp1 | tp2`. Special cases for matching methods and classes, with - * the possibility of raising a merge error. - */ - def infoJoin(tp1: Type, tp2: Type): Type = tp1 match { - case tp1: TypeBounds => - tp2 match { - case tp2: TypeBounds => tp1 | tp2 - case tp2: ClassInfo if tp1 contains tp2 => tp1 - case _ => mergeConflict(tp1, tp2, that) - } - case tp1: ClassInfo => - tp2 match { - case tp2: ClassInfo if tp1.cls eq tp2.cls => tp1.derivedClassInfo(tp1.prefix | tp2.prefix) - case tp2: TypeBounds if tp2 contains tp1 => tp2 - case _ => mergeConflict(tp1, tp2, that) - } - case tp1: MethodType => - tp2 match { - case tp2: MethodType - if ctx.typeComparer.matchingMethodParams(tp1, tp2) && - tp1.isImplicitMethod == tp2.isImplicitMethod => - tp1.derivedLambdaType( - mergeParamNames(tp1, tp2), - tp1.paramInfos, - tp1.resultType | tp2.resultType.subst(tp2, tp1)) - case _ => - mergeConflict(tp1, tp2, that) - } - case tp1: PolyType => - tp2 match { - case tp2: PolyType - if ctx.typeComparer.matchingPolyParams(tp1, tp2) => - tp1.derivedLambdaType( - mergeParamNames(tp1, tp2), - tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => - (p1 | p2.subst(tp2, tp1)).bounds - }, - tp1.resultType | tp2.resultType.subst(tp2, tp1)) - case _ => - mergeConflict(tp1, tp2, that) - } - case _ => - tp1 | tp2 - } - def unionDenot(denot1: SingleDenotation, denot2: SingleDenotation): Denotation = if (denot1.matches(denot2)) { val sym1 = denot1.symbol @@ -646,7 +524,7 @@ object Denotations { lubSym(sym1.allOverriddenSymbols, NoSymbol) } new JointRefDenotation( - jointSym, infoJoin(info1, info2), denot1.validFor & denot2.validFor) + jointSym, infoJoin(info1, info2, sym1, sym2), denot1.validFor & denot2.validFor) } } else NoDenotation @@ -678,6 +556,134 @@ object Denotations { final def containsSym(sym: Symbol): Boolean = hasUniqueSym && (symbol eq sym) } + // ------ Info meets and joins --------------------------------------------- + + /** Handle merge conflict by throwing a `MergeError` exception */ + private def mergeConflict(sym1: Symbol, sym2: Symbol, tp1: Type, tp2: Type)(implicit ctx: Context): Type = + throw new MergeError(sym1, sym2, tp1, tp2, NoPrefix) + + /** Merge parameter names of lambda types. If names in corresponding positions match, keep them, + * otherwise generate new synthetic names. + */ + private def mergeParamNames(tp1: LambdaType, tp2: LambdaType): List[tp1.ThisName] = + (for ((name1, name2, idx) <- (tp1.paramNames, tp2.paramNames, tp1.paramNames.indices).zipped) + yield if (name1 == name2) name1 else tp1.companion.syntheticParamName(idx)).toList + + /** Normally, `tp1 & tp2`. Special cases for matching methods and classes, with + * the possibility of raising a merge error. + */ + def infoMeet(tp1: Type, tp2: Type, sym1: Symbol, sym2: Symbol, safeIntersection: Boolean)(implicit ctx: Context): Type = { + if (tp1 eq tp2) tp1 + else tp1 match { + case tp1: TypeBounds => + tp2 match { + case tp2: TypeBounds => if (safeIntersection) tp1 safe_& tp2 else tp1 & tp2 + case tp2: ClassInfo if tp1 contains tp2 => tp2 + case _ => mergeConflict(sym1, sym2, tp1, tp2) + } + case tp1: ClassInfo => + tp2 match { + case tp2: ClassInfo if tp1.cls eq tp2.cls => tp1.derivedClassInfo(tp1.prefix & tp2.prefix) + case tp2: TypeBounds if tp2 contains tp1 => tp1 + case _ => mergeConflict(sym1, sym2, tp1, tp2) + } + + // Two remedial strategies: + // + // 1. Prefer method types over poly types. This is necessary to handle + // overloaded definitions like the following + // + // def ++ [B >: A](xs: C[B]): D[B] + // def ++ (xs: C[A]): D[A] + // + // (Code like this is found in the collection strawman) + // + // 2. In the case of two method types or two polytypes with matching + // parameters and implicit status, merge corresponding parameter + // and result types. + case tp1: MethodType => + tp2 match { + case tp2: PolyType => + tp1 + case tp2: MethodType if ctx.typeComparer.matchingMethodParams(tp1, tp2) && + tp1.isImplicitMethod == tp2.isImplicitMethod => + tp1.derivedLambdaType( + mergeParamNames(tp1, tp2), + tp1.paramInfos, + infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2, safeIntersection)) + case _ => + mergeConflict(sym1, sym2, tp1, tp2) + } + case tp1: PolyType => + tp2 match { + case tp2: MethodType => + tp2 + case tp2: PolyType if ctx.typeComparer.matchingPolyParams(tp1, tp2) => + tp1.derivedLambdaType( + mergeParamNames(tp1, tp2), + tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => + infoMeet(p1, p2.subst(tp2, tp1), sym1, sym2, safeIntersection).bounds + }, + infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2, safeIntersection)) + case _ => + mergeConflict(sym1, sym2, tp1, tp2) + } + + case _ => + try tp1 & tp2 + catch { + case ex: Throwable => + println(i"error for meet: $tp1 &&& $tp2, ${tp1.getClass}, ${tp2.getClass}") + throw ex + } + } + } + + /** Normally, `tp1 | tp2`. Special cases for matching methods and classes, with + * the possibility of raising a merge error. + */ + def infoJoin(tp1: Type, tp2: Type, sym1: Symbol, sym2: Symbol)(implicit ctx: Context): Type = tp1 match { + case tp1: TypeBounds => + tp2 match { + case tp2: TypeBounds => tp1 | tp2 + case tp2: ClassInfo if tp1 contains tp2 => tp1 + case _ => mergeConflict(sym1, sym2, tp1, tp2) + } + case tp1: ClassInfo => + tp2 match { + case tp2: ClassInfo if tp1.cls eq tp2.cls => tp1.derivedClassInfo(tp1.prefix | tp2.prefix) + case tp2: TypeBounds if tp2 contains tp1 => tp2 + case _ => mergeConflict(sym1, sym2, tp1, tp2) + } + case tp1: MethodType => + tp2 match { + case tp2: MethodType + if ctx.typeComparer.matchingMethodParams(tp1, tp2) && + tp1.isImplicitMethod == tp2.isImplicitMethod => + tp1.derivedLambdaType( + mergeParamNames(tp1, tp2), + tp1.paramInfos, + tp1.resultType | tp2.resultType.subst(tp2, tp1)) + case _ => + mergeConflict(sym1, sym2, tp1, tp2) + } + case tp1: PolyType => + tp2 match { + case tp2: PolyType + if ctx.typeComparer.matchingPolyParams(tp1, tp2) => + tp1.derivedLambdaType( + mergeParamNames(tp1, tp2), + tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => + (p1 | p2.subst(tp2, tp1)).bounds + }, + tp1.resultType | tp2.resultType.subst(tp2, tp1)) + case _ => + mergeConflict(sym1, sym2, tp1, tp2) + } + case _ => + tp1 | tp2 + } + /** A non-overloaded denotation */ abstract class SingleDenotation(symbol: Symbol, initInfo: Type) extends Denotation(symbol, initInfo) { protected def newLikeThis(symbol: Symbol, info: Type): SingleDenotation diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index bdc192bc6bd7..29943eb1f553 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1643,8 +1643,12 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { case tp1: RefinedType => tp2 match { case tp2: RefinedType if tp1.refinedName == tp2.refinedName => - tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, - tp1.refinedInfo & tp2.refinedInfo) + val jointInfo = + try Denotations.infoMeet(tp1.refinedInfo, tp2.refinedInfo, NoSymbol, NoSymbol, safeIntersection = false) + catch { + case ex: MergeError => NoType + } + tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, jointInfo) case _ => NoType } diff --git a/tests/neg/i5592.scala b/tests/neg/i5592.scala new file mode 100644 index 000000000000..f86746f55396 --- /dev/null +++ b/tests/neg/i5592.scala @@ -0,0 +1,29 @@ +object Test { + type Obj + type Forall[F[_]] = (x: Obj) => F[x.type] + + trait EQ[A, B] { + def sub[F[_]]: F[A] => F[B]; + def commute: EQ[B, A] = this.sub[[b] => EQ[b, A]](implicitly[EQ[A, A]]) + } + implicit def typeEq[A]: EQ[A, A] = new EQ[A, A] { + def sub[F[_]]: F[A] => F[A] = identity + } + + // these are both fine + val eqReflexive1: (x: Obj) => (EQ[x.type, x.type]) = { x: Obj => implicitly } + val eqReflexive2: Forall[[x] => EQ[x, x]] = { x: Obj => implicitly } + + // this compiles + val eqSymmetric1: (x: Obj) => (y: Obj) => EQ[x.type, y.type] => EQ[y.type, x.type] = { + { x: Obj => { y: Obj => { xEqy: EQ[x.type, y.type] => xEqy.commute } } } + } + + val eqSymmetric2: Forall[[x] => (y: Obj) => (EQ[x, y.type]) => (EQ[y.type, x])] = { + { x: Obj => { y: Obj => { xEqy: EQ[x.type, y.type] => xEqy.commute } } } // error // error + } + + val eqSymmetric3: Forall[[x] => Forall[[y] => EQ[x, y] => EQ[y, x]]] = { + { x: Obj => { y: Obj => { xEqy: EQ[x.type, y.type] => xEqy.commute } } } // error // error + } +} \ No newline at end of file From 3f96efe86471769441416619a41d9c2204abe629 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 17 Dec 2018 17:39:05 +0100 Subject: [PATCH 2/3] Address review comments --- .../src/dotty/tools/dotc/core/Denotations.scala | 4 ++-- .../src/dotty/tools/dotc/core/TypeComparer.scala | 13 +++++++------ 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index c05035341f3a..2ac5f9114297 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -663,7 +663,7 @@ object Denotations { tp1.derivedLambdaType( mergeParamNames(tp1, tp2), tp1.paramInfos, - tp1.resultType | tp2.resultType.subst(tp2, tp1)) + infoJoin(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2)) case _ => mergeConflict(sym1, sym2, tp1, tp2) } @@ -676,7 +676,7 @@ object Denotations { tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => (p1 | p2.subst(tp2, tp1)).bounds }, - tp1.resultType | tp2.resultType.subst(tp2, tp1)) + infoJoin(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2)) case _ => mergeConflict(sym1, sym2, tp1, tp2) } diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 29943eb1f553..1a3fd06e9f5e 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1643,12 +1643,13 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { case tp1: RefinedType => tp2 match { case tp2: RefinedType if tp1.refinedName == tp2.refinedName => - val jointInfo = - try Denotations.infoMeet(tp1.refinedInfo, tp2.refinedInfo, NoSymbol, NoSymbol, safeIntersection = false) - catch { - case ex: MergeError => NoType - } - tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, jointInfo) + try { + val jointInfo = Denotations.infoMeet(tp1.refinedInfo, tp2.refinedInfo, NoSymbol, NoSymbol, safeIntersection = false) + tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, jointInfo) + } + catch { + case ex: MergeError => tp1.parent & tp2.parent + } case _ => NoType } From cf3fdfb27ad93fe69835d494c4ad60c10ed10475 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Mon, 17 Dec 2018 18:01:59 +0100 Subject: [PATCH 3/3] Address next round of comments --- compiler/src/dotty/tools/dotc/core/Denotations.scala | 4 ++-- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/Denotations.scala b/compiler/src/dotty/tools/dotc/core/Denotations.scala index 2ac5f9114297..eddd48806c4f 100644 --- a/compiler/src/dotty/tools/dotc/core/Denotations.scala +++ b/compiler/src/dotty/tools/dotc/core/Denotations.scala @@ -622,7 +622,7 @@ object Denotations { tp1.derivedLambdaType( mergeParamNames(tp1, tp2), tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => - infoMeet(p1, p2.subst(tp2, tp1), sym1, sym2, safeIntersection).bounds + infoMeet(p1, p2.subst(tp2, tp1), sym1, sym2, safeIntersection).bounds }, infoMeet(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2, safeIntersection)) case _ => @@ -674,7 +674,7 @@ object Denotations { tp1.derivedLambdaType( mergeParamNames(tp1, tp2), tp1.paramInfos.zipWithConserve(tp2.paramInfos) { (p1, p2) => - (p1 | p2.subst(tp2, tp1)).bounds + infoJoin(p1, p2.subst(tp2, tp1), sym1, sym2).bounds }, infoJoin(tp1.resultType, tp2.resultType.subst(tp2, tp1), sym1, sym2)) case _ => diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 1a3fd06e9f5e..632cf6b4d465 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1648,7 +1648,7 @@ class TypeComparer(initctx: Context) extends ConstraintHandling { tp1.derivedRefinedType(tp1.parent & tp2.parent, tp1.refinedName, jointInfo) } catch { - case ex: MergeError => tp1.parent & tp2.parent + case ex: MergeError => NoType } case _ => NoType