Skip to content
Prev Previous commit
Next Next commit
support check java enum from classfile and source file
  • Loading branch information
liufengyun committed Jul 5, 2016
commit 9a0952cdd0c2bc7a5ed0fdb4bf1cff968d1ab874
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
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 elimAnonymousClass(implicit ctx: Context): Type = this match {
Copy link
Contributor

Choose a reason for hiding this comment

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

deAnonymize seems more in sync with deAlias.
Additionally elim at least in my head means eliminating deeply inside the type, ie if there's a reference to an anonymous class somewhere deep you'll also eliminate it.

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
9 changes: 5 additions & 4 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 @@ -56,9 +57,9 @@ class PatternMatcher extends MiniPhaseTransform with DenotTransformer {thisTrans

// 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.skipCheck(sel.tpe)) {
engine.exhaustivity(tree)
engine.redundancy(tree)
if (engine.checkable(sel.tpe.widen.elimAnonymousClass)) {
engine.checkExhaustivity(tree)
engine.checkRedundancy(tree)
}

translated.ensureConforms(tree.tpe)
Expand Down Expand Up @@ -1275,7 +1276,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.elimAnonymousClass/*withoutAnnotations*/

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

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package dotty.tools.dotc
package transform
package patmat

import core.Types._
import core.Contexts._
Expand All @@ -12,8 +13,26 @@ import core.Symbols._
import core.NameOps._
import core.Constants._

/** Space logic for checking exhaustivity and unreachability of
* pattern matching.
/** Space logic for checking exhaustivity and unreachability of pattern matching.
*
* The core idea of the algorithm is that patterns and types are value
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to give some intuition what a Space is here

* spaces, which is recursively defined as follows:
*
* 1. `Empty` is a space
* 2. For a type T, `Typ(T)` is a space
* 3. A union of spaces `S1 | S2 | ...` is a space
* 4. For a case class Kon(x1: T1, x2: T2, .., xn: Tn), if S1, S2, ..., Sn
* are spaces, then `Kon(S1, S2, ..., Sn)` is a space.
* 5. A constant `Const(value, T)` is a point in space
* 6. A stable identifier `Var(sym, T)` is a space
*
* For the problem of exhaustivity check, its formulation in terms of space is as follows:
*
* Is the space Typ(T) a subspace of the union of space covered by all the patterns?
*
* The problem of unreachable patterns can be formulated as follows:
*
* Is the space covered by a pattern a subspace of the space covered by previous patterns?
*/


Expand Down Expand Up @@ -41,7 +60,7 @@ trait SpaceLogic {
/** Is the type `tp` decomposable? i.e. all values of the type can be covered
* by its decomposed types.
*
* Abstract sealed class, OrType and Boolean can be decomposed.
* Abstract sealed class, OrType, Boolean and Java enums can be decomposed.
*/
def canDecompose(tp: Type): Boolean

Expand Down Expand Up @@ -197,7 +216,7 @@ trait SpaceLogic {
case (Const(_, tp1), Typ(tp2, _)) =>
if (isSubType(tp1, tp2)) Empty else a
case (Const(_, _), _) => a
case (Typ(tp1, _), Const(_, tp2)) => // Boolean
case (Typ(tp1, _), Const(_, tp2)) => // Boolean & Java enum
if (isSubType(tp2, tp1) && canDecompose(tp1))
minus(Or(partitions(tp1)), b)
else a
Expand All @@ -215,10 +234,6 @@ trait SpaceLogic {
class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
Copy link
Member

Choose a reason for hiding this comment

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

do import tpd._ here so that you can use Tree instead of tpd.Tree below, a lot of phases do this.

import tpd._

def debug(s: String): Unit = {
if (ctx.debug) println(s)
}

/** Return the space that represents the pattern `pat`
*
* If roundUp is true, approximate extractors to its type,
Expand All @@ -227,10 +242,13 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
def project(pat: Tree, roundUp: Boolean = true)(implicit ctx: Context): Space = pat match {
case Literal(c) => Const(c, c.tpe)
case _: BackquotedIdent => Var(pat.symbol, pat.tpe)
case Ident(_) => Typ(pat.tpe.stripAnnots, false)
case Select(_, _) =>
case Ident(_) =>
Typ(pat.tpe.stripAnnots, false)
case Select(_, _) =>
if (pat.symbol.is(Module))
Typ(pat.tpe.stripAnnots, false)
else if (pat.symbol.is(Enum))
Const(Constant(pat.symbol), pat.tpe)
else
Var(pat.symbol, pat.tpe)
case Alternative(trees) => Or(trees.map(project(_, roundUp)))
Expand All @@ -243,7 +261,6 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
case Typed(pat @ UnApply(_, _, _), _) => project(pat)
case Typed(expr, _) => Typ(expr.tpe.stripAnnots, true)
case _ =>
debug(s"========unkown tree: $pat========")
Empty
}

Expand All @@ -255,71 +272,61 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
case (tp1: RefinedType, tp2: RefinedType) => isSubType(tp1.parent, tp2.parent)
case (tp1: RefinedType, _) => isSubType(tp1.parent, tp2)
case (_, tp2: RefinedType) => isSubType(tp1, tp2.parent)
case (_, _) =>
val res = tp1 <:< tp2
debug(s"$tp1 <: $tp2 ? $res")
res
case (_, _) => tp1 <:< tp2
}

def isEqualType(tp1: Type, tp2: Type): Boolean = {
val res = tp1 =:= tp2
debug(s"$tp1 == $tp2 ? $res")
res
}
def isEqualType(tp1: Type, tp2: Type): Boolean = tp1 =:= tp2

def signature(tp: Type): List[Type] = {
val ktor = tp.classSymbol.primaryConstructor.info

debug(s"=======ktor: $ktor")

val meth =
if (ktor.isInstanceOf[MethodType]) ktor
else
tp match {
case AppliedType(_, params) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

ping @odersky, this kind-of reimplements asSeenFrom for constructor. I guess there should be a better way to do it but @liufengyun says it does not work.

debug(s"=======params: $params")
val refined = params.map {
// TypeBounds would generate an exception
case tp: TypeBounds => tp.underlying
case tp => tp
}
debug(s"=======refined params: $refined")
ktor.appliedTo(refined)
case _ =>
ktor
}

val sign = meth.firstParamTypes.map(_.stripTypeVar).map(paramTp => refine(tp, paramTp))

debug(s"====signature of $tp: $sign")
sign
meth.firstParamTypes.map(_.stripTypeVar).map(refine(tp, _))
}

def partitions(tp: Type): List[Space] = tp match {
case OrType(tp1, tp2) => List(Typ(tp1, true), Typ(tp2, true))
case _ if tp =:= ctx.definitions.BooleanType =>
List(
Const(Constant(true), ctx.definitions.BooleanType),
Const(Constant(false), ctx.definitions.BooleanType)
)
case _ =>
val children = tp.classSymbol.annotations.filter(_.symbol == ctx.definitions.ChildAnnot).map { annot =>
// refer to definition of Annotation.makeChild
val sym = annot.tree match {
case Apply(TypeApply(_, List(tpTree)), _) => tpTree.symbol.asClass
}

if (sym.is(ModuleClass))
sym.classInfo.selfType
else if (sym.info.typeParams.length > 0 || tp.isInstanceOf[TypeRef])
refine(tp, sym.classInfo.symbolicTypeRef)
else
sym.info
} filter(_ <:< tp) // child may not always be subtype of parent: SI-4020

debug(s"=========child of ${tp.show}: ${children.map(_.show).mkString(", ")}")
def partitions(tp: Type): List[Space] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

decompose

Copy link
Contributor

Choose a reason for hiding this comment

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

add a comment that says that there is an assumption that canDecompose(tp)

val children = tp.classSymbol.annotations.filter(_.symbol == ctx.definitions.ChildAnnot).map { annot =>
Copy link
Contributor

Choose a reason for hiding this comment

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

getAnnotation

// refer to definition of Annotation.makeChild
annot.tree match {
case Apply(TypeApply(_, List(tpTree)), _) => tpTree.symbol
}
}

children.map(tp => Typ(tp, true))
tp match {
case OrType(tp1, tp2) => List(Typ(tp1, true), Typ(tp2, true))
case _ if tp =:= ctx.definitions.BooleanType =>
List(
Const(Constant(true), ctx.definitions.BooleanType),
Const(Constant(false), ctx.definitions.BooleanType)
)
case _ if tp.classSymbol.is(Enum) =>
children.map(sym => Const(Constant(sym), tp))
case _ =>
val parts = children.map { sym =>
if (sym.is(ModuleClass))
sym.asClass.classInfo.selfType
else if (sym.info.typeParams.length > 0 || tp.isInstanceOf[TypeRef])
refine(tp, sym.asClass.classInfo.symbolicTypeRef)
else
sym.info
} filter(_ <:< tp) // child may not always be subtype of parent: SI-4020

parts.map(tp => Typ(tp, true))
}
}

/** Refine tp2 based on tp1
Expand All @@ -337,13 +344,13 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
case _ => tp2
}

/** Abstract sealed types, or-types and Boolean can be decomposed */
def canDecompose(tp: Type): Boolean = {
/** Abstract sealed types, or-types, Boolean and Java enums can be decomposed */
def canDecompose(tp: Type): Boolean =
tp.typeSymbol.is(allOf(Abstract, Sealed)) ||
tp.typeSymbol.is(allOf(Trait, Sealed)) ||
tp.isInstanceOf[OrType] ||
tp =:= ctx.definitions.BooleanType
}
tp =:= ctx.definitions.BooleanType ||
tp.typeSymbol.is(Enum)

def isCaseClass(tp: Type): Boolean = tp.classSymbol.isClass && tp.classSymbol.is(CaseClass)

Expand Down Expand Up @@ -428,27 +435,24 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
flatten(s).map(doShow(_, false)).distinct.mkString(", ")
}

/** Does the type t have @unchecked annotation? */
def skipCheck(tp: Type): Boolean = tp match {
case AnnotatedType(tp, annot) => ctx.definitions.UncheckedAnnot == annot.symbol
case _ => false
}

/** Widen a type and eliminate anonymous classes */
private def widen(tp: Type): Type = tp.widen match {
case tp:TypeRef if tp.symbol.isAnonymousClass =>
tp.symbol.asClass.typeRef.asSeenFrom(tp.prefix, tp.symbol.owner)
case tp => tp
def checkable(tp: Type): Boolean = tp match {
case AnnotatedType(tp, annot) =>
(ctx.definitions.UncheckedAnnot != annot.symbol) && checkable(tp)
case _ => true // actually everything is checkable unless @unchecked

Copy link
Contributor

Choose a reason for hiding this comment

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

scalac only checks sealed classes. We should consider if we indeed want to change this.
I'll create an issue about this.

// tp.classSymbol.is(Sealed) ||
// tp.isInstanceOf[OrType] ||
// tp.classSymbol.is(Enum) ||
// Boolean
// Int
// ...
}

def exhaustivity(_match: Match): Unit = {
def checkExhaustivity(_match: Match): Unit = {
val Match(sel, cases) = _match
val selTyp = widen(sel.tpe)
val selTyp = sel.tpe.widen.elimAnonymousClass

debug(s"====patterns:\n${cases.map(_.pat).mkString("\n")}")
val patternSpace = cases.map(x => project(x.pat)).reduce((a, b) => Or(List(a, b)))
debug(s"====selector:\n" + selTyp)
debug("====pattern space:\n" + show(patternSpace))
val uncovered = simplify(minus(Typ(selTyp, true), patternSpace))

if (uncovered != Empty) {
Expand All @@ -460,9 +464,9 @@ class SpaceEngine(implicit ctx: Context) extends SpaceLogic {
}
}

def redundancy(_match: Match): Unit = {
def checkRedundancy(_match: Match): Unit = {
val Match(sel, cases) = _match
val selTyp = widen(sel.tpe)
val selTyp = sel.tpe.widen.elimAnonymousClass

// starts from the second, the first can't be redundant
(1 until cases.length).foreach { i =>
Copy link
Member

Choose a reason for hiding this comment

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

I think you can just do cases.foreach { c => ... }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I need an i here to take the first i cases to check redundancy.

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I see, you could do cases.zipWithIndex.foreach { (c, i) => ... }. There might be a better way but I can't think of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

zipWithIndex actually is a little inconvenient here, as I want to start from the second element.

Expand Down
Loading