Skip to content

Optimize lub algorithm #20034

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 4 commits into from
Mar 29, 2024
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
133 changes: 68 additions & 65 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2352,8 +2352,8 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
}

/** The greatest lower bound of two types */
def glb(tp1: Type, tp2: Type): Type = /*>|>*/ trace(s"glb(${tp1.show}, ${tp2.show})", subtyping, show = true) /*<|<*/ {
if (tp1 eq tp2) tp1
def glb(tp1: Type, tp2: Type): Type = // trace(s"glb(${tp1.show}, ${tp2.show})", subtyping, show = true):
if tp1 eq tp2 then tp1
else if !tp1.exists || (tp1 eq WildcardType) then tp2
else if !tp2.exists || (tp2 eq WildcardType) then tp1
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp2
Expand All @@ -2366,12 +2366,12 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
val tp2a = dropIfSuper(tp2, tp1)
if tp2a ne tp2 then glb(tp1, tp2a)
else tp2 match // normalize to disjunctive normal form if possible.
case tp2 @ OrType(tp21, tp22) =>
lub(tp1 & tp21, tp1 & tp22, isSoft = tp2.isSoft)
case tp2 @ OrType(tp2L, tp2R) =>
lub(tp1 & tp2L, tp1 & tp2R, isSoft = tp2.isSoft)
case _ =>
tp1 match
case tp1 @ OrType(tp11, tp12) =>
lub(tp11 & tp2, tp12 & tp2, isSoft = tp1.isSoft)
case tp1 @ OrType(tp1L, tp1R) =>
lub(tp1L & tp2, tp1R & tp2, isSoft = tp1.isSoft)
case tp1: ConstantType =>
tp2 match
case tp2: ConstantType =>
Expand All @@ -2386,8 +2386,10 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
NothingType
case _ => andType(tp1, tp2)
case _ => andType(tp1, tp2)
end mergedGlb

mergedGlb(dropExpr(tp1.stripLazyRef), dropExpr(tp2.stripLazyRef))
}
end glb

def widenInUnions(using Context): Boolean =
migrateTo3 || ctx.erasedTypes
Expand All @@ -2396,14 +2398,23 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
* @param canConstrain If true, new constraints might be added to simplify the lub.
* @param isSoft If the lub is a union, this determines whether it's a soft union.
*/
def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = /*>|>*/ trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true) /*<|<*/ {
if (tp1 eq tp2) tp1
def lub(tp1: Type, tp2: Type, canConstrain: Boolean = false, isSoft: Boolean = true): Type = // trace(s"lub(${tp1.show}, ${tp2.show}, canConstrain=$canConstrain, isSoft=$isSoft)", subtyping, show = true):
if tp1 eq tp2 then tp1
else if !tp1.exists || (tp2 eq WildcardType) then tp1
else if !tp2.exists || (tp1 eq WildcardType) then tp2
else if tp1.isAny && !tp2.isLambdaSub || tp1.isAnyKind || isBottom(tp2) then tp1
else if tp2.isAny && !tp1.isLambdaSub || tp2.isAnyKind || isBottom(tp1) then tp2
else
def mergedLub(tp1: Type, tp2: Type): Type = {
def mergedLub(tp1: Type, tp2: Type): Type =
// First, if tp1 and tp2 are the same singleton type, return one of them.
if tp1.isSingleton && isSubType(tp1, tp2, whenFrozen = !canConstrain) then
return tp2
if tp2.isSingleton && isSubType(tp2, tp1, whenFrozen = !canConstrain) then
return tp1

// Second, handle special cases when tp1 and tp2 are disjunctions of
// singleton types. This saves time otherwise spent in
// costly subtype comparisons performed in dropIfSub below.
tp1.atoms match
case Atoms.Range(lo1, hi1) if !widenInUnions =>
tp2.atoms match
Expand All @@ -2413,20 +2424,24 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
if (hi1 & hi2).isEmpty then return orType(tp1, tp2, isSoft = isSoft)
case none =>
case none =>
val t1 = mergeIfSuper(tp1, tp2, canConstrain)
if (t1.exists) return t1

val t2 = mergeIfSuper(tp2, tp1, canConstrain)
if (t2.exists) return t2

def widen(tp: Type) = if (widenInUnions) tp.widen else tp.widenIfUnstable
// Third, try to simplify after widening as follows:
// 1. Drop all or-factors in tp2 that are subtypes of an or-factor
// in tp1, yielding tp2Final.
// 2. Drop all or-factors in tp1 that are subtypes of an or-factor
// in tp2Final, yielding tp1Final.
// 3. Combine the two final types in an OrType
def widen(tp: Type) =
if widenInUnions then tp.widen else tp.widenIfUnstable
val tp1w = widen(tp1)
val tp2w = widen(tp2)
if ((tp1 ne tp1w) || (tp2 ne tp2w)) lub(tp1w, tp2w, canConstrain = canConstrain, isSoft = isSoft)
else orType(tp1w, tp2w, isSoft = isSoft) // no need to check subtypes again
}
val tp2Final = dropIfSub(tp2w, tp1w, canConstrain)
val tp1Final = dropIfSub(tp1w, tp2Final, canConstrain)
recombine(tp1Final, tp2Final, orType(_, _, isSoft = isSoft))
end mergedLub

mergedLub(dropExpr(tp1.stripLazyRef), dropExpr(tp2.stripLazyRef))
}
end lub

/** Try to produce joint arguments for a lub `A[T_1, ..., T_n] | A[T_1', ..., T_n']` using
* the following strategies:
Expand Down Expand Up @@ -2488,60 +2503,48 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
Nil
}

private def recombineAnd(tp: AndType, tp1: Type, tp2: Type) =
if (!tp1.exists) tp2
else if (!tp2.exists) tp1
else tp.derivedAndType(tp1, tp2)
private def recombine(tp1: Type, tp2: Type, rebuild: (Type, Type) => Type): Type =
if !tp1.exists then tp2
else if !tp2.exists then tp1
else rebuild(tp1, tp2)

private def recombine(tp1: Type, tp2: Type, tp: AndOrType): Type =
recombine(tp1, tp2, tp.derivedAndOrType)

/** If some (&-operand of) `tp` is a supertype of `sub` replace it with `NoType`.
*/
private def dropIfSuper(tp: Type, sub: Type): Type =
if (isSubTypeWhenFrozen(sub, tp)) NoType
else tp match {

def isSuperOf(sub: Type): Boolean = sub match
case AndType(sub1, sub2) => isSuperOf(sub1) || isSuperOf(sub2)
case sub: TypeVar if sub.isInstantiated => isSuperOf(sub.inst)
case _ => isSubTypeWhenFrozen(sub, tp)

tp match
case tp @ AndType(tp1, tp2) =>
recombineAnd(tp, dropIfSuper(tp1, sub), dropIfSuper(tp2, sub))
recombine(dropIfSuper(tp1, sub), dropIfSuper(tp2, sub), tp)
case tp: TypeVar if tp.isInstantiated =>
dropIfSuper(tp.inst, sub)
case _ =>
tp
}
if isSuperOf(sub) then NoType else tp
end dropIfSuper

/** Merge `t1` into `tp2` if t1 is a subtype of some &-summand of tp2.
*/
private def mergeIfSub(tp1: Type, tp2: Type): Type =
if (isSubTypeWhenFrozen(tp1, tp2)) tp1
else tp2 match {
case tp2 @ AndType(tp21, tp22) =>
val lower1 = mergeIfSub(tp1, tp21)
if (lower1 eq tp21) tp2
else if (lower1.exists) lower1 & tp22
else {
val lower2 = mergeIfSub(tp1, tp22)
if (lower2 eq tp22) tp2
else if (lower2.exists) tp21 & lower2
else NoType
}
case _ =>
NoType
}
/** If some (|-operand of) `tp` is a subtype of `sup` replace it with `NoType`. */
private def dropIfSub(tp: Type, sup: Type, canConstrain: Boolean): Type =
Copy link
Member

Choose a reason for hiding this comment

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

The new operations are much clearer than the original mergeIfs. 👍🏼

Copy link
Member

Choose a reason for hiding this comment

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

Add a comment:

/** If some (|-operand of) `tp` is a subtype of `sup` replace it with `NoType`. */


/** Merge `tp1` into `tp2` if tp1 is a supertype of some |-summand of tp2.
* @param canConstrain If true, new constraints might be added to make the merge possible.
*/
private def mergeIfSuper(tp1: Type, tp2: Type, canConstrain: Boolean): Type =
if (isSubType(tp2, tp1, whenFrozen = !canConstrain)) tp1
else tp2 match {
case tp2 @ OrType(tp21, tp22) =>
val higher1 = mergeIfSuper(tp1, tp21, canConstrain)
if (higher1 eq tp21) tp2
else if (higher1.exists) lub(higher1, tp22, isSoft = tp2.isSoft)
else {
val higher2 = mergeIfSuper(tp1, tp22, canConstrain)
if (higher2 eq tp22) tp2
else if (higher2.exists) lub(tp21, higher2, isSoft = tp2.isSoft)
else NoType
}
def isSubOf(sup: Type): Boolean = sup match
case OrType(sup1, sup2) => isSubOf(sup1) || isSubOf(sup2)
case sup: TypeVar if sup.isInstantiated => isSubOf(sup.inst)
case _ => isSubType(tp, sup, whenFrozen = !canConstrain)

tp match
case tp @ OrType(tp1, tp2) =>
recombine(dropIfSub(tp1, sup, canConstrain), dropIfSub(tp2, sup, canConstrain), tp)
case tp: TypeVar if tp.isInstantiated =>
dropIfSub(tp.inst, sup, canConstrain)
case _ =>
NoType
}
if isSubOf(sup) then NoType else tp
end dropIfSub

/** There's a window of vulnerability between ElimByName and Erasure where some
* ExprTypes `=> T` that appear as parameters of function types are not yet converted
Expand Down
11 changes: 2 additions & 9 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -157,15 +157,8 @@ object TypeOps:
tp.derivedAlias(simplify(tp.alias, theMap))
case AndType(l, r) if !ctx.mode.is(Mode.Type) =>
simplify(l, theMap) & simplify(r, theMap)

Choose a reason for hiding this comment

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

Should this be calling into TypeComparer.glb It just seems odd to be altering the Or case, but not the And case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

& and glb are in fact the same. It's just for lub that we need the additional isSoft parameter.

case tp @ OrType(l, r)
if !ctx.mode.is(Mode.Type)
&& (tp.isSoft || l.isBottomType || r.isBottomType) =>
// Normalize A | Null and Null | A to A even if the union is hard (i.e.
// explicitly declared), but not if -Yexplicit-nulls is set. The reason is
// that in this case the normal asSeenFrom machinery is not prepared to deal
// with Nulls (which have no base classes). Under -Yexplicit-nulls, we take
// corrective steps, so no widening is wanted.
simplify(l, theMap) | simplify(r, theMap)
case tp @ OrType(l, r) if !ctx.mode.is(Mode.Type) =>
TypeComparer.lub(simplify(l, theMap), simplify(r, theMap), isSoft = tp.isSoft)
case tp @ CapturingType(parent, refs) =>
if !ctx.mode.is(Mode.Type)
&& refs.subCaptures(parent.captureSet, frozen = true).isOK
Expand Down
11 changes: 5 additions & 6 deletions compiler/src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3598,12 +3598,11 @@ object Types extends TypeUtils {

override def widenUnionWithoutNull(using Context): Type =
if myUnionPeriod != ctx.period then
myUnion =
if isSoft then
TypeComparer.lub(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = true, isSoft = isSoft) match
case union: OrType => union.join
case res => res
else derivedOrType(tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, soft = isSoft)
val union = TypeComparer.lub(
tp1.widenUnionWithoutNull, tp2.widenUnionWithoutNull, canConstrain = isSoft, isSoft = isSoft)
myUnion = union match
case union: OrType if isSoft => union.join
case _ => union
if !isProvisional then myUnionPeriod = ctx.period
myUnion

Expand Down
3 changes: 2 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Namer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1961,7 +1961,8 @@ class Namer { typer: Typer =>
else
// don't strip @uncheckedVariance annot for default getters
TypeOps.simplify(tp.widenTermRefExpr,
if defaultTp.exists then TypeOps.SimplifyKeepUnchecked() else null) match
if defaultTp.exists then TypeOps.SimplifyKeepUnchecked() else null)
match
case ctp: ConstantType if sym.isInlineVal => ctp
case tp => TypeComparer.widenInferred(tp, pt, widenUnions = true)

Expand Down
8 changes: 8 additions & 0 deletions tests/pos/i10693.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
def test[A, B](a: A, b: B): A | B = a
val v0 = test("string", 1)
val v1 = test(1, "string")
val v2 = test(v0, v1)
val v3 = test(v1, v0)
val v4 = test(v2, v3)
val v5 = test(v3, v2)
val v6 = test(v4, v5)
4 changes: 2 additions & 2 deletions tests/semanticdb/metac.expect
Original file line number Diff line number Diff line change
Expand Up @@ -2020,7 +2020,7 @@ Symbols:
example/InstrumentTyper# => class InstrumentTyper extends Object { self: AnyRef & InstrumentTyper => +5 decls }
example/InstrumentTyper#AnnotatedType# => type AnnotatedType = Int @param
example/InstrumentTyper#`<init>`(). => primary ctor <init> (): InstrumentTyper
example/InstrumentTyper#all(). => method all => List[Float | Double | List[Nothing] | Boolean | Unit | Char | String | LinkOption | Int | Long | Class[Option[Int]]]
example/InstrumentTyper#all(). => method all => List[Char | String | LinkOption | Int | Long | Class[Option[Int]] | Float | Double | Boolean | Unit | List[Nothing]]
example/InstrumentTyper#clazzOf. => final val method clazzOf Option[Int]
example/InstrumentTyper#singletonType(). => method singletonType (param x: Predef.type): Nothing
example/InstrumentTyper#singletonType().(x) => param x: Predef.type
Expand Down Expand Up @@ -2082,7 +2082,7 @@ Occurrences:
[24:37..24:40): Int -> scala/Int#

Synthetics:
[8:12..8:16):List => *.apply[Float | Double | List[Nothing] | Boolean | Unit | Char | String | LinkOption | Int | Long | Class[Option[Int]]]
[8:12..8:16):List => *.apply[Char | String | LinkOption | Int | Long | Class[Option[Int]] | Float | Double | Boolean | Unit | List[Nothing]]
[20:4..20:8):List => *.apply[Nothing]

expect/InventedNames.scala
Expand Down