From 4f22672d12faf90e395769ee1028ac48b9c3d7e9 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 12 Jun 2024 16:06:10 +0100 Subject: [PATCH 01/11] Re-fix skipping match analysis & inlining --- .../tools/dotc/transform/PatternMatcher.scala | 14 ++------------ .../dotty/tools/dotc/transform/patmat/Space.scala | 5 +---- 2 files changed, 3 insertions(+), 16 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala index a5c85d4f9f3a..9750c41b7252 100644 --- a/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala +++ b/compiler/src/dotty/tools/dotc/transform/PatternMatcher.scala @@ -35,13 +35,6 @@ class PatternMatcher extends MiniPhase { override def runsAfter: Set[String] = Set(ElimRepeated.name) - private val InInlinedCode = new util.Property.Key[Boolean] - private def inInlinedCode(using Context) = ctx.property(InInlinedCode).getOrElse(false) - - override def prepareForInlined(tree: Inlined)(using Context): Context = - if inInlinedCode then ctx - else ctx.fresh.setProperty(InInlinedCode, true) - override def transformMatch(tree: Match)(using Context): Tree = if (tree.isInstanceOf[InlineMatch]) tree else { @@ -53,13 +46,10 @@ class PatternMatcher extends MiniPhase { case rt => tree.tpe val translated = new Translator(matchType, this).translateMatch(tree) - if !inInlinedCode then + // Skip analysis on inlined code (eg pos/i19157) + if !tpd.enclosingInlineds.nonEmpty then // check exhaustivity and unreachability SpaceEngine.checkMatch(tree) - else - // only check exhaustivity, as inlining may generate unreachable code - // like in i19157.scala - SpaceEngine.checkMatchExhaustivityOnly(tree) translated.ensureConforms(matchType) } diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index eb74058dfb10..774909ee271e 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -902,9 +902,6 @@ object SpaceEngine { } def checkMatch(m: Match)(using Context): Unit = - checkMatchExhaustivityOnly(m) - if reachabilityCheckable(m.selector) then checkReachability(m) - - def checkMatchExhaustivityOnly(m: Match)(using Context): Unit = if exhaustivityCheckable(m.selector) then checkExhaustivity(m) + if reachabilityCheckable(m.selector) then checkReachability(m) } From 3ba51a60fad87fc80c041df0ba5cb0c2c7306baf Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 17 Jun 2024 10:25:08 +0100 Subject: [PATCH 02/11] Fix PrefixSetting --- compiler/src/dotty/tools/dotc/config/Settings.scala | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/config/Settings.scala b/compiler/src/dotty/tools/dotc/config/Settings.scala index 9250303e8cc8..7454682fba56 100644 --- a/compiler/src/dotty/tools/dotc/config/Settings.scala +++ b/compiler/src/dotty/tools/dotc/config/Settings.scala @@ -411,9 +411,10 @@ object Settings: def PhasesSetting(category: SettingCategory, name: String, descr: String, default: String = "", aliases: List[String] = Nil, deprecation: Option[Deprecation] = None): Setting[List[String]] = publish(Setting(category, prependName(name), descr, if (default.isEmpty) Nil else List(default), aliases = aliases, deprecation = deprecation)) - def PrefixSetting(category: SettingCategory, name: String, descr: String, deprecation: Option[Deprecation] = None): Setting[List[String]] = + def PrefixSetting(category: SettingCategory, name0: String, descr: String, deprecation: Option[Deprecation] = None): Setting[List[String]] = + val name = prependName(name0) val prefix = name.takeWhile(_ != '<') - publish(Setting(category, "-" + name, descr, Nil, prefix = Some(prefix), deprecation = deprecation)) + publish(Setting(category, name, descr, Nil, prefix = Some(prefix), deprecation = deprecation)) def VersionSetting(category: SettingCategory, name: String, descr: String, default: ScalaVersion = NoScalaVersion, legacyArgs: Boolean = false, deprecation: Option[Deprecation] = None): Setting[ScalaVersion] = publish(Setting(category, prependName(name), descr, default, legacyArgs = legacyArgs, deprecation = deprecation)) From 5994ea79dce2ce30778592effff8b00df29a930f Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 1 Jul 2024 12:27:44 +0100 Subject: [PATCH 03/11] Childless --- .../src/dotty/tools/dotc/transform/patmat/Space.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 774909ee271e..7b33c0398f00 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -616,7 +616,7 @@ object SpaceEngine { case tp if tp.classSymbol.isAllOf(JavaEnum) => tp.classSymbol.children.map(_.termRef) // the class of a java enum value is the enum class, so this must follow SingletonType to not loop infinitely - case tp @ AppliedType(Parts(parts), targs) if tp.classSymbol.children.isEmpty => + case Childless(tp @ AppliedType(Parts(parts), targs)) => // It might not obvious that it's OK to apply the type arguments of a parent type to child types. // But this is guarded by `tp.classSymbol.children.isEmpty`, // meaning we'll decompose to the same class, just not the same type. @@ -676,6 +676,12 @@ object SpaceEngine { final class PartsExtractor(val get: List[Type]) extends AnyVal: def isEmpty: Boolean = get == ListOfNoType + object Childless: + def unapply(tp: Type)(using Context): Result = + Result(if tp.classSymbol.children.isEmpty then tp else NoType) + class Result(val get: Type) extends AnyVal: + def isEmpty: Boolean = !get.exists + /** Show friendly type name with current scope in mind * * E.g. C.this.B --> B if current owner is C From b56da91c7ad1f1c61c8cf0499db5f5399abc5802 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 20 Jun 2024 16:47:24 +0100 Subject: [PATCH 04/11] Fix a bundle of patmat issues Some of the issues are legitimate --- .../tools/dotc/transform/patmat/Space.scala | 31 ++++++++++++------- tests/warn/i20121.scala | 13 ++++++++ tests/warn/i20122.scala | 17 ++++++++++ tests/warn/i20123.scala | 16 ++++++++++ tests/warn/i20128.scala | 9 ++++++ tests/warn/i20129.scala | 14 +++++++++ tests/warn/i20130.scala | 11 +++++++ tests/warn/i20131.scala | 17 ++++++++++ tests/warn/i20132.alt.scala | 8 +++++ tests/warn/i20132.scala | 8 +++++ tests/warn/i20132.wo.scala | 8 +++++ tests/warn/i5422.scala | 9 ++++++ tests/warn/t11620.scala | 9 ++++++ 13 files changed, 158 insertions(+), 12 deletions(-) create mode 100644 tests/warn/i20121.scala create mode 100644 tests/warn/i20122.scala create mode 100644 tests/warn/i20123.scala create mode 100644 tests/warn/i20128.scala create mode 100644 tests/warn/i20129.scala create mode 100644 tests/warn/i20130.scala create mode 100644 tests/warn/i20131.scala create mode 100644 tests/warn/i20132.alt.scala create mode 100644 tests/warn/i20132.scala create mode 100644 tests/warn/i20132.wo.scala create mode 100644 tests/warn/i5422.scala create mode 100644 tests/warn/t11620.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 7b33c0398f00..a4bd62622d8a 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -528,8 +528,7 @@ object SpaceEngine { // force type inference to infer a narrower type: could be singleton // see tests/patmat/i4227.scala mt.paramInfos(0) <:< scrutineeTp - instantiateSelected(mt, tvars) - isFullyDefined(mt, ForceDegree.all) + maximizeType(mt.paramInfos(0), Spans.NoSpan) mt } @@ -543,7 +542,7 @@ object SpaceEngine { // Case unapplySeq: // 1. return the type `List[T]` where `T` is the element type of the unapplySeq return type `Seq[T]` - val resTp = ctx.typeAssigner.safeSubstMethodParams(mt, scrutineeTp :: Nil).finalResultType + val resTp = wildApprox(ctx.typeAssigner.safeSubstMethodParams(mt, scrutineeTp :: Nil).finalResultType) val sig = if (resTp.isRef(defn.BooleanClass)) @@ -564,20 +563,14 @@ object SpaceEngine { if (arity > 0) productSelectorTypes(resTp, unappSym.srcPos) else { - val getTp = resTp.select(nme.get).finalResultType match - case tp: TermRef if !tp.isOverloaded => - // Like widenTermRefExpr, except not recursively. - // For example, in i17184 widen Option[foo.type]#get - // to Option[foo.type] instead of Option[Int]. - tp.underlying.widenExpr - case tp => tp + val getTp = extractorMemberType(resTp, nme.get, unappSym.srcPos) if (argLen == 1) getTp :: Nil else productSelectorTypes(getTp, unappSym.srcPos) } } } - sig.map(_.annotatedToRepeated) + sig.map { case tp: WildcardType => tp.bounds.hi case tp => tp } } /** Whether the extractor covers the given type */ @@ -623,7 +616,21 @@ object SpaceEngine { // For instance, from i15029, `decompose((X | Y).Field[T]) = [X.Field[T], Y.Field[T]]`. parts.map(tp.derivedAppliedType(_, targs)) - case tp if tp.isDecomposableToChildren => + case tpOriginal if tpOriginal.isDecomposableToChildren => + // isDecomposableToChildren uses .classSymbol.is(Sealed) + // But that classSymbol could be from an AppliedType + // where the type constructor is a non-class type + // E.g. t11620 where `?1.AA[X]` returns as "sealed" + // but using that we're not going to infer A1[X] and A2[X] + // but end up with A1[] and A2[]. + // So we widen (like AppliedType superType does) away + // non-class type constructors. + def getAppliedClass(tp: Type): Type = tp match + case tp @ AppliedType(_: HKTypeLambda, _) => tp + case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => tp + case tp @ AppliedType(tycon: TypeProxy, _) => getAppliedClass(tycon.superType.applyIfParameterized(tp.args)) + case tp => tp + val tp = getAppliedClass(tpOriginal) def getChildren(sym: Symbol): List[Symbol] = sym.children.flatMap { child => if child eq sym then List(sym) // i3145: sealed trait Baz, val x = new Baz {}, Baz.children returns Baz... diff --git a/tests/warn/i20121.scala b/tests/warn/i20121.scala new file mode 100644 index 000000000000..ce8e3e4d74f6 --- /dev/null +++ b/tests/warn/i20121.scala @@ -0,0 +1,13 @@ +sealed trait T_A[A, B] +type X = T_A[Byte, Byte] + +case class CC_B[A](a: A) extends T_A[A, X] + +val v_a: T_A[X, X] = CC_B(null) +val v_b = v_a match + case CC_B(_) => 0 // warn: unreachable + case _ => 1 + // for CC_B[A] to match T_A[X, X] + // A := X + // so require X, aka T_A[Byte, Byte] + // which isn't instantiable, outside of null diff --git a/tests/warn/i20122.scala b/tests/warn/i20122.scala new file mode 100644 index 000000000000..50da42a5926c --- /dev/null +++ b/tests/warn/i20122.scala @@ -0,0 +1,17 @@ +sealed trait T_B[C, D] + +case class CC_A() +case class CC_B[A, C](a: A) extends T_B[C, CC_A] +case class CC_C[C, D](a: T_B[C, D]) extends T_B[Int, CC_A] +case class CC_E(a: CC_C[Char, Byte]) + +val v_a: T_B[Int, CC_A] = CC_B(CC_E(CC_C(null))) +val v_b = v_a match + case CC_B(CC_E(CC_C(_))) => 0 // warn: unreachable + case _ => 1 + // for CC_B[A, C] to match T_B[C, CC_A] + // C <: Int, ok + // A <: CC_E, ok + // but you need a CC_C[Char, Byte] + // which requires a T_B[Char, Byte] + // which isn't instantiable, outside of null diff --git a/tests/warn/i20123.scala b/tests/warn/i20123.scala new file mode 100644 index 000000000000..32de903210b2 --- /dev/null +++ b/tests/warn/i20123.scala @@ -0,0 +1,16 @@ +sealed trait T_A[A, B] +sealed trait T_B[C] + +case class CC_D[A, C]() extends T_A[A, C] +case class CC_E() extends T_B[Nothing] +case class CC_G[A, C](c: C) extends T_A[A, C] + +val v_a: T_A[Boolean, T_B[Boolean]] = CC_G(null) +val v_b = v_a match { + case CC_D() => 0 + case CC_G(_) => 1 // warn: unreachable + // for CC_G[A, C] to match T_A[Boolean, T_B[Boolean]] + // A := Boolean, which is ok + // C := T_B[Boolean], + // which isn't instantiable, outside of null +} diff --git a/tests/warn/i20128.scala b/tests/warn/i20128.scala new file mode 100644 index 000000000000..f09b323c6ca0 --- /dev/null +++ b/tests/warn/i20128.scala @@ -0,0 +1,9 @@ +sealed trait T_A[A] +case class CC_B[A](a: T_A[A]) extends T_A[Byte] +case class CC_E[A](b: T_A[A]) extends T_A[Byte] + +val v_a: T_A[Byte] = CC_E(CC_B(null)) +val v_b: Int = v_a match { // warn: not exhaustive + case CC_E(CC_E(_)) => 0 + case CC_B(_) => 1 +} diff --git a/tests/warn/i20129.scala b/tests/warn/i20129.scala new file mode 100644 index 000000000000..de0f9af76718 --- /dev/null +++ b/tests/warn/i20129.scala @@ -0,0 +1,14 @@ +sealed trait T_A[A] +case class CC_B[A](a: T_A[A], c: T_A[A]) extends T_A[Char] +case class CC_C[A]() extends T_A[A] +case class CC_G() extends T_A[Char] + +val v_a: T_A[Char] = CC_B(CC_G(), CC_C()) +val v_b: Int = v_a match { // warn: not exhaustive + case CC_C() => 0 + case CC_G() => 1 + case CC_B(CC_B(_, _), CC_C()) => 2 + case CC_B(CC_C(), CC_C()) => 3 + case CC_B(_, CC_G()) => 4 + case CC_B(_, CC_B(_, _)) => 5 +} diff --git a/tests/warn/i20130.scala b/tests/warn/i20130.scala new file mode 100644 index 000000000000..571959c2b388 --- /dev/null +++ b/tests/warn/i20130.scala @@ -0,0 +1,11 @@ +sealed trait T_A[B] +sealed trait T_B[C] +case class CC_B[C]() extends T_A[T_B[C]] +case class CC_C[B, C](c: T_A[B], d: T_B[C]) extends T_B[C] +case class CC_E[C]() extends T_B[C] + +val v_a: T_B[Int] = CC_C(null, CC_E()) +val v_b: Int = v_a match { // warn: not exhaustive + case CC_C(_, CC_C(_, _)) => 0 + case CC_E() => 5 +} diff --git a/tests/warn/i20131.scala b/tests/warn/i20131.scala new file mode 100644 index 000000000000..662c2896dc9a --- /dev/null +++ b/tests/warn/i20131.scala @@ -0,0 +1,17 @@ +sealed trait Foo +case class Foo1() extends Foo +case class Foo2[A, B]() extends Foo + +sealed trait Bar[A, B] +case class Bar1[A, C, D](a: Bar[C, D]) extends Bar[A, Bar[C, D]] +case class Bar2[ C, D](b: Bar[C, D], c: Foo) extends Bar[Bar1[Int, Byte, Int], Bar[C, D]] + +class Test: + def m1(bar: Bar[Bar1[Int, Byte, Int], Bar[Char, Char]]): Int = bar match + case Bar1(_) => 0 + case Bar2(_, Foo2()) => 1 + def t1 = m1(Bar2(null, Foo1())) + // for Bar2[C, D] to match the scrutinee + // C := Char and D := Char + // which requires a Bar[Char, Char] + // which isn't instantiable, outside of null diff --git a/tests/warn/i20132.alt.scala b/tests/warn/i20132.alt.scala new file mode 100644 index 000000000000..2d45367c61b8 --- /dev/null +++ b/tests/warn/i20132.alt.scala @@ -0,0 +1,8 @@ +sealed trait Foo[A] +case class Bar[C](x: Foo[C]) extends Foo[C] +case class End[B]() extends Foo[B] +class Test: + def m1[M](foo: Foo[M]): Int = foo match // warn: not exhaustive + case End() => 0 + case Bar(End()) => 1 + def t1 = m1[Int](Bar[Int](Bar[Int](End[Int]()))) diff --git a/tests/warn/i20132.scala b/tests/warn/i20132.scala new file mode 100644 index 000000000000..a5f40278234a --- /dev/null +++ b/tests/warn/i20132.scala @@ -0,0 +1,8 @@ +sealed trait Foo[A] +case class Bar[C](x: Foo[C]) extends Foo[Int] +case class End[B]() extends Foo[B] +class Test: + def m1[M](foo: Foo[M]): Int = foo match // warn: not exhaustive + case End() => 0 + case Bar(End()) => 1 + def t1 = m1[Int](Bar[Int](Bar[Int](End[Int]()))) diff --git a/tests/warn/i20132.wo.scala b/tests/warn/i20132.wo.scala new file mode 100644 index 000000000000..a6945758ae8d --- /dev/null +++ b/tests/warn/i20132.wo.scala @@ -0,0 +1,8 @@ +sealed trait Foo[A] +case class Bar[C](x: Foo[C]) extends Foo[Int] +case class End[B]() extends Foo[B] +class Test: + def m1[M](foo: Foo[M]): Int = foo match + case End() => 0 + case Bar(_) => 1 + def t1 = m1[Int](Bar[Int](Bar[Int](End[Int]()))) diff --git a/tests/warn/i5422.scala b/tests/warn/i5422.scala new file mode 100644 index 000000000000..bc124382d7d3 --- /dev/null +++ b/tests/warn/i5422.scala @@ -0,0 +1,9 @@ +sealed trait Foo[A[_]] + +case class Bar[C[_], X](x: C[X]) extends Foo[C] +case class End[B[_]]() extends Foo[B] + +class Test: + def foo[M[_]](foo: Foo[M]): Int = foo match + case End() => 0 + case Bar(_) => 1 diff --git a/tests/warn/t11620.scala b/tests/warn/t11620.scala new file mode 100644 index 000000000000..2d87d4c1a2c6 --- /dev/null +++ b/tests/warn/t11620.scala @@ -0,0 +1,9 @@ +sealed trait A[+T0] +case class A1[+T1](t1: T1) extends A[T1] +case class A2[+T2](t2: T2) extends A[T2] +sealed trait B[+T3] { type AA[+U] <: A[U] ; def a: AA[T3] } +object B { def unapply[T4](b: B[T4]): Some[b.AA[T4]] = Some(b.a) } +class Test: + def m1[X](b: B[X]): X = b match + case B(A1(v1)) => v1 + case B(A2(v2)) => v2 From 7183bb2714ac85125a38637950cd4266436113d6 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 4 Jul 2024 12:09:33 +0100 Subject: [PATCH 05/11] Detail why not baseType --- .../dotty/tools/dotc/transform/patmat/Space.scala | 8 ++++++++ tests/warn/i15893.min.scala | 13 +++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 tests/warn/i15893.min.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index a4bd62622d8a..d4aafa91676f 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -625,6 +625,14 @@ object SpaceEngine { // but end up with A1[] and A2[]. // So we widen (like AppliedType superType does) away // non-class type constructors. + // + // Can't use `tpOriginal.baseType(cls)` because it causes + // i15893 to return exhaustivity warnings, because instead of: + // <== refineUsingParent(N, class Succ, []) = Succ[] + // <== isSub(Succ[] <:< Succ[Succ[]]) = true + // we get + // <== refineUsingParent(NatT, class Succ, []) = Succ[NatT] + // <== isSub(Succ[NatT] <:< Succ[Succ[]]) = false def getAppliedClass(tp: Type): Type = tp match case tp @ AppliedType(_: HKTypeLambda, _) => tp case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => tp diff --git a/tests/warn/i15893.min.scala b/tests/warn/i15893.min.scala new file mode 100644 index 000000000000..755dda5cbcda --- /dev/null +++ b/tests/warn/i15893.min.scala @@ -0,0 +1,13 @@ +sealed trait NatT +case class Zero() extends NatT +case class Succ[+N <: NatT](n: N) extends NatT + +type Mod2[N <: NatT] <: NatT = N match + case Zero => Zero + case Succ[Zero] => Succ[Zero] + case Succ[Succ[predPredN]] => Mod2[predPredN] + +def dependentlyTypedMod2[N <: NatT](n: N): Mod2[N] = n match + case Zero(): Zero => Zero() // warn + case Succ(Zero()): Succ[Zero] => Succ(Zero()) // warn + case Succ(Succ(predPredN)): Succ[Succ[?]] => dependentlyTypedMod2(predPredN) // warn From 8df91fa50500b1f5b87e78499a3988591c104f12 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 4 Jul 2024 18:01:56 +0100 Subject: [PATCH 06/11] Strip null in exhaustivityCheckable --- .../tools/dotc/transform/patmat/Space.scala | 10 ++++++---- tests/warn/i20132.stream-Tuple2.scala | 18 ++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 tests/warn/i20132.stream-Tuple2.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index d4aafa91676f..39bf0d9bfc2a 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -3,7 +3,9 @@ package dotc package transform package patmat -import core.*, Constants.*, Contexts.*, Decorators.*, Flags.*, Names.*, NameOps.*, StdNames.*, Symbols.*, Types.* +import core.* +import Constants.*, Contexts.*, Decorators.*, Flags.*, NullOpsDecorator.*, Symbols.*, Types.* +import Names.*, NameOps.*, StdNames.* import ast.*, tpd.* import config.Printers.* import printing.{ Printer, * }, Texts.* @@ -793,12 +795,12 @@ object SpaceEngine { doShow(s) } - private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = { + private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = trace(i"exhaustivityCheckable($sel ${sel.className})") { val seen = collection.mutable.Set.empty[Symbol] // Possible to check everything, but be compatible with scalac by default - def isCheckable(tp: Type): Boolean = - val tpw = tp.widen.dealias + def isCheckable(tp: Type): Boolean = trace(i"isCheckable($tp ${tp.className})"): + val tpw = tp.widen.dealias.stripNull() val classSym = tpw.classSymbol classSym.is(Sealed) && !tpw.isLargeGenericTuple || // exclude large generic tuples from exhaustivity // requires an unknown number of changes to make work diff --git a/tests/warn/i20132.stream-Tuple2.scala b/tests/warn/i20132.stream-Tuple2.scala new file mode 100644 index 000000000000..b7cf58f8f930 --- /dev/null +++ b/tests/warn/i20132.stream-Tuple2.scala @@ -0,0 +1,18 @@ +//> using options -Yexplicit-nulls -Yno-flexible-types + +// Previously failed because the scrutinee under +// unsafeNulls/explicit-nulls/no-flexible-types +// is (String, String) | Null +// Need to strip the null before considering it exhaustivity checkable + +import scala.language.unsafeNulls + +import scala.jdk.CollectionConverters.* + +class Test2: + def t1: Unit = { + val xs = List.empty[(String, String)] + xs.asJava.forEach { case (a, b) => + () + } + } From e160de736a0e223d7b4acdc7b1d4710632c74deb Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 5 Jul 2024 12:24:01 +0100 Subject: [PATCH 07/11] Fix SeqFactoryClass#unapplySeq Previously `defn.ListType.appliedTo(elemTp) <:< pat.tpe` failed when pat.tpe is something like ParamClause, an alias. --- .../src/dotty/tools/dotc/transform/patmat/Space.scala | 2 +- tests/warn/i20132.list-Seq.scala | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 tests/warn/i20132.list-Seq.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 39bf0d9bfc2a..9d60336c02d7 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -352,7 +352,7 @@ object SpaceEngine { val funRef = fun1.tpe.asInstanceOf[TermRef] if (fun.symbol.name == nme.unapplySeq) val (arity, elemTp, resultTp) = unapplySeqInfo(fun.tpe.widen.finalResultType, fun.srcPos) - if (fun.symbol.owner == defn.SeqFactoryClass && defn.ListType.appliedTo(elemTp) <:< pat.tpe) + if fun.symbol.owner == defn.SeqFactoryClass && pat.tpe.hasClassSymbol(defn.ListClass) then // The exhaustivity and reachability logic already handles decomposing sum types (into its subclasses) // and product types (into its components). To get better counter-examples for patterns that are of type // List (or a super-type of list, like LinearSeq) we project them into spaces that use `::` and Nil. diff --git a/tests/warn/i20132.list-Seq.scala b/tests/warn/i20132.list-Seq.scala new file mode 100644 index 000000000000..95d6e962d547 --- /dev/null +++ b/tests/warn/i20132.list-Seq.scala @@ -0,0 +1,10 @@ +class D1 +class D2 + +class Test1: + type Ds = List[D1] | List[D2] + def m1(dss: List[Ds]) = + dss.flatMap: + case Seq(d) => Some(1) + case Seq(head, tail*) => Some(2) + case Seq() => None From 5fd810e6eda0d357b5e3e3117ad9260b237de6d5 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Fri, 5 Jul 2024 14:21:05 +0100 Subject: [PATCH 08/11] Strip null on the scrutinee Without that, we end up with either a flexible type or a `| Null`. --- .../dotty/tools/dotc/transform/patmat/Space.scala | 4 ++-- tests/warn/i20132.future-Left.scala | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 tests/warn/i20132.future-Left.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 9d60336c02d7..804beab83b13 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -836,7 +836,7 @@ object SpaceEngine { /** Return the underlying type of non-module, non-constant, non-enum case singleton types. * Also widen ExprType to its result type, and rewrap any annotation wrappers. * For example, with `val opt = None`, widen `opt.type` to `None.type`. */ - def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp)")(tp match { + def toUnderlying(tp: Type)(using Context): Type = trace(i"toUnderlying($tp ${tp.className})")(tp match { case _: ConstantType => tp case tp: TermRef if tp.symbol.is(Module) => tp case tp: TermRef if tp.symbol.isAllOf(EnumCase) => tp @@ -847,7 +847,7 @@ object SpaceEngine { }) def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") { - val selTyp = toUnderlying(m.selector.tpe).dealias + val selTyp = toUnderlying(m.selector.tpe.stripNull()).dealias val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp)) val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) => diff --git a/tests/warn/i20132.future-Left.scala b/tests/warn/i20132.future-Left.scala new file mode 100644 index 000000000000..a25718eadb6b --- /dev/null +++ b/tests/warn/i20132.future-Left.scala @@ -0,0 +1,13 @@ +//> using options -Yexplicit-nulls -Yno-flexible-types + +import scala.language.unsafeNulls + +import java.util.concurrent.CompletableFuture +import scala.jdk.CollectionConverters._ + +class Test1: + def m1 = + val fut: CompletableFuture[Either[String, List[String]]] = ??? + fut.thenApply: + case Right(edits: List[String]) => edits.asJava + case Left(error: String) => throw new Exception(error) From 4dd8e8ae3b6adae09a5c8a8bd80f9c022c4264e8 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 8 Jul 2024 11:40:29 +0100 Subject: [PATCH 09/11] Maximise once more --- .../src/dotty/tools/dotc/transform/patmat/Space.scala | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 804beab83b13..42983597e37a 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -524,6 +524,7 @@ object SpaceEngine { val mt: MethodType = unapp.widen match { case mt: MethodType => mt case pt: PolyType => + val locked = ctx.typerState.ownedVars val tvars = constrained(pt) val mt = pt.instantiate(tvars).asInstanceOf[MethodType] scrutineeTp <:< mt.paramInfos(0) @@ -531,6 +532,14 @@ object SpaceEngine { // see tests/patmat/i4227.scala mt.paramInfos(0) <:< scrutineeTp maximizeType(mt.paramInfos(0), Spans.NoSpan) + if !(ctx.typerState.ownedVars -- locked).isEmpty then + // constraining can create type vars out of wildcard types + // (in legalBound, by using a LevelAvoidMap) + // maximise will only do one pass at maximising the type vars in the target type + // which means we can maximise to types that include other type vars + // this fails TreeChecker's "non-empty constraint at end of $fusedPhase" check + // e.g. run-macros/string-context-implicits + maximizeType(mt.paramInfos(0), Spans.NoSpan) mt } From 5b425ee40d51e1c7e5c36745c83102f04f13af42 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 5 Aug 2024 15:42:27 +0100 Subject: [PATCH 10/11] Detail the second-pass maximizeType in Space.signature --- compiler/src/dotty/tools/dotc/transform/patmat/Space.scala | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 42983597e37a..f7ec95e21c90 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -539,6 +539,10 @@ object SpaceEngine { // which means we can maximise to types that include other type vars // this fails TreeChecker's "non-empty constraint at end of $fusedPhase" check // e.g. run-macros/string-context-implicits + // I can't prove that a second call won't also create type vars, + // but I'd rather have an unassigned new-new type var, than an infinite loop. + // After all, there's nothing strictly "wrong" with unassigned type vars, + // it just fails TreeChecker's linting. maximizeType(mt.paramInfos(0), Spans.NoSpan) mt } From ab240f1d5b12e242a79b8791e020a89b1a877c7f Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Mon, 5 Aug 2024 16:19:18 +0100 Subject: [PATCH 11/11] Only strip under unsafeNulls --- .../dotty/tools/dotc/transform/patmat/Space.scala | 7 +++++-- tests/warn/i20132.stream-Tuple2.safeNulls.check | 8 ++++++++ .../warn/i20132.stream-Tuple2.safeNulls.fixed.scala | 12 ++++++++++++ tests/warn/i20132.stream-Tuple2.safeNulls.scala | 11 +++++++++++ 4 files changed, 36 insertions(+), 2 deletions(-) create mode 100644 tests/warn/i20132.stream-Tuple2.safeNulls.check create mode 100644 tests/warn/i20132.stream-Tuple2.safeNulls.fixed.scala create mode 100644 tests/warn/i20132.stream-Tuple2.safeNulls.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index f7ec95e21c90..20b0099d82e2 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -808,12 +808,15 @@ object SpaceEngine { doShow(s) } + extension (self: Type) private def stripUnsafeNulls()(using Context): Type = + if Nullables.unsafeNullsEnabled then self.stripNull() else self + private def exhaustivityCheckable(sel: Tree)(using Context): Boolean = trace(i"exhaustivityCheckable($sel ${sel.className})") { val seen = collection.mutable.Set.empty[Symbol] // Possible to check everything, but be compatible with scalac by default def isCheckable(tp: Type): Boolean = trace(i"isCheckable($tp ${tp.className})"): - val tpw = tp.widen.dealias.stripNull() + val tpw = tp.widen.dealias.stripUnsafeNulls() val classSym = tpw.classSymbol classSym.is(Sealed) && !tpw.isLargeGenericTuple || // exclude large generic tuples from exhaustivity // requires an unknown number of changes to make work @@ -860,7 +863,7 @@ object SpaceEngine { }) def checkExhaustivity(m: Match)(using Context): Unit = trace(i"checkExhaustivity($m)") { - val selTyp = toUnderlying(m.selector.tpe.stripNull()).dealias + val selTyp = toUnderlying(m.selector.tpe.stripUnsafeNulls()).dealias val targetSpace = trace(i"targetSpace($selTyp)")(project(selTyp)) val patternSpace = Or(m.cases.foldLeft(List.empty[Space]) { (acc, x) => diff --git a/tests/warn/i20132.stream-Tuple2.safeNulls.check b/tests/warn/i20132.stream-Tuple2.safeNulls.check new file mode 100644 index 000000000000..e444ef8c3340 --- /dev/null +++ b/tests/warn/i20132.stream-Tuple2.safeNulls.check @@ -0,0 +1,8 @@ +-- [E029] Pattern Match Exhaustivity Warning: tests/warn/i20132.stream-Tuple2.safeNulls.scala:8:24 --------------------- +8 | xs.asJava.forEach { case (a, b) => // warn + | ^ + | match may not be exhaustive. + | + | It would fail on pattern case: _: Null + | + | longer explanation available when compiling with `-explain` diff --git a/tests/warn/i20132.stream-Tuple2.safeNulls.fixed.scala b/tests/warn/i20132.stream-Tuple2.safeNulls.fixed.scala new file mode 100644 index 000000000000..817d8ce06cee --- /dev/null +++ b/tests/warn/i20132.stream-Tuple2.safeNulls.fixed.scala @@ -0,0 +1,12 @@ +//> using options -Yexplicit-nulls -Yno-flexible-types + +import scala.jdk.CollectionConverters.* + +class Test2: + def t1: Unit = { + val xs = List.empty[(String, String)] + xs.asJava.forEach { + case (a, b) => () + case null => () + } + } diff --git a/tests/warn/i20132.stream-Tuple2.safeNulls.scala b/tests/warn/i20132.stream-Tuple2.safeNulls.scala new file mode 100644 index 000000000000..2d4a2318039e --- /dev/null +++ b/tests/warn/i20132.stream-Tuple2.safeNulls.scala @@ -0,0 +1,11 @@ +//> using options -Yexplicit-nulls -Yno-flexible-types + +import scala.jdk.CollectionConverters.* + +class Test2: + def t1: Unit = { + val xs = List.empty[(String, String)] + xs.asJava.forEach { case (a, b) => // warn + () + } + }