From e259e329ebda31b76714d8fb69e8feb0151d9f07 Mon Sep 17 00:00:00 2001 From: Alexander Date: Thu, 10 Oct 2024 22:18:26 +0300 Subject: [PATCH 1/4] Do not consider uninhabited constructors when performing exhaustive match checking --- .../tools/dotc/transform/patmat/Space.scala | 49 +++++++++++++++++-- tests/init-global/pos/i18629.scala | 6 +++ tests/patmat/i13931.scala | 2 +- tests/warn/patmat-nothing-exhaustive.scala | 10 ++++ 4 files changed, 61 insertions(+), 6 deletions(-) create mode 100644 tests/init-global/pos/i18629.scala create mode 100644 tests/warn/patmat-nothing-exhaustive.scala diff --git a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala index 1b9ef9b9eaa7..f1b74d3fb67c 100644 --- a/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala +++ b/compiler/src/dotty/tools/dotc/transform/patmat/Space.scala @@ -625,16 +625,55 @@ 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 => - def getChildren(sym: Symbol): List[Symbol] = + 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. + // + // 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, List[Type]) = tp match + case tp @ AppliedType(_: HKTypeLambda, _) => (tp, Nil) + case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => (tp, tp.args) + case tp @ AppliedType(tycon: TypeProxy, _) => getAppliedClass(tycon.superType.applyIfParameterized(tp.args)) + case tp => (tp, Nil) + val (tp, typeArgs) = getAppliedClass(tpOriginal) + // This function is needed to get the arguments of the types that will be applied to the class. + // This is necessary because if the arguments of the types contain Nothing, + // then this can affect whether the class will be taken into account during the exhaustiveness check + def getTypeArgs(parent: Symbol, child: Symbol, typeArgs: List[Type]): List[Type] = + val superType = child.typeRef.superType + if typeArgs.exists(_.isBottomType) && superType.isInstanceOf[ClassInfo] then + val parentClass = superType.asInstanceOf[ClassInfo].declaredParents.find(_.classSymbol == parent).get + val paramTypeMap = Map.from(parentClass.argTypes.map(_.typeSymbol).zip(typeArgs)) + val substArgs = child.typeRef.typeParamSymbols.map(param => paramTypeMap.getOrElse(param, WildcardType)) + substArgs + else Nil + def getChildren(sym: Symbol, typeArgs: List[Type]): 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... else if tp.classSymbol == defn.TupleClass || tp.classSymbol == defn.NonEmptyTupleClass then List(child) // TupleN and TupleXXL classes are used for Tuple, but they aren't Tuple's children - else if (child.is(Private) || child.is(Sealed)) && child.isOneOf(AbstractOrTrait) then getChildren(child) - else List(child) + else if (child.is(Private) || child.is(Sealed)) && child.isOneOf(AbstractOrTrait) then + getChildren(child, getTypeArgs(sym, child, typeArgs)) + else + val childSubstTypes = child.typeRef.applyIfParameterized(getTypeArgs(sym, child, typeArgs)) + // if a class contains a field of type Nothing, + // then it can be ignored in pattern matching, because it is impossible to obtain an instance of it + val existFieldWithBottomType = childSubstTypes.fields.exists(_.info.isBottomType) + if existFieldWithBottomType then Nil else List(child) } - val children = trace(i"getChildren($tp)")(getChildren(tp.classSymbol)) + val children = trace(i"getChildren($tp)")(getChildren(tp.classSymbol, typeArgs)) val parts = children.map { sym => val sym1 = if (sym.is(ModuleClass)) sym.sourceModule else sym diff --git a/tests/init-global/pos/i18629.scala b/tests/init-global/pos/i18629.scala new file mode 100644 index 000000000000..03f1f5d5cda4 --- /dev/null +++ b/tests/init-global/pos/i18629.scala @@ -0,0 +1,6 @@ +object Foo { + val bar = List() match { + case List() => ??? + case null => ??? + } +} diff --git a/tests/patmat/i13931.scala b/tests/patmat/i13931.scala index 0d8d9eb9dcd3..562f059771c1 100644 --- a/tests/patmat/i13931.scala +++ b/tests/patmat/i13931.scala @@ -3,5 +3,5 @@ class Test: case Seq() => println("empty") case _ => println("non-empty") - def test2 = IndexedSeq() match { case IndexedSeq() => case _ => } + def test2 = IndexedSeq() match { case IndexedSeq() => case null => } def test3 = IndexedSeq() match { case IndexedSeq(1) => case _ => } diff --git a/tests/warn/patmat-nothing-exhaustive.scala b/tests/warn/patmat-nothing-exhaustive.scala new file mode 100644 index 000000000000..4e9181256fda --- /dev/null +++ b/tests/warn/patmat-nothing-exhaustive.scala @@ -0,0 +1,10 @@ +enum TestAdt: + case Inhabited + case Uninhabited(no: Nothing) + +def test1(t: TestAdt): Int = t match + case TestAdt.Inhabited => 1 + +def test2(o: Option[Option[Nothing]]): Int = o match + case Some(None) => 1 + case None => 2 From 382ba38c8295bcad1fd25cc2c20ad9eb43e5116b Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 6 Feb 2025 13:48:14 +0100 Subject: [PATCH 2/4] Do not consider uninhabited constructors when performing exhaustive match checking [Cherry-picked d459140ef14fe66644f90b9cc3b359e3cb3741ce][modified] From e3eacaeb630c969f766e4e9216417c9c59bdf960 Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 6 Feb 2025 13:51:49 +0100 Subject: [PATCH 3/4] bugfix: Fix issue with changed compiler plugin API --- tests/plugins/run/scriptWrapper/LineNumberPlugin_1.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/plugins/run/scriptWrapper/LineNumberPlugin_1.scala b/tests/plugins/run/scriptWrapper/LineNumberPlugin_1.scala index 888d5f95838d..c41d26379c9b 100644 --- a/tests/plugins/run/scriptWrapper/LineNumberPlugin_1.scala +++ b/tests/plugins/run/scriptWrapper/LineNumberPlugin_1.scala @@ -12,7 +12,7 @@ class LineNumberPlugin extends StandardPlugin { val name: String = "linenumbers" val description: String = "adjusts line numbers of script files" - override def initialize(options: List[String])(using Context): List[PluginPhase] = FixLineNumbers() :: Nil + override def init(options: List[String]): List[PluginPhase] = FixLineNumbers() :: Nil } // Loosely follows Mill linenumbers plugin (scan for marker with "original" source, adjust line numbers to match) From 6fc147ed2eabafcf97671406f4ebaf348132187d Mon Sep 17 00:00:00 2001 From: Tomasz Godzik Date: Thu, 6 Feb 2025 16:58:35 +0100 Subject: [PATCH 4/4] chore: Fix test to pass, since community build doesn't break more --- tests/patmat/i13931.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/patmat/i13931.scala b/tests/patmat/i13931.scala index 562f059771c1..ed801d0f63ad 100644 --- a/tests/patmat/i13931.scala +++ b/tests/patmat/i13931.scala @@ -1,7 +1,7 @@ class Test: def test = Vector() match case Seq() => println("empty") - case _ => println("non-empty") + case null => println("non-empty") def test2 = IndexedSeq() match { case IndexedSeq() => case null => } def test3 = IndexedSeq() match { case IndexedSeq(1) => case _ => }