Skip to content

Commit 470e012

Browse files
authored
Fix problems in checking that a constructor is uninhabited for exhaustive match checking (#23403)
1. If the field with bottom type is `lazy`, then the constructor is inhabited. 2. Now it is taken into account that the type member can be `Nothing` and the field can be this type member. 3. The check has been moved to the `inhabited` function, allowing some changes to be reverted from #21750 I was also thinking about making the check recursive, i.e. checking that all fields are inhabited. This could cause problems with cycles and performance, so I'm not sure whether to do it.
2 parents 7f27eaf + 0696aec commit 470e012

File tree

4 files changed

+45
-28
lines changed

4 files changed

+45
-28
lines changed

compiler/src/dotty/tools/dotc/transform/patmat/Space.scala

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -661,49 +661,37 @@ object SpaceEngine {
661661
// we get
662662
// <== refineUsingParent(NatT, class Succ, []) = Succ[NatT]
663663
// <== isSub(Succ[NatT] <:< Succ[Succ[<?>]]) = false
664-
def getAppliedClass(tp: Type): (Type, List[Type]) = tp match
665-
case tp @ AppliedType(_: HKTypeLambda, _) => (tp, Nil)
666-
case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => (tp, tp.args)
664+
def getAppliedClass(tp: Type): Type = tp match
665+
case tp @ AppliedType(_: HKTypeLambda, _) => tp
666+
case tp @ AppliedType(tycon: TypeRef, _) if tycon.symbol.isClass => tp
667667
case tp @ AppliedType(tycon: TypeProxy, _) => getAppliedClass(tycon.superType.applyIfParameterized(tp.args))
668-
case tp => (tp, Nil)
669-
val (tp, typeArgs) = getAppliedClass(tpOriginal)
670-
// This function is needed to get the arguments of the types that will be applied to the class.
671-
// This is necessary because if the arguments of the types contain Nothing,
672-
// then this can affect whether the class will be taken into account during the exhaustiveness check
673-
def getTypeArgs(parent: Symbol, child: Symbol, typeArgs: List[Type]): List[Type] =
674-
val superType = child.typeRef.superType
675-
if typeArgs.exists(_.isBottomType) && superType.isInstanceOf[ClassInfo] then
676-
val parentClass = superType.asInstanceOf[ClassInfo].declaredParents.find(_.classSymbol == parent).get
677-
val paramTypeMap = Map.from(parentClass.argInfos.map(_.typeSymbol).zip(typeArgs))
678-
val substArgs = child.typeRef.typeParamSymbols.map(param => paramTypeMap.getOrElse(param, WildcardType))
679-
substArgs
680-
else Nil
681-
def getChildren(sym: Symbol, typeArgs: List[Type]): List[Symbol] =
668+
case tp => tp
669+
val tp = getAppliedClass(tpOriginal)
670+
def getChildren(sym: Symbol): List[Symbol] =
682671
sym.children.flatMap { child =>
683672
if child eq sym then List(sym) // i3145: sealed trait Baz, val x = new Baz {}, Baz.children returns Baz...
684673
else if tp.classSymbol == defn.TupleClass || tp.classSymbol == defn.NonEmptyTupleClass then
685674
List(child) // TupleN and TupleXXL classes are used for Tuple, but they aren't Tuple's children
686-
else if (child.is(Private) || child.is(Sealed)) && child.isOneOf(AbstractOrTrait) then
687-
getChildren(child, getTypeArgs(sym, child, typeArgs))
688-
else
689-
val childSubstTypes = child.typeRef.applyIfParameterized(getTypeArgs(sym, child, typeArgs))
690-
// if a class contains a field of type Nothing,
691-
// then it can be ignored in pattern matching, because it is impossible to obtain an instance of it
692-
val existFieldWithBottomType = childSubstTypes.fields.exists(_.info.isBottomType)
693-
if existFieldWithBottomType then Nil else List(child)
675+
else if (child.is(Private) || child.is(Sealed)) && child.isOneOf(AbstractOrTrait) then getChildren(child)
676+
else List(child)
694677
}
695-
val children = trace(i"getChildren($tp)")(getChildren(tp.classSymbol, typeArgs))
678+
val children = trace(i"getChildren($tp)")(getChildren(tp.classSymbol))
696679

697680
val parts = children.map { sym =>
698681
val sym1 = if (sym.is(ModuleClass)) sym.sourceModule else sym
699682
val refined = trace(i"refineUsingParent($tp, $sym1, $mixins)")(TypeOps.refineUsingParent(tp, sym1, mixins))
700683

684+
def containsUninhabitedField(tp: Type): Boolean =
685+
tp.fields.exists { field =>
686+
!field.symbol.flags.is(Lazy) && field.info.dealias.isBottomType
687+
}
688+
701689
def inhabited(tp: Type): Boolean = tp.dealias match
702690
case AndType(tp1, tp2) => !TypeComparer.provablyDisjoint(tp1, tp2)
703691
case OrType(tp1, tp2) => inhabited(tp1) || inhabited(tp2)
704692
case tp: RefinedType => inhabited(tp.parent)
705-
case tp: TypeRef => inhabited(tp.prefix)
706-
case _ => true
693+
case tp: TypeRef => !containsUninhabitedField(tp) && inhabited(tp.prefix)
694+
case _ => !containsUninhabitedField(tp)
707695

708696
if inhabited(refined) then refined
709697
else NoType
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
8: Pattern Match Exhaustivity: Bar()
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
sealed trait Adt
2+
case class Foo() extends Adt
3+
case class Bar() extends Adt {
4+
lazy val x: Nothing = throw new Exception()
5+
}
6+
7+
def shouldThrowAWarning(x: Adt) =
8+
x match { // warn
9+
case Foo() => "Foo"
10+
}
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
trait Phase {
2+
type FooTy
3+
type BarTy
4+
sealed trait Adt
5+
case class Foo(x: FooTy) extends Adt
6+
case class Bar(x: BarTy) extends Adt
7+
}
8+
9+
object Basic extends Phase {
10+
type FooTy = Unit
11+
type BarTy = Nothing
12+
}
13+
14+
15+
def test(a: Basic.Adt) = {
16+
a match
17+
case Basic.Foo(x) =>
18+
}

0 commit comments

Comments
 (0)