Skip to content

[Deprecated] exhaustivity & redundancy check for pattern matching #1261

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

Closed
wants to merge 11 commits into from
4 changes: 0 additions & 4 deletions src/dotty/tools/dotc/Compiler.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,8 @@ import Symbols._
import Types._
import Scopes._
import typer.{FrontEnd, Typer, ImportInfo, RefChecks}
import reporting.{Reporter, ConsoleReporter}
import Phases.Phase
import transform._
import transform.TreeTransforms.{TreeTransform, TreeTransformer}
import core.DenotTransformers.DenotTransformer
import core.Denotations.SingleDenotation

import dotty.tools.backend.jvm.{LabelDefs, GenBCode}
import dotty.tools.backend.sjs.GenSJSIR
Expand Down
16 changes: 10 additions & 6 deletions src/dotty/tools/dotc/ast/Desugar.scala
Original file line number Diff line number Diff line change
Expand Up @@ -616,16 +616,20 @@ object desugar {
*
* { cases }
* ==>
* x$1 => x$1 match { cases }
* x$1 => (x$1 @unchecked) match { cases }
*
* If `nparams` != 1, expand instead to
*
* (x$1, ..., x$n) => (x$0, ..., x${n-1}) match { cases }
* (x$1, ..., x$n) => (x$0, ..., x${n-1} @unchecked) match { cases }
*/
def makeCaseLambda(cases: List[CaseDef], nparams: Int = 1)(implicit ctx: Context) = {
def makeCaseLambda(cases: List[CaseDef], nparams: Int = 1, unchecked: Boolean = true)(implicit ctx: Context) = {
val params = (1 to nparams).toList.map(makeSyntheticParameter(_))
val selector = makeTuple(params.map(p => Ident(p.name)))
Function(params, Match(selector, cases))

if (unchecked)
Function(params, Match(Annotated(New(ref(defn.UncheckedAnnotType)), selector), cases))
else
Function(params, Match(selector, cases))
}

/** Map n-ary function `(p1, ..., pn) => body` where n != 1 to unary function as follows:
Expand Down Expand Up @@ -753,7 +757,7 @@ object desugar {
case VarPattern(named, tpt) =>
Function(derivedValDef(named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body)
case _ =>
makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil)
makeCaseLambda(CaseDef(pat, EmptyTree, body) :: Nil, unchecked = false)
}

/** If `pat` is not an Identifier, a Typed(Ident, _), or a Bind, wrap
Expand Down Expand Up @@ -799,7 +803,7 @@ object desugar {
val cases = List(
CaseDef(pat, EmptyTree, Literal(Constant(true))),
CaseDef(Ident(nme.WILDCARD), EmptyTree, Literal(Constant(false))))
Apply(Select(rhs, nme.withFilter), Match(EmptyTree, cases))
Apply(Select(rhs, nme.withFilter), makeCaseLambda(cases))
}

/** Is pattern `pat` irrefutable when matched against `rhs`?
Expand Down
8 changes: 4 additions & 4 deletions src/dotty/tools/dotc/core/Phases.scala
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
package dotty.tools.dotc
package core

import Periods._
import Contexts._
import dotty.tools.backend.jvm.{LabelDefs, GenBCode}
import dotty.tools.backend.jvm.{GenBCode, LabelDefs}
import dotty.tools.dotc.core.Symbols.ClassSymbol
import util.DotClass
import DenotTransformers._
import Denotations._
import Decorators._
import config.Printers._
import scala.collection.mutable.{ListBuffer, ArrayBuffer}
import dotty.tools.dotc.transform.TreeTransforms.{TreeTransformer, MiniPhase, TreeTransform}

import scala.collection.mutable.ListBuffer
import dotty.tools.dotc.transform.TreeTransforms.{MiniPhase, TreeTransformer}
import dotty.tools.dotc.transform._
import Periods._
import typer.{FrontEnd, RefChecks}
Expand Down
7 changes: 7 additions & 0 deletions src/dotty/tools/dotc/core/Types.scala
Original file line number Diff line number Diff line change
Expand Up @@ -810,6 +810,13 @@ object Types {
case _ => this
}

/** Eliminate anonymous classes */
final def deAnonymize(implicit ctx: Context): Type = this match {
case tp:TypeRef if tp.symbol.isAnonymousClass =>
tp.symbol.asClass.typeRef.asSeenFrom(tp.prefix, tp.symbol.owner)
case tp => tp
}

/** Follow aliases and dereferences LazyRefs and instantiated TypeVars until type
* is no longer alias type, LazyRef, or instantiated type variable.
*/
Expand Down
10 changes: 10 additions & 0 deletions src/dotty/tools/dotc/core/classfile/ClassfileParser.scala
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ class ClassfileParser(
val jflags = in.nextChar
val isAnnotation = hasAnnotation(jflags)
val sflags = classTranslation.flags(jflags)
val isEnum = (jflags & JAVA_ACC_ENUM) != 0
val nameIdx = in.nextChar
currentClassName = pool.getClassName(nameIdx)

Expand Down Expand Up @@ -145,6 +146,15 @@ class ClassfileParser(
setClassInfo(classRoot, classInfo)
setClassInfo(moduleRoot, staticInfo)
}

// eager load java enum definitions for exhaustivity check of pattern match
if (isEnum) {
instanceScope.toList.map(_.ensureCompleted())
staticScope.toList.map(_.ensureCompleted())
classRoot.setFlag(Flags.Enum)
moduleRoot.setFlag(Flags.Enum)
}

result
}

Expand Down
3 changes: 2 additions & 1 deletion src/dotty/tools/dotc/transform/ExpandSAMs.scala
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,8 @@ class ExpandSAMs extends MiniPhaseTransform { thisTransformer =>
Bind(defaultSym, Underscore(selector.tpe.widen)),
EmptyTree,
Literal(Constant(false)))
cpy.Match(applyRhs)(paramRef, cases.map(translateCase) :+ defaultCase)
val annotated = Annotated(New(ref(defn.UncheckedAnnotType)), paramRef)
cpy.Match(applyRhs)(annotated, cases.map(translateCase) :+ defaultCase)
case _ =>
tru
}
Expand Down
18 changes: 10 additions & 8 deletions src/dotty/tools/dotc/transform/PatternMatcher.scala
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import Applications._
import TypeApplications._
import SymUtils._, core.NameOps._
import core.Mode
import patmat._

import dotty.tools.dotc.util.Positions.Position
import dotty.tools.dotc.core.Decorators._
Expand Down Expand Up @@ -52,6 +53,13 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
override def transformMatch(tree: Match)(implicit ctx: Context, info: TransformerInfo): Tree = {
val translated = new Translator()(ctx).translator.translateMatch(tree)

// check exhaustivity and unreachability
val engine = new SpaceEngine
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PatternMatcher now has a 🌠SpaceEngine🌠 and can do interstellar travels. Looking forward to battle space invaders.
👾👾👾👾

if (engine.checkable(tree)) {
engine.checkExhaustivity(tree)
engine.checkRedundancy(tree)
}

translated.ensureConforms(tree.tpe)
}

Expand Down Expand Up @@ -1246,13 +1254,6 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
case _ => false
}

def elimAnonymousClass(t: Type) = t match {
case t:TypeRef if t.symbol.isAnonymousClass =>
t.symbol.asClass.typeRef.asSeenFrom(t.prefix, t.symbol.owner)
case _ =>
t
}

/** Implement a pattern match by turning its cases (including the implicit failure case)
* into the corresponding (monadic) extractors, and combining them with the `orElse` combinator.
*
Expand All @@ -1266,7 +1267,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
def translateMatch(match_ : Match): Tree = {
val Match(sel, cases) = match_

val selectorTp = elimAnonymousClass(sel.tpe.widen/*withoutAnnotations*/)
val selectorTp = sel.tpe.widen.deAnonymize/*withoutAnnotations*/

val selectorSym = freshSym(sel.pos, selectorTp, "selector")

Expand All @@ -1275,6 +1276,7 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans
case _ => (cases, None)
}


// checkMatchVariablePatterns(nonSyntheticCases) // only used for warnings

// we don't transform after uncurry
Expand Down
9 changes: 9 additions & 0 deletions src/dotty/tools/dotc/transform/PostTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ import Symbols._, TypeUtils._
*
* (9) Adds SourceFile annotations to all top-level classes and objects
*
* (10) Adds Child annotations to all sealed classes
*
* The reason for making this a macro transform is that some functions (in particular
* super and protected accessors and instantiation checks) are naturally top-down and
* don't lend themselves to the bottom-up approach of a mini phase. The other two functions
Expand Down Expand Up @@ -231,6 +233,13 @@ class PostTyper extends MacroTransform with IdentityDenotTransformer { thisTran
ctx.compilationUnit.source.exists &&
sym != defn.SourceFileAnnot)
sym.addAnnotation(Annotation.makeSourceFile(ctx.compilationUnit.source.file.path))

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're unlucky, this will conflict with #1343.

if (!sym.isAnonymousClass) // ignore anonymous class
for (parent <- sym.asClass.classInfo.classParents) {
val pclazz = parent.classSymbol
if (pclazz.is(Sealed)) pclazz.addAnnotation(Annotation.makeChild(sym))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anonymous classes caused pickling-related error for tests/run/t8611b.scala:

https://gist.github.com/liufengyun/f4aca0853e06f96c9130a0a05bfa0d4f

If I ignore anonymous classes, then it works:

              if (!sym.isAnonymousClass) // ignore anonymous class
                for (parent <- sym.asClass.classInfo.classParents) {
                  val pclazz = parent.classSymbol
                  if (pclazz.is(Sealed)) pclazz.addAnnotation(Annotation.makeChild(sym))
                }

The problem is that ignore anonymous classes would make the algorithm silent in case it should report a warning:

sealed trait A
case class B() extends A
case class C() extends A

object M {
  val x = new A {}

  x match {
     case _: B => true
     case _: C => true
  }
}

Any ideas, @odersky @smarter ?

Copy link
Member

@smarter smarter May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a sealed trait has direct anonymous subclasses then it's impossible to pattern match on all subclasses. I think the simplest solution is to disallow direct anonymous subclasses of sealed traits.

tree
}
else {
Expand Down
Loading