Skip to content

Commit

Permalink
irrefutable name-based extractors
Browse files Browse the repository at this point in the history
  • Loading branch information
martijnhoekstra committed Nov 28, 2020
1 parent 867f63a commit add8a01
Show file tree
Hide file tree
Showing 11 changed files with 164 additions and 41 deletions.
40 changes: 37 additions & 3 deletions src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
Expand Up @@ -374,10 +374,42 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
def tru = True
}


private def isIrrefutabilityProof(sym: Symbol): Boolean = {
sym.isMethod && sym.name == TermName("isEmpty") && {
//In some cases, this return true while checking for `ConstantFalse`
//returns false, for example in test/files/neg/t12240.scala
sym.asMethod.returnType match {
case c: ConstantType => c.value == Constant(false)
case _ => false
}
}
}
// will an extractor with unapply method of methodtype `tp` always succeed?
// note: this assumes the other side-conditions implied by the extractor are met
// (argument of the right type, length check succeeds for unapplySeq,...)
private def irrefutableExtractorType(tp: Type): Boolean = tp.resultType.dealias match {
//Some(x) is always irrefutable
case TypeRef(_, SomeClass, _) => true
//name based pattern matching checks for constant false `isEmpty`.
case TypeRef(_, res, _) => res.tpe.members.exists(isIrrefutabilityProof)
// probably not useful since this type won't be inferred nor can it be written down (yet)
case ConstantTrue => true
case _ => false
}

private val irrefutableExtractor: PartialFunction[TreeMaker, Prop] = {
// the extra condition is None, the extractor's result indicates it always succeeds,
// if the extra condition is None, the extractor's result indicates it always succeeds,
// (the potential type-test for the argument is represented by a separate TypeTestTreeMaker)
case IrrefutableExtractorTreeMaker(_, _) => True
case ExtractorTreeMaker(extractor, None, _) if irrefutableExtractorType(extractor.tpe) => True
// Otherwise, if we call the pattern irrefutable here, these conditions
// are no longer checked and considered true in exhaustiveness and
// reachability checking.
// Therefore, the below case alone would treat too much "irrefutable"
// that really isn't. Something similar is needed (perhaps elsewhere)
// to check whether a set of unapplySeq's with all arities is toegether
// exhaustive
//case p @ ExtractorTreeMaker(extractor, Some(conditions), _) if irrefutableExtractorType(extractor.tpe) => True
}

// special-case: interpret pattern `List()` as `Nil`
Expand Down Expand Up @@ -446,11 +478,12 @@ trait MatchAnalysis extends MatchApproximation {
// thus, the case is unreachable if there is no model for -(-P /\ C),
// or, equivalently, P \/ -C, or C => P
def unreachableCase(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type): Option[Int] = {
debug.patmat("reachability analysis")
val start = if (StatisticsStatics.areSomeColdStatsEnabled) statistics.startTimer(statistics.patmatAnaReach) else null

// use the same approximator so we share variables,
// but need different conditions depending on whether we're conservatively looking for failure or success
// don't rewrite List-like patterns, as List() and Nil need to distinguished for unreachability
// don't rewrite List-like patterns, as List() and Nil allegedly need to be distinguished for unreachability
val approx = new TreeMakersToProps(prevBinder)
def approximate(default: Prop) = approx.approximateMatch(cases, approx.onUnknown { tm =>
approx.refutableRewrite.applyOrElse(tm, (_: TreeMaker) => default )
Expand Down Expand Up @@ -508,6 +541,7 @@ trait MatchAnalysis extends MatchApproximation {
// exhaustivity

def exhaustive(prevBinder: Symbol, cases: List[List[TreeMaker]], pt: Type): List[String] = if (!settings.warnStrictUnsealedPatMat && uncheckableType(prevBinder.info)) Nil else {
debug.patmat("exhaustiveness analysis")
// customize TreeMakersToProps (which turns a tree of tree makers into a more abstract DAG of tests)
// - approximate the pattern `List()` (unapplySeq on List with empty length) as `Nil`,
// otherwise the common (xs: List[Any]) match { case List() => case x :: xs => } is deemed unexhaustive
Expand Down
Expand Up @@ -314,7 +314,7 @@ trait MatchTranslation {
* a function that will take care of binding and substitution of the next ast (to the right).
*
*/
def translateCase(scrutSym: Symbol, pt: Type)(caseDef: CaseDef) = {
def translateCase(scrutSym: Symbol, pt: Type)(caseDef: CaseDef): List[TreeMaker] = {
val CaseDef(pattern, guard, body) = caseDef
translatePattern(BoundTree(scrutSym, pattern)) ++ translateGuard(guard) :+ translateBody(body, pt)
}
Expand Down
Expand Up @@ -323,25 +323,6 @@ trait MatchTreeMaking extends MatchCodeGen with Debugging {
override def toString = "P"+((prevBinder.name, extraCond getOrElse "", localSubstitution))
}

object IrrefutableExtractorTreeMaker {
// will an extractor with unapply method of methodtype `tp` always succeed?
// note: this assumes the other side-conditions implied by the extractor are met
// (argument of the right type, length check succeeds for unapplySeq,...)
def irrefutableExtractorType(tp: Type): Boolean = tp.resultType.dealias match {
case TypeRef(_, SomeClass, _) => true
// probably not useful since this type won't be inferred nor can it be written down (yet)
case ConstantTrue => true
case _ => false
}

def unapply(xtm: ExtractorTreeMaker): Option[(Tree, Symbol)] = xtm match {
case ExtractorTreeMaker(extractor, None, nextBinder) if irrefutableExtractorType(extractor.tpe) =>
Some((extractor, nextBinder))
case _ =>
None
}
}

object TypeTestTreeMaker {
// factored out so that we can consistently generate other representations of the tree that implements the test
// (e.g. propositions for exhaustivity and friends, boolean for isPureTypeTest)
Expand Down
2 changes: 1 addition & 1 deletion src/library/scala/Array.scala
Expand Up @@ -559,7 +559,7 @@ object Array {
def unapplySeq[T](x: Array[T]): UnapplySeqWrapper[T] = new UnapplySeqWrapper(x)

final class UnapplySeqWrapper[T](private val a: Array[T]) extends AnyVal {
def isEmpty: Boolean = false
def isEmpty: false = false
def get: UnapplySeqWrapper[T] = this
def lengthCompare(len: Int): Int = a.lengthCompare(len)
def apply(i: Int): T = a(i)
Expand Down
2 changes: 1 addition & 1 deletion src/library/scala/collection/Factory.scala
Expand Up @@ -308,7 +308,7 @@ object SeqFactory {
}

final class UnapplySeqWrapper[A](private val c: SeqOps[A, Seq, Seq[A]]) extends AnyVal {
def isEmpty: Boolean = false
def isEmpty: false = false
def get: UnapplySeqWrapper[A] = this
def lengthCompare(len: Int): Int = c.lengthCompare(len)
def apply(i: Int): A = c(i)
Expand Down
13 changes: 13 additions & 0 deletions test/files/neg/t12240.check
@@ -0,0 +1,13 @@
t12240.scala:21: warning: match may not be exhaustive.
def guardedNonExhaustive(x: Int) = x match {
^
t12240.scala:25: warning: match may not be exhaustive.
def guardedSeqNonExhaustive(x: Int) = x match {
^
t12240.scala:32: warning: match may not be exhaustive.
It would fail on the following input: Vector1()
def reported(v: Vector[String]) = v match {
^
error: No warnings can be incurred under -Werror.
3 warnings
1 error
38 changes: 38 additions & 0 deletions test/files/neg/t12240.scala
@@ -0,0 +1,38 @@
// scalac: -Xfatal-warnings -Xlint:strict-unsealed-patmat
//

object Test {

//see also pos/t12240.scala

class IrrefutableNameBasedResult[Result](r: Result) {
def isEmpty: false = false
def get: Result = r
}

object IrrefutableIdentityExtractor {
def unapply[A](a: A): IrrefutableNameBasedResult[A] = new IrrefutableNameBasedResult(a)
}

object IrrefutableSeqExtractor {
def unapplySeq[A](a: A) = new IrrefutableNameBasedResult(List(a))
}

def guardedNonExhaustive(x: Int) = x match {
case IrrefutableIdentityExtractor(_) if false => "non-exhautive"
}

def guardedSeqNonExhaustive(x: Int) = x match {
case IrrefutableSeqExtractor(_*) if false => "non-exhaustive"
}

//status quo:
//should be in pos/t12240.scala but isn't exhaustive per
//per https://github.com/scala/bug/issues/12252
def reported(v: Vector[String]) = v match {
case Vector() => "empty"
case Vector(_) => "one"
case Vector(_, _, _*) => "scalac doesn't now this is exhaustive"
}

}
65 changes: 65 additions & 0 deletions test/files/pos/t12240.scala
@@ -0,0 +1,65 @@
// scalac: -Xfatal-warnings -Xlint:strict-unsealed-patmat
//

object Test {

//original reports

def originalReported(v: Vector[String]) = v match {
case Vector() => "empty"
case Vector(_) => "one"
case Vector(_*) => "this pattern is irrefutable"
}

def originalMinimized(v: Vector[String]) = v match {
case Vector(_*) => "this pattern is irrefutable"
}

//broader applicability

class IrrefutableNameBasedResult[Result](r: Result) {
def isEmpty: false = false
def get: Result = r
}

object IrrefutableIdentityExtractor {
def unapply[A](a: A) = new IrrefutableNameBasedResult(a)
}

object IrrefutableSeqExtractor {
def unapplySeq[A](a: A) = new IrrefutableNameBasedResult(List(a))
}

def nameBasedPatternIsExhaustive(x: Int) = x match {
case IrrefutableIdentityExtractor(_) => "exhaustive"
}

def nameBasedSeqIsExhaustive(x: Int) = x match {
case IrrefutableSeqExtractor(_*) => "exhaustive"
}

//status quo:
//should be in neg/t12240.scala but isn't exhaustive per
//per https://github.com/scala/bug/issues/12252

def reported(v: Vector[String]) = v match {
case Vector() => "empty"
case Vector(_) => "one"
case Vector(_, _, _*) => "this ought to be exhaustive"
case Vector(_*) => "scalac doesn't know it's not exhaustive"
}

//status quo:
//should be in neg/t12240.scala, but the unreachable code isn't reported
//per https://github.com/scala/bug/issues/12251
def nameBasedPatternUnreachable(x: Int) = x match {
case IrrefutableIdentityExtractor(_) => "exhaustive"
case _ => "unreachable"
}

def nameBasedSeqUnreachable(x: Int) = x match {
case IrrefutableSeqExtractor(_*) => "exhaustive"
case _ => "unreachable"
}

}
16 changes: 8 additions & 8 deletions test/files/run/patmat-seq.check
@@ -1,5 +1,5 @@
newSource1.scala:35: warning: unreachable code
case B(x, y) => (x, y)
newSource1.scala:32: warning: unreachable code
case A(x, y) => (x, y) // wrapper.get.apply(0/1)
^
[[syntax trees at end of patmat]] // newSource1.scala
package <empty> {
Expand Down Expand Up @@ -66,7 +66,7 @@ package <empty> {
case <synthetic> val x1: Int(2) = 2;
case16(){
<synthetic> val o18: collection.SeqFactory.UnapplySeqWrapper[Int] = A.unapplySeq(x1);
if (o18.isEmpty.unary_!)
if (false.unary_!)
{
val xs: Seq[Int] = o18.get.toSeq;
matchEnd15(xs)
Expand All @@ -76,7 +76,7 @@ package <empty> {
};
case17(){
<synthetic> val o20: collection.SeqFactory.UnapplySeqWrapper[Int] = A.unapplySeq(x1);
if (o20.isEmpty.unary_!.&&(o20.get.!=(null).&&(o20.get.lengthCompare(2).==(0))))
if (false.unary_!.&&(o20.get.!=(null).&&(o20.get.lengthCompare(2).==(0))))
{
val x: Int = o20.get.apply(0);
val y: Int = o20.get.apply(1);
Expand All @@ -87,7 +87,7 @@ package <empty> {
};
case19(){
<synthetic> val o22: collection.SeqFactory.UnapplySeqWrapper[Int] = A.unapplySeq(x1);
if (o22.isEmpty.unary_!.&&(o22.get.!=(null).&&(o22.get.lengthCompare(1).>=(0))))
if (false.unary_!.&&(o22.get.!=(null).&&(o22.get.lengthCompare(1).>=(0))))
{
val x: Int = o22.get.apply(0);
val xs: Seq[Int] = o22.get.drop(1);
Expand Down Expand Up @@ -267,7 +267,7 @@ package <empty> {
case <synthetic> val x1: Int(2) = 2;
case16(){
<synthetic> val o18: scala.collection.SeqOps = A.unapplySeq(x1);
if (scala.collection.SeqFactory.UnapplySeqWrapper.isEmpty$extension(o18).unary_!())
if (false.unary_!())
{
val xs: Seq = scala.collection.SeqFactory.UnapplySeqWrapper.toSeq$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o18));
matchEnd15(xs)
Expand All @@ -277,7 +277,7 @@ package <empty> {
};
case17(){
<synthetic> val o20: scala.collection.SeqOps = A.unapplySeq(x1);
if (scala.collection.SeqFactory.UnapplySeqWrapper.isEmpty$extension(o20).unary_!().&&(new collection.SeqFactory.UnapplySeqWrapper(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20)).!=(null).&&(scala.collection.SeqFactory.UnapplySeqWrapper.lengthCompare$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20), 2).==(0))))
if (false.unary_!().&&(new collection.SeqFactory.UnapplySeqWrapper(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20)).!=(null).&&(scala.collection.SeqFactory.UnapplySeqWrapper.lengthCompare$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20), 2).==(0))))
{
val x: Int = unbox(scala.collection.SeqFactory.UnapplySeqWrapper.apply$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20), 0));
val y: Int = unbox(scala.collection.SeqFactory.UnapplySeqWrapper.apply$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o20), 1));
Expand All @@ -288,7 +288,7 @@ package <empty> {
};
case19(){
<synthetic> val o22: scala.collection.SeqOps = A.unapplySeq(x1);
if (scala.collection.SeqFactory.UnapplySeqWrapper.isEmpty$extension(o22).unary_!().&&(new collection.SeqFactory.UnapplySeqWrapper(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22)).!=(null).&&(scala.collection.SeqFactory.UnapplySeqWrapper.lengthCompare$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22), 1).>=(0))))
if (false.unary_!().&&(new collection.SeqFactory.UnapplySeqWrapper(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22)).!=(null).&&(scala.collection.SeqFactory.UnapplySeqWrapper.lengthCompare$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22), 1).>=(0))))
{
val x: Int = unbox(scala.collection.SeqFactory.UnapplySeqWrapper.apply$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22), 0));
val xs: Seq = scala.collection.SeqFactory.UnapplySeqWrapper.drop$extension(scala.collection.SeqFactory.UnapplySeqWrapper.get$extension(o22), 1);
Expand Down
4 changes: 0 additions & 4 deletions test/files/run/patmatnew.check
Expand Up @@ -16,10 +16,6 @@ patmatnew.scala:356: warning: multiline expressions might require enclosing pare
patmatnew.scala:356: warning: a pure expression does nothing in statement position
case 3 => assert(false); "KO"
^
patmatnew.scala:175: warning: match may not be exhaustive.
It would fail on the following inputs: List(_), Nil
def doMatch(xs: List[String]): String = xs match {
^
patmatnew.scala:178: warning: match may not be exhaustive.
It would fail on the following inputs: List(_), Nil
def doMatch2(xs: List[String]): List[String] = xs match {
Expand Down
4 changes: 0 additions & 4 deletions test/files/run/t3530.check
@@ -1,7 +1,3 @@
t3530.scala:9: warning: match may not be exhaustive.
It would fail on the following inputs: List(_), Nil
def f2[T](x: List[T]) = println(x match {
^
two
three
list: 4
Expand Down

0 comments on commit add8a01

Please sign in to comment.