Skip to content

Commit ebf5ae1

Browse files
committed
Revert "Fix a bundle of patmat issues"
This reverts PR #21000 due to introduced performance regression
1 parent 49198f0 commit ebf5ae1

21 files changed

+36
-286
lines changed

Diff for: compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala

+12-2
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,13 @@ class PatternMatcher extends MiniPhase {
3535

3636
override def runsAfter: Set[String] = Set(ElimRepeated.name)
3737

38+
private val InInlinedCode = new util.Property.Key[Boolean]
39+
private def inInlinedCode(using Context) = ctx.property(InInlinedCode).getOrElse(false)
40+
41+
override def prepareForInlined(tree: Inlined)(using Context): Context =
42+
if inInlinedCode then ctx
43+
else ctx.fresh.setProperty(InInlinedCode, true)
44+
3845
override def transformMatch(tree: Match)(using Context): Tree =
3946
if (tree.isInstanceOf[InlineMatch]) tree
4047
else {
@@ -46,10 +53,13 @@ class PatternMatcher extends MiniPhase {
4653
case rt => tree.tpe
4754
val translated = new Translator(matchType, this).translateMatch(tree)
4855

49-
// Skip analysis on inlined code (eg pos/i19157)
50-
if !tpd.enclosingInlineds.nonEmpty then
56+
if !inInlinedCode then
5157
// check exhaustivity and unreachability
5258
SpaceEngine.checkMatch(tree)
59+
else
60+
// only check exhaustivity, as inlining may generate unreachable code
61+
// like in i19157.scala
62+
SpaceEngine.checkMatchExhaustivityOnly(tree)
5363

5464
translated.ensureConforms(matchType)
5565
}

Diff for: compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

+24-60
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@ package dotc
33
package transform
44
package patmat
55

6-
import core.*
7-
import Constants.*, Contexts.*, Decorators.*, Flags.*, NullOpsDecorator.*, Symbols.*, Types.*
8-
import Names.*, NameOps.*, StdNames.*
6+
import core.*, Constants.*, Contexts.*, Decorators.*, Flags.*, Names.*, NameOps.*, StdNames.*, Symbols.*, Types.*
97
import ast.*, tpd.*
108
import config.Printers.*
119
import printing.{ Printer, * }, Texts.*
@@ -352,7 +350,7 @@ object SpaceEngine {
352350
val funRef = fun1.tpe.asInstanceOf[TermRef]
353351
if (fun.symbol.name == nme.unapplySeq)
354352
val (arity, elemTp, resultTp) = unapplySeqInfo(fun.tpe.widen.finalResultType, fun.srcPos)
355-
if fun.symbol.owner == defn.SeqFactoryClass && pat.tpe.hasClassSymbol(defn.ListClass) then
353+
if (fun.symbol.owner == defn.SeqFactoryClass && defn.ListType.appliedTo(elemTp) <:< pat.tpe)
356354
// The exhaustivity and reachability logic already handles decomposing sum types (into its subclasses)
357355
// and product types (into its components). To get better counter-examples for patterns that are of type
358356
// List (or a super-type of list, like LinearSeq) we project them into spaces that use `::` and Nil.
@@ -524,26 +522,14 @@ object SpaceEngine {
524522
val mt: MethodType = unapp.widen match {
525523
case mt: MethodType => mt
526524
case pt: PolyType =>
527-
val locked = ctx.typerState.ownedVars
528525
val tvars = constrained(pt)
529526
val mt = pt.instantiate(tvars).asInstanceOf[MethodType]
530527
scrutineeTp <:< mt.paramInfos(0)
531528
// force type inference to infer a narrower type: could be singleton
532529
// see tests/patmat/i4227.scala
533530
mt.paramInfos(0) <:< scrutineeTp
534-
maximizeType(mt.paramInfos(0), Spans.NoSpan)
535-
if !(ctx.typerState.ownedVars -- locked).isEmpty then
536-
// constraining can create type vars out of wildcard types
537-
// (in legalBound, by using a LevelAvoidMap)
538-
// maximise will only do one pass at maximising the type vars in the target type
539-
// which means we can maximise to types that include other type vars
540-
// this fails TreeChecker's "non-empty constraint at end of $fusedPhase" check
541-
// e.g. run-macros/string-context-implicits
542-
// I can't prove that a second call won't also create type vars,
543-
// but I'd rather have an unassigned new-new type var, than an infinite loop.
544-
// After all, there's nothing strictly "wrong" with unassigned type vars,
545-
// it just fails TreeChecker's linting.
546-
maximizeType(mt.paramInfos(0), Spans.NoSpan)
531+
instantiateSelected(mt, tvars)
532+
isFullyDefined(mt, ForceDegree.all)
547533
mt
548534
}
549535

@@ -557,7 +543,7 @@ object SpaceEngine {
557543
// Case unapplySeq:
558544
// 1. return the type `List[T]` where `T` is the element type of the unapplySeq return type `Seq[T]`
559545

560-
val resTp = wildApprox(ctx.typeAssigner.safeSubstMethodParams(mt, scrutineeTp :: Nil).finalResultType)
546+
val resTp = ctx.typeAssigner.safeSubstMethodParams(mt, scrutineeTp :: Nil).finalResultType
561547

562548
val sig =
563549
if (resTp.isRef(defn.BooleanClass))
@@ -578,14 +564,20 @@ object SpaceEngine {
578564
if (arity > 0)
579565
productSelectorTypes(resTp, unappSym.srcPos)
580566
else {
581-
val getTp = extractorMemberType(resTp, nme.get, unappSym.srcPos)
567+
val getTp = resTp.select(nme.get).finalResultType match
568+
case tp: TermRef if !tp.isOverloaded =>
569+
// Like widenTermRefExpr, except not recursively.
570+
// For example, in i17184 widen Option[foo.type]#get
571+
// to Option[foo.type] instead of Option[Int].
572+
tp.underlying.widenExpr
573+
case tp => tp
582574
if (argLen == 1) getTp :: Nil
583575
else productSelectorTypes(getTp, unappSym.srcPos)
584576
}
585577
}
586578
}
587579

588-
sig.map { case tp: WildcardType => tp.bounds.hi case tp => tp }
580+
sig.map(_.annotatedToRepeated)
589581
}
590582

591583
/** Whether the extractor covers the given type */
@@ -624,36 +616,14 @@ object SpaceEngine {
624616
case tp if tp.classSymbol.isAllOf(JavaEnum) => tp.classSymbol.children.map(_.termRef)
625617
// the class of a java enum value is the enum class, so this must follow SingletonType to not loop infinitely
626618

627-
case Childless(tp @ AppliedType(Parts(parts), targs)) =>
619+
case tp @ AppliedType(Parts(parts), targs) if tp.classSymbol.children.isEmpty =>
628620
// It might not obvious that it's OK to apply the type arguments of a parent type to child types.
629621
// But this is guarded by `tp.classSymbol.children.isEmpty`,
630622
// meaning we'll decompose to the same class, just not the same type.
631623
// For instance, from i15029, `decompose((X | Y).Field[T]) = [X.Field[T], Y.Field[T]]`.
632624
parts.map(tp.derivedAppliedType(_, targs))
633625

634-
case tpOriginal if tpOriginal.isDecomposableToChildren =>
635-
// isDecomposableToChildren uses .classSymbol.is(Sealed)
636-
// But that classSymbol could be from an AppliedType
637-
// where the type constructor is a non-class type
638-
// E.g. t11620 where `?1.AA[X]` returns as "sealed"
639-
// but using that we're not going to infer A1[X] and A2[X]
640-
// but end up with A1[<?>] and A2[<?>].
641-
// So we widen (like AppliedType superType does) away
642-
// non-class type constructors.
643-
//
644-
// Can't use `tpOriginal.baseType(cls)` because it causes
645-
// i15893 to return exhaustivity warnings, because instead of:
646-
// <== refineUsingParent(N, class Succ, []) = Succ[<? <: NatT>]
647-
// <== isSub(Succ[<? <: NatT>] <:< Succ[Succ[<?>]]) = true
648-
// we get
649-
// <== refineUsingParent(NatT, class Succ, []) = Succ[NatT]
650-
// <== isSub(Succ[NatT] <:< Succ[Succ[<?>]]) = false
651-
def getAppliedClass(tp: Type): Type = tp match
652-
case tp @ AppliedType(_: HKTypeLambda, _) => tp
653-
case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => tp
654-
case tp @ AppliedType(tycon: TypeProxy, _) => getAppliedClass(tycon.superType.applyIfParameterized(tp.args))
655-
case tp => tp
656-
val tp = getAppliedClass(tpOriginal)
626+
case tp if tp.isDecomposableToChildren =>
657627
def getChildren(sym: Symbol): List[Symbol] =
658628
sym.children.flatMap { child =>
659629
if child eq sym then List(sym) // i3145: sealed trait Baz, val x = new Baz {}, Baz.children returns Baz...
@@ -706,12 +676,6 @@ object SpaceEngine {
706676
final class PartsExtractor(val get: List[Type]) extends AnyVal:
707677
def isEmpty: Boolean = get == ListOfNoType
708678

709-
object Childless:
710-
def unapply(tp: Type)(using Context): Result =
711-
Result(if tp.classSymbol.children.isEmpty then tp else NoType)
712-
class Result(val get: Type) extends AnyVal:
713-
def isEmpty: Boolean = !get.exists
714-
715679
/** Show friendly type name with current scope in mind
716680
*
717681
* E.g. C.this.B --> B if current owner is C
@@ -808,15 +772,12 @@ object SpaceEngine {
808772
doShow(s)
809773
}
810774

811-
extension (self: Type) private def stripUnsafeNulls()(using Context): Type =
812-
if Nullables.unsafeNullsEnabled then self.stripNull() else self
813-
814-
private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = trace(i"exhaustivityCheckable($sel ${sel.className})") {
775+
private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = {
815776
val seen = collection.mutable.Set.empty[Symbol]
816777

817778
// Possible to check everything, but be compatible with scalac by default
818-
def isCheckable(tp: Type): Boolean = trace(i"isCheckable($tp ${tp.className})"):
819-
val tpw = tp.widen.dealias.stripUnsafeNulls()
779+
def isCheckable(tp: Type): Boolean =
780+
val tpw = tp.widen.dealias
820781
val classSym = tpw.classSymbol
821782
classSym.is(Sealed) && !tpw.isLargeGenericTuple || // exclude large generic tuples from exhaustivity
822783
// requires an unknown number of changes to make work
@@ -851,7 +812,7 @@ object SpaceEngine {
851812
/** Return the underlying type of non-module, non-constant, non-enum case singleton types.
852813
* Also widen ExprType to its result type, and rewrap any annotation wrappers.
853814
* For example, with `val opt = None`, widen `opt.type` to `None.type`. */
854-
def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp ${tp.className})")(tp match {
815+
def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp)")(tp match {
855816
case _: ConstantType => tp
856817
case tp: TermRef if tp.symbol.is(Module) => tp
857818
case tp: TermRef if tp.symbol.isAllOf(EnumCase) => tp
@@ -862,7 +823,7 @@ object SpaceEngine {
862823
})
863824

864825
def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") {
865-
val selTyp = toUnderlying(m.selector.tpe.stripUnsafeNulls()).dealias
826+
val selTyp = toUnderlying(m.selector.tpe).dealias
866827
val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp))
867828

868829
val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) =>
@@ -940,6 +901,9 @@ object SpaceEngine {
940901
}
941902

942903
def checkMatch(m: Match)(using Context): Unit =
943-
if exhaustivityCheckable(m.selector) then checkExhaustivity(m)
904+
checkMatchExhaustivityOnly(m)
944905
if reachabilityCheckable(m.selector) then checkReachability(m)
906+
907+
def checkMatchExhaustivityOnly(m: Match)(using Context): Unit =
908+
if exhaustivityCheckable(m.selector) then checkExhaustivity(m)
945909
}

Diff for: tests/warn/i15893.min.scala

-13
This file was deleted.

Diff for: tests/warn/i20121.scala

-13
This file was deleted.

Diff for: tests/warn/i20122.scala

-17
This file was deleted.

Diff for: tests/warn/i20123.scala

-16
This file was deleted.

Diff for: tests/warn/i20128.scala

-9
This file was deleted.

Diff for: tests/warn/i20129.scala

-14
This file was deleted.

Diff for: tests/warn/i20130.scala

-11
This file was deleted.

Diff for: tests/warn/i20131.scala

-17
This file was deleted.

Diff for: tests/warn/i20132.alt.scala

-8
This file was deleted.

Diff for: tests/warn/i20132.future-Left.scala

-13
This file was deleted.

Diff for: tests/warn/i20132.list-Seq.scala

-10
This file was deleted.

Diff for: tests/warn/i20132.scala

-8
This file was deleted.

Diff for: tests/warn/i20132.stream-Tuple2.safeNulls.check

-8
This file was deleted.

Diff for: tests/warn/i20132.stream-Tuple2.safeNulls.fixed.scala

-12
This file was deleted.

0 commit comments

Comments
 (0)