Skip to content
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

Remove unnecessary and recursive Space decomposition #19216

Merged
merged 4 commits into from
Dec 18, 2023
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
8 changes: 5 additions & 3 deletions compiler/src/dotty/tools/dotc/core/TypeComparer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2841,10 +2841,12 @@ class TypeComparer(@constructorOnly initctx: Context) extends ConstraintHandling
case (tp1: TypeRef, tp2: TypeRef) if tp1.symbol.isClass && tp2.symbol.isClass =>
val cls1 = tp1.classSymbol
val cls2 = tp2.classSymbol
def isDecomposable(tp: Symbol): Boolean =
tp.is(Sealed) && !tp.hasAnonymousChild
val sameKind = tp1.hasSameKindAs(tp2)
def isDecomposable(sym: Symbol): Boolean =
sameKind && sym.is(Sealed) && !sym.hasAnonymousChild
def decompose(sym: Symbol, tp: Type): List[Type] =
sym.children.map(x => refineUsingParent(tp, x)).filter(_.exists)
val tpSimple = tp.applyIfParameterized(tp.typeParams.map(_ => WildcardType))
sym.children.map(x => refineUsingParent(tpSimple, x)).filter(_.exists)
if (cls1.derivesFrom(cls2) || cls2.derivesFrom(cls1))
false
else
Expand Down
45 changes: 25 additions & 20 deletions compiler/src/dotty/tools/dotc/core/TypeOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -839,33 +839,34 @@ object TypeOps:
}
}

/** Gather GADT symbols and `ThisType`s found in `tp2`, ie. the scrutinee. */
/** Gather GADT symbols and singletons found in `tp2`, ie. the scrutinee. */
object TraverseTp2 extends TypeTraverser:
val thisTypes = util.HashSet[ThisType]()
val gadtSyms = new mutable.ListBuffer[Symbol]
val singletons = util.HashMap[Symbol, SingletonType]()
val gadtSyms = new mutable.ListBuffer[Symbol]

def traverse(tp: Type) = {
def traverse(tp: Type) = try
val tpd = tp.dealias
if tpd ne tp then traverse(tpd)
else tp match
case tp: ThisType if !tp.tref.symbol.isStaticOwner && !thisTypes.contains(tp) =>
thisTypes += tp
case tp: ThisType if !singletons.contains(tp.tref.symbol) && !tp.tref.symbol.isStaticOwner =>
singletons(tp.tref.symbol) = tp
traverseChildren(tp.tref)
case tp: TypeRef if tp.symbol.isAbstractOrParamType =>
case tp: TermRef if tp.symbol.is(Param) =>
singletons(tp.typeSymbol) = tp
traverseChildren(tp)
case tp: TypeRef if !gadtSyms.contains(tp.symbol) && tp.symbol.isAbstractOrParamType =>
gadtSyms += tp.symbol
traverseChildren(tp)
val owners = Iterator.iterate(tp.symbol)(_.maybeOwner).takeWhile(_.exists)
for sym <- owners do
// add ThisType's for the classes symbols in the ownership of `tp`
// for example, i16451.CanForward.scala, add `Namer.this`, as one of the owners of the type parameter `A1`
if sym.isClass && !sym.isAnonymousClass && !sym.isStaticOwner then
traverse(sym.thisType)
// traverse abstract type infos, to add any singletons
// for example, i16451.CanForward.scala, add `Namer.this`, from the info of the type parameter `A1`
// also, i19031.ci-reg2.scala, add `out`, from the info of the type parameter `A1` (from synthetic applyOrElse)
traverseChildren(tp.info)
case _ =>
traverseChildren(tp)
}
catch case ex: Throwable => handleRecursive("traverseTp2", tp.show, ex)
TraverseTp2.traverse(tp2)
val thisTypes = TraverseTp2.thisTypes
val gadtSyms = TraverseTp2.gadtSyms.toList
val singletons = TraverseTp2.singletons
val gadtSyms = TraverseTp2.gadtSyms.toList

// Prefix inference, given `p.C.this.Child`:
// 1. return it as is, if `C.this` is found in `tp`, i.e. the scrutinee; or
Expand All @@ -875,10 +876,13 @@ object TypeOps:
class InferPrefixMap extends TypeMap {
var prefixTVar: Type | Null = null
def apply(tp: Type): Type = tp match {
case tp @ ThisType(tref) if !tref.symbol.isStaticOwner =>
case tp: TermRef if singletons.contains(tp.symbol) =>
prefixTVar = singletons(tp.symbol) // e.g. tests/pos/i19031.ci-reg2.scala, keep out
prefixTVar.uncheckedNN
case ThisType(tref) if !tref.symbol.isStaticOwner =>
val symbol = tref.symbol
if thisTypes.contains(tp) then
prefixTVar = tp // e.g. tests/pos/i16785.scala, keep Outer.this
if singletons.contains(symbol) then
prefixTVar = singletons(symbol) // e.g. tests/pos/i16785.scala, keep Outer.this
prefixTVar.uncheckedNN
else if symbol.is(Module) then
TermRef(this(tref.prefix), symbol.sourceModule)
Expand Down Expand Up @@ -913,7 +917,8 @@ object TypeOps:
}

def instantiate(): Type = {
for tp <- mixins.reverseIterator do protoTp1 <:< tp
for tp <- mixins.reverseIterator do
protoTp1 <:< tp
maximizeType(protoTp1, NoSpan)
wildApprox(protoTp1)
}
Expand Down
3 changes: 1 addition & 2 deletions compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,7 @@ class PatternMatcher extends MiniPhase {

if !inInlinedCode then
// check exhaustivity and unreachability
SpaceEngine.checkExhaustivity(tree)
SpaceEngine.checkRedundancy(tree)
SpaceEngine.checkMatch(tree)

translated.ensureConforms(matchType)
}
Expand Down
14 changes: 7 additions & 7 deletions compiler/src/dotty/tools/dotc/transform/patmat/Space.scala
Original file line number Diff line number Diff line change
Expand Up @@ -210,17 +210,13 @@ object SpaceEngine {
case (a @ Typ(tp1, _), b @ Typ(tp2, _)) =>
if isSubType(tp1, tp2) then a
else if isSubType(tp2, tp1) then b
else if canDecompose(a) then intersect(Or(decompose(a)), b)
else if canDecompose(b) then intersect(a, Or(decompose(b)))
else intersectUnrelatedAtomicTypes(tp1, tp2)(a)
case (a @ Typ(tp1, _), Prod(tp2, fun, ss)) =>
if isSubType(tp2, tp1) then b
else if canDecompose(a) then intersect(Or(decompose(a)), b)
else if isSubType(tp1, tp2) then a // problematic corner case: inheriting a case class
else intersectUnrelatedAtomicTypes(tp1, tp2)(b)
case (Prod(tp1, fun, ss), b @ Typ(tp2, _)) =>
if isSubType(tp1, tp2) then a
else if canDecompose(b) then intersect(a, Or(decompose(b)))
else if isSubType(tp2, tp1) then a // problematic corner case: inheriting a case class
else intersectUnrelatedAtomicTypes(tp1, tp2)(a)
case (a @ Prod(tp1, fun1, ss1), Prod(tp2, fun2, ss2)) =>
Expand Down Expand Up @@ -880,7 +876,7 @@ object SpaceEngine {
case _ => tp
})

def checkExhaustivity(m: Match)(using Context): Unit = if exhaustivityCheckable(m.selector) then trace(i"checkExhaustivity($m)", debug) {
def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)", debug) {
val selTyp = toUnderlying(m.selector.tpe).dealias
debug.println(i"selTyp = $selTyp")

Expand All @@ -903,7 +899,7 @@ object SpaceEngine {
report.warning(PatternMatchExhaustivity(showSpaces(deduped), m), m.selector)
}

private def redundancyCheckable(sel: Tree)(using Context): Boolean =
private def reachabilityCheckable(sel: Tree)(using Context): Boolean =
// Ignore Expr[T] and Type[T] for unreachability as a special case.
// Quote patterns produce repeated calls to the same unapply method, but with different implicit parameters.
// Since we assume that repeated calls to the same unapply method overlap
Expand All @@ -913,7 +909,7 @@ object SpaceEngine {
&& !sel.tpe.widen.isRef(defn.QuotedExprClass)
&& !sel.tpe.widen.isRef(defn.QuotedTypeClass)

def checkRedundancy(m: Match)(using Context): Unit = if redundancyCheckable(m.selector) then trace(i"checkRedundancy($m)", debug) {
def checkReachability(m: Match)(using Context): Unit = trace(i"checkReachability($m)", debug) {
val cases = m.cases.toIndexedSeq

val selTyp = toUnderlying(m.selector.tpe).dealias
Expand Down Expand Up @@ -965,4 +961,8 @@ object SpaceEngine {
i += 1
}
}

def checkMatch(m: Match)(using Context): Unit =
if exhaustivityCheckable(m.selector) then checkExhaustivity(m)
if reachabilityCheckable(m.selector) then checkReachability(m)
}
4 changes: 2 additions & 2 deletions scaladoc-js/common/src/utils/html.scala
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ object HTML {
case ("id", id) => elem.id = id
case ("class", value) => value.split("\\s+").foreach(cls => elem.classList.add(cls))
case (attr, value) => elem.setAttribute(attr, value)
case s: Seq[AppliedAttr] => unpackAttributes(s*)
case s: Seq[AppliedAttr @unchecked] => unpackAttributes(s*)
}

unpackTags(tags*)
Expand Down Expand Up @@ -118,4 +118,4 @@ object HTML {
val titleAttr =Attr("title")
val onkeyup = Attr("onkeyup")

}
}
16 changes: 16 additions & 0 deletions tests/pos/i19031.ci-reg1.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//> using options -Werror

sealed trait Mark[T]

trait Foo[T]
class Bar1[T] extends Foo[T]
class Bar2[T] extends Foo[T] with Mark[T]

class Test:
def t1(foo: Foo[Int]): Unit = foo match
case _: Mark[t] =>
case _ =>

def t2[F <: Foo[Int]](foo: F): Unit = foo match
case _: Mark[t] =>
case _ =>
15 changes: 15 additions & 0 deletions tests/pos/i19031.ci-reg2.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//> using options -Werror

trait Outer:
sealed trait Foo
case class Bar1() extends Foo
case class Bar2() extends Foo
case object Bar3 extends Foo

def foos: List[Foo]

class Test:
def t1(out: Outer) = out.foos.collect:
case out.Bar1() => 1
case out.Bar2() => 2
case out.Bar3 => 3
9 changes: 9 additions & 0 deletions tests/pos/i19031.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//> using options -Werror

sealed trait A:
class B extends A

class Test:
def t1(a: A): Boolean =
a match
case b: A#B => true