Skip to content

Simplify polyfunction logic #18200

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.

Sign up for GitHub

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

Closed
wants to merge 5 commits into from
Closed

Simplify polyfunction logic #18200

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a839c03
Simplify polyFunction logic in TypeComparer
nicolasstucki Jul 13, 2023
47f4519
Merge the regular and capture-checking version of isSubInfo
smarter Jul 14, 2023
dd21be8
Tweak isSubInfo for capture checking
Linyxus Jul 18, 2023
e4fc9e4
Use nested pattern matching in isSubInfo
Linyxus Jul 19, 2023
01e3397
Address review comments
Linyxus Jul 19, 2023
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
108 changes: 53 additions & 55 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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._
Expand Down Expand Up @@ -642,41 +642,12 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
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)

def compareInfo(info1: Type, info2: Type): Boolean =
isSubInfo(info1, info2, NoType)
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 compareInfo(tp1.refinedInfo, tp2.refinedInfo)
case _ =>

val skipped2 = skipMatching(tp1w, tp2)
if (skipped2 eq tp2) || !Config.fastPathForRefinedSubtype then
Expand Down Expand Up @@ -1954,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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous version used two pattern matches on info1 and info2 instead of a pattern match on a tuple for performance, I suggest keeping the two pattern matches since this code path could be hot.

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
*/
Expand Down Expand Up @@ -2008,30 +2013,23 @@ 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)

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, symInfo: Type): Boolean =
info2 match
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) }
def fallback = (info1: Type, info2: Type) =>
def tp1IsSingleton: Boolean = tp1.isInstanceOf[SingletonType]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using a def here instead of a val doesn't seem useful since it's evaluated in the next line

inFrozenGadtIf(tp1IsSingleton):
isSubType(info1, info2)

def qualifies(m: SingleDenotation): Boolean =
val info2 = tp2.refinedInfo
Expand All @@ -2046,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))

Expand Down