From a839c0319aa0df6b70c6d4bd9cb7b42d11ca7546 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Thu, 13 Jul 2023 17:21:40 +0200 Subject: [PATCH 1/5] Simplify polyFunction logic in TypeComparer --- .../src/dotty/tools/dotc/core/TypeComparer.scala | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index e83cefb570cb..fc6d5a78484a 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -666,17 +666,10 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling isSubType(info1, info2) if defn.isFunctionType(tp2) then - if tp2.derivesFrom(defn.PolyFunctionClass) then - // TODO should we handle ErasedFunction is this same way? - tp1.member(nme.apply).info match - case info1: PolyType => - return isSubInfo(info1, tp2.refinedInfo) - case _ => - else - tp1w.widenDealias match - case tp1: RefinedType => - return isSubInfo(tp1.refinedInfo, tp2.refinedInfo) - case _ => + tp1w.widenDealias match + case tp1: RefinedType => + return isSubInfo(tp1.refinedInfo, tp2.refinedInfo) + case _ => val skipped2 = skipMatching(tp1w, tp2) if (skipped2 eq tp2) || !Config.fastPathForRefinedSubtype then From 47f4519401e739219bdc90664f4ecf5aabd6882c Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Fri, 14 Jul 2023 17:29:07 +0200 Subject: [PATCH 2/5] Merge the regular and capture-checking version of isSubInfo The isSubInfo for capture-checking introduced in #15877 had the following TODO: // A relaxed version of subtyping for dependent functions where method types // are treated as contravariant. // TODO: Merge with isSubInfo in hasMatchingMember. Currently, we can't since // the isSubinfo of hasMatchingMember has problems dealing with PolyTypes // (---> orphan params during pickling) The orphan params error was due to the recursion on the result of the PolyTypes being done without first calling `compareTypeLambda`. After fixing this we can safely merge the two versions while keeping the new behavior for dependent and polymorphic function types hidden under the `captureChecking` feature since they're language changes. I'll open a separate PR to create a `relaxedSubtyping` feature for these improvements since their usefulness is independent of capture checking. The isSubInfo for capture-checking got two additional cases involving CapturingTypes in 3e690a80a914f9a950c2fbe2edaf80c9337eeb47, I don't know what they're supposed to do, but moving those to the regular isSubInfo breaks various capture-checking tests: -- [E007] Type Mismatch Error: tests/pos-custom-args/captures/classes.scala:11:32 -------------------------------------- 11 | val c1: C{val n: B^{x}}^{x} = c0 | ^^ | Found: (c0 : C{val n: B^}^{x}) | Required: C{val n: B^{x}}^{x} So this commit breaks capture-checking and I don't know enough about what this code is supposed to do to fix it. --- .../dotty/tools/dotc/core/TypeComparer.scala | 68 +++++++++---------- 1 file changed, 32 insertions(+), 36 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index fc6d5a78484a..b4cb8121b03c 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -10,7 +10,7 @@ import TypeOps.refineUsingParent import collection.mutable import util.{Stats, NoSourcePosition, EqHashMap} import config.Config -import config.Feature.migrateTo3 +import config.Feature.{ccEnabled, migrateTo3} import config.Printers.{subtyping, gadts, matchTypes, noPrinter} import TypeErasure.{erasedLub, erasedGlb} import TypeApplications._ @@ -641,36 +641,6 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling def compareRefined: Boolean = val tp1w = tp1.widen - if ctx.phase == Phases.checkCapturesPhase then - - // A relaxed version of subtyping for dependent functions where method types - // are treated as contravariant. - // TODO: Merge with isSubInfo in hasMatchingMember. Currently, we can't since - // the isSubinfo of hasMatchingMember has problems dealing with PolyTypes - // (---> orphan params during pickling) - def isSubInfo(info1: Type, info2: Type): Boolean = (info1, info2) match - case (info1: PolyType, info2: PolyType) => - info1.paramNames.hasSameLengthAs(info2.paramNames) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) - case (info1: MethodType, info2: MethodType) => - matchingMethodParams(info1, info2, precise = false) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1)) - case (info1 @ CapturingType(parent1, refs1), info2: Type) => - subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) - && isSubInfo(parent1, info2) - case (info1: Type, CapturingType(parent2, refs2)) => - val refs1 = info1.captureSet - (refs1.isAlwaysEmpty || subCaptures(refs1, refs2, frozenConstraint).isOK) && sameBoxed(info1, info2, refs1) - && isSubInfo(info1, parent2) - case _ => - isSubType(info1, info2) - - if defn.isFunctionType(tp2) then - tp1w.widenDealias match - case tp1: RefinedType => - return isSubInfo(tp1.refinedInfo, tp2.refinedInfo) - case _ => - val skipped2 = skipMatching(tp1w, tp2) if (skipped2 eq tp2) || !Config.fastPathForRefinedSubtype then if containsAnd(tp1) then @@ -2001,8 +1971,15 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // conceivably dispatch without knowing precise parameter signatures. One can signal // this by inheriting from the `scala.reflect.SignatureCanBeImprecise` marker trait, // in which case the signature test is elided. + // + // Additionally, for refined function types we do not need to check + // signatures since they're erased in a special way, so we skip the + // signature check but only under `ccEnabled` for now. + // TODO: add a new relaxedSubtyping feature for this (and make a SIP) + // since this is useful in general, but is a language change. def sigsOK(symInfo: Type, info2: Type) = - tp2.underlyingClassRef(refinementOK = true).member(name).exists + (ccEnabled && defn.isFunctionType(tp2)) + || tp2.underlyingClassRef(refinementOK = true).member(name).exists || tp2.derivesFrom(defn.WithoutPreciseParameterTypesClass) || symInfo.isInstanceOf[MethodType] && symInfo.signature.consistentParams(info2.signature) @@ -2014,17 +1991,36 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // but under the condition that signatures might have to match (see sigsOK) // This relaxed version is needed to correctly compare dependent function types. // See pos/i12211.scala. - def isSubInfo(info1: Type, info2: Type, symInfo: Type): Boolean = + def isSubInfo(info1: Type, info2: Type, symInfo1: Type): Boolean = + def fallback = inFrozenGadtIf(tp1IsSingleton) { isSubType(info1, info2) } info2 match + case info2: PolyType if ccEnabled => // See TODO about `ccEnabled` in `sigsOK`, this should also go under `relaxedSubtyping`. + info1 match + case info1: PolyType => + comparingTypeLambdas(info1, info2): + info1.paramNames.hasSameLengthAs(info2.paramNames) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType) + // Signature checks are never necessary because polymorphic + // refinements are only allowed for the `apply` method of a + // PolyFunction. + case _ => fallback case info2: MethodType => info1 match case info1: MethodType => - val symInfo1 = symInfo.stripPoly matchingMethodParams(info1, info2, precise = false) && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType) && sigsOK(symInfo1, info2) - case _ => inFrozenGadtIf(tp1IsSingleton) { isSubType(info1, info2) } - case _ => inFrozenGadtIf(tp1IsSingleton) { isSubType(info1, info2) } + case _ => fallback + case info2 @ CapturingType(parent2, refs2) if ctx.phase == Phases.checkCapturesPhase => + val refs1 = info1.captureSet + (refs1.isAlwaysEmpty || subCaptures(refs1, refs2, frozenConstraint).isOK) && sameBoxed(info1, info2, refs1) + && isSubInfo(info1, parent2, symInfo1) + case _ => + info1 match + case info1 @ CapturingType(parent1, refs1) if ctx.phase == Phases.checkCapturesPhase => + subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) + && isSubInfo(parent1, info2, symInfo1) + case _ => fallback def qualifies(m: SingleDenotation): Boolean = val info2 = tp2.refinedInfo From dd21be8979b2bed59a70450fd3acfdefd8e11a67 Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Wed, 19 Jul 2023 03:42:37 +0800 Subject: [PATCH 3/5] Tweak isSubInfo for capture checking - Keep the shortcut for function types in compareRefined - Temper isSubInfo logic --- .../dotty/tools/dotc/core/TypeComparer.scala | 85 ++++++++++--------- 1 file changed, 47 insertions(+), 38 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index b4cb8121b03c..0a45723ac491 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -641,6 +641,14 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling def compareRefined: Boolean = val tp1w = tp1.widen + if ctx.phase == Phases.checkCapturesPhase then + def compareInfo(info1: Type, info2: Type): Boolean = + isSubInfo(info1, info2, NoType) + if defn.isFunctionType(tp2) then + tp1w.widenDealias match + case tp1: RefinedType => return compareInfo(tp1.refinedInfo, tp2.refinedInfo) + case _ => + val skipped2 = skipMatching(tp1w, tp2) if (skipped2 eq tp2) || !Config.fastPathForRefinedSubtype then if containsAnd(tp1) then @@ -1917,6 +1925,40 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling case _ => refines.map(RefinedType(tp, _, _): Type).reduce(AndType(_, _)) + // A relaxed version of isSubType, which compares method types + // under the standard arrow rule which is contravariant in the parameter types, + // but under the condition that signatures might have to match (see sigsOK) + // This relaxed version is needed to correctly compare dependent function types. + // See pos/i12211.scala. + def isSubInfo(info1: Type, info2: Type, symInfo1: Type, sigsOK: ((Type, Type) => Boolean) | Null = null, fallbackFn: ((Type, Type) => Boolean) | Null = null): Boolean = trace(i"isSubInfo $info1 <:< $info2"): + def fallback = + if fallbackFn != null then fallbackFn(info1, info2) + else isSubType(info1, info2) + val isCCPhase = ctx.phase == Phases.checkCapturesPhase + + (info1, info2) match + case (info1: PolyType, info2: PolyType) if ccEnabled => // See TODO about `ccEnabled` in `sigsOK`, this should also go under `relaxedSubtyping`. + comparingTypeLambdas(info1, info2): + info1.paramNames.hasSameLengthAs(info2.paramNames) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) + // Signature checks are never necessary because polymorphic + // refinements are only allowed for the `apply` method of a + // PolyFunction. + case (info1: MethodType, info2: MethodType) => + matchingMethodParams(info1, info2, precise = false) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) + && (if sigsOK != null then sigsOK(symInfo1, info2) else true) + case _ => (info1.widenDealias, info2) match + case (CapturingType(parent1, _), info2: Type) if isCCPhase => + val refs1 = info1.captureSet + subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) + && isSubInfo(parent1, info2, symInfo1, sigsOK, fallbackFn) + case (info1: Type, CapturingType(parent2, refs2)) if isCCPhase => + val refs1 = info1.captureSet + (refs1.isAlwaysEmpty || subCaptures(refs1, refs2, frozenConstraint).isOK) && sameBoxed(info1, info2, refs1) + && isSubInfo(info1, parent2, symInfo1, sigsOK, fallbackFn) + case _ => fallback + /** Can comparing this type on the left lead to an either? This is the case if * the type is and AndType or contains embedded occurrences of AndTypes */ @@ -1984,43 +2026,10 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling || symInfo.isInstanceOf[MethodType] && symInfo.signature.consistentParams(info2.signature) - def tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType] - - // A relaxed version of isSubType, which compares method types - // under the standard arrow rule which is contravariant in the parameter types, - // but under the condition that signatures might have to match (see sigsOK) - // This relaxed version is needed to correctly compare dependent function types. - // See pos/i12211.scala. - def isSubInfo(info1: Type, info2: Type, symInfo1: Type): Boolean = - def fallback = inFrozenGadtIf(tp1IsSingleton) { isSubType(info1, info2) } - info2 match - case info2: PolyType if ccEnabled => // See TODO about `ccEnabled` in `sigsOK`, this should also go under `relaxedSubtyping`. - info1 match - case info1: PolyType => - comparingTypeLambdas(info1, info2): - info1.paramNames.hasSameLengthAs(info2.paramNames) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType) - // Signature checks are never necessary because polymorphic - // refinements are only allowed for the `apply` method of a - // PolyFunction. - case _ => fallback - case info2: MethodType => - info1 match - case info1: MethodType => - matchingMethodParams(info1, info2, precise = false) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType) - && sigsOK(symInfo1, info2) - case _ => fallback - case info2 @ CapturingType(parent2, refs2) if ctx.phase == Phases.checkCapturesPhase => - val refs1 = info1.captureSet - (refs1.isAlwaysEmpty || subCaptures(refs1, refs2, frozenConstraint).isOK) && sameBoxed(info1, info2, refs1) - && isSubInfo(info1, parent2, symInfo1) - case _ => - info1 match - case info1 @ CapturingType(parent1, refs1) if ctx.phase == Phases.checkCapturesPhase => - subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) - && isSubInfo(parent1, info2, symInfo1) - case _ => fallback + def fallback = (info1: Type, info2: Type) => + def tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType] + inFrozenGadtIf(tp1IsSingleton): + isSubType(info1, info2) def qualifies(m: SingleDenotation): Boolean = val info2 = tp2.refinedInfo @@ -2035,7 +2044,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // OK{ { def x(): T } <: { def x: T} // if x is Java defined ExprType(info1.resType) case info1 => info1 - isSubInfo(info1, info2, m.symbol.info.orElse(info1)) + isSubInfo(info1, info2, m.symbol.info.orElse(info1), sigsOK, fallback) || matchAbstractTypeMember(m.info) || (tp1.isStable && m.symbol.isStableMember && isSubType(TermRef(tp1, m.symbol), tp2.refinedInfo)) From e4fc9e467d03df3f0fc2683e274f6401f07a851f Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Wed, 19 Jul 2023 09:40:02 +0800 Subject: [PATCH 4/5] Use nested pattern matching in isSubInfo instead of matching a tuple. Suggested by @smarter. --- .../dotty/tools/dotc/core/TypeComparer.scala | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 0a45723ac491..1b1b525e5858 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -1931,34 +1931,39 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling // This relaxed version is needed to correctly compare dependent function types. // See pos/i12211.scala. def isSubInfo(info1: Type, info2: Type, symInfo1: Type, sigsOK: ((Type, Type) => Boolean) | Null = null, fallbackFn: ((Type, Type) => Boolean) | Null = null): Boolean = trace(i"isSubInfo $info1 <:< $info2"): - def fallback = - if fallbackFn != null then fallbackFn(info1, info2) - else isSubType(info1, info2) - val isCCPhase = ctx.phase == Phases.checkCapturesPhase - - (info1, info2) match - case (info1: PolyType, info2: PolyType) if ccEnabled => // See TODO about `ccEnabled` in `sigsOK`, this should also go under `relaxedSubtyping`. - comparingTypeLambdas(info1, info2): - info1.paramNames.hasSameLengthAs(info2.paramNames) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) - // Signature checks are never necessary because polymorphic - // refinements are only allowed for the `apply` method of a - // PolyFunction. - case (info1: MethodType, info2: MethodType) => - matchingMethodParams(info1, info2, precise = false) - && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) - && (if sigsOK != null then sigsOK(symInfo1, info2) else true) - case _ => (info1.widenDealias, info2) match - case (CapturingType(parent1, _), info2: Type) if isCCPhase => - val refs1 = info1.captureSet - subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) - && isSubInfo(parent1, info2, symInfo1, sigsOK, fallbackFn) - case (info1: Type, CapturingType(parent2, refs2)) if isCCPhase => + def fallback = if fallbackFn != null then fallbackFn(info1, info2) else isSubType(info1, info2) + + def tryCapturing = info1.widenDealias match + case CapturingType(parent1, _) => + val refs1 = info1.captureSet + subCaptures(refs1, info2.captureSet, frozenConstraint).isOK && sameBoxed(info1, info2, refs1) + && isSubInfo(parent1, info2, symInfo1, sigsOK, fallbackFn) + case _ => info2.widenDealias match + case CapturingType(parent2, _) => val refs1 = info1.captureSet + val refs2 = info2.captureSet (refs1.isAlwaysEmpty || subCaptures(refs1, refs2, frozenConstraint).isOK) && sameBoxed(info1, info2, refs1) && isSubInfo(info1, parent2, symInfo1, sigsOK, fallbackFn) case _ => fallback + info2 match + case info2: PolyType if ccEnabled => info1 match // See TODO about `ccEnabled` in `sigsOK`, this should also go under `relaxedSubtyping`. + case info1: PolyType => + comparingTypeLambdas(info1, info2): + info1.paramNames.hasSameLengthAs(info2.paramNames) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) + // Signature checks are never necessary because polymorphic + // refinements are only allowed for the `apply` method of a + // PolyFunction. + case _ => tryCapturing + case info2: MethodType => info1 match + case info1: MethodType => + matchingMethodParams(info1, info2, precise = false) + && isSubInfo(info1.resultType, info2.resultType.subst(info2, info1), symInfo1.resultType, sigsOK, fallbackFn) + && (if sigsOK != null then sigsOK(symInfo1, info2) else true) + case _ => tryCapturing + case _ => tryCapturing + /** Can comparing this type on the left lead to an either? This is the case if * the type is and AndType or contains embedded occurrences of AndTypes */ From 01e3397af6cddcd20e89bde5e889e55f7468323f Mon Sep 17 00:00:00 2001 From: Yichen Xu Date: Wed, 19 Jul 2023 09:44:35 +0800 Subject: [PATCH 5/5] Address review comments - Document the shortcut in compareRefined for function types under CC phase - Change a `def` to a `val` --- compiler/src/dotty/tools/dotc/core/TypeComparer.scala | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala index 1b1b525e5858..f28f6c40b819 100644 --- a/compiler/src/dotty/tools/dotc/core/TypeComparer.scala +++ b/compiler/src/dotty/tools/dotc/core/TypeComparer.scala @@ -641,6 +641,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling def compareRefined: Boolean = val tp1w = tp1.widen + // FIXME: this shortcut is currently necessary to correctly compare function types in CC phase. + // Going to `hasMatchingMember` breaks tests (e.g. neg-custom-args/captures/lazyref.scala). See #18200. if ctx.phase == Phases.checkCapturesPhase then def compareInfo(info1: Type, info2: Type): Boolean = isSubInfo(info1, info2, NoType) @@ -2032,7 +2034,7 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling && symInfo.signature.consistentParams(info2.signature) def fallback = (info1: Type, info2: Type) => - def tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType] + val tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType] inFrozenGadtIf(tp1IsSingleton): isSubType(info1, info2)