Skip to content

Commit

Permalink
Merge pull request #9343 from martijnhoekstra/namebasedirrefutable
Browse files Browse the repository at this point in the history
  • Loading branch information
dwijnand committed Feb 17, 2021
2 parents 860d76a + a275b61 commit c475777
Show file tree
Hide file tree
Showing 21 changed files with 311 additions and 56 deletions.
9 changes: 9 additions & 0 deletions src/compiler/scala/tools/nsc/Global.scala
Expand Up @@ -1016,11 +1016,20 @@ class Global(var currentSettings: Settings, reporter0: Reporter)
&& rootMirror.isMirrorInitialized
)
override def isPastTyper = isPast(currentRun.typerPhase)
def isBeforeErasure = isBefore(currentRun.erasurePhase)
def isPast(phase: Phase) = (
(curRun ne null)
&& isGlobalInitialized // defense against init order issues
&& (globalPhase.id > phase.id)
)
def isBefore(phase: Phase) = (
(curRun ne null)
&& isGlobalInitialized // defense against init order issues
&& (phase match {
case NoPhase => true // if phase is NoPhase then that phase ain't comin', so we're "before it"
case _ => globalPhase.id < phase.id
})
)

// TODO - trim these to the absolute minimum.
@inline final def exitingErasure[T](op: => T): T = exitingPhase(currentRun.erasurePhase)(op)
Expand Down
40 changes: 37 additions & 3 deletions src/compiler/scala/tools/nsc/transform/patmat/MatchAnalysis.scala
Expand Up @@ -354,10 +354,42 @@ trait MatchApproximation extends TreeAndTypeAnalysis with ScalaLogic with MatchT
def tru = True
}


private def isIrrefutabilityProof(sym: Symbol): Boolean = {
sym.isMethod && sym.name == nme.isEmpty && {
// ConstantFalse is foldable but in joint compilation (bug?) this will be a literal type
// with case using `==` rather than `=:=` we need to do this instead. neg/t12240.scala
sym.tpe.finalResultType 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 irrefutable
case TypeRef(_, SomeClass, _) => true
//name based pattern matching checks for constant false `isEmpty`.
case TypeRef(_, res, _) => res.tpe.members.exists(isIrrefutabilityProof)
//`true.type` is irrefutable for boolean extractors
case c: ConstantType => c.value == Constant(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 @@ -426,11 +458,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 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 @@ -488,6 +521,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 = s"P(${prevBinder.name}, ${extraCond.fold("")(_.toString)}, ${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
Expand Up @@ -127,7 +127,7 @@ trait PatternExpansion {
// rest is private
private val isUnapply = fun.symbol.name == nme.unapply
private val isUnapplySeq = fun.symbol.name == nme.unapplySeq
private def isBooleanUnapply = isUnapply && unapplyResultType() =:= BooleanTpe
private def isBooleanUnapply = isUnapply && unapplyResultType().typeSymbol == definitions.BooleanClass
private def isRepeatedCaseClass = caseCtorParamTypes.exists(tpes => tpes.nonEmpty && isScalaRepeatedParamType(tpes.last))

private def caseCtorParamTypes: Option[List[Type]] =
Expand Down Expand Up @@ -248,7 +248,10 @@ trait PatternExpansion {

// emit error/warning on mismatch
if (isStar && !isSeq) err("Star pattern must correspond with varargs or unapplySeq")
else if (equivConstrParamTypes == List(NoType)) err(s"The result type of an ${fun.symbol.name} method must contain a member `get` to be used as an extractor pattern, no such member exists in ${unapplyResultType()}")
else if (equivConstrParamTypes == List(NoType) && unapplyResultType().isNothing)
err(s"${fun.symbol.owner} can't be used as an extractor: The result type of an ${fun.symbol.name} method may not be Nothing")
else if (equivConstrParamTypes == List(NoType))
err(s"The result type of an ${fun.symbol.name} method must contain a member `get` to be used as an extractor pattern, no such member exists in ${unapplyResultType()}")
else if (elementArity < 0) arityError("not enough")
else if (elementArity > 0 && !isSeq) arityError("too many")
else if (settings.warnStarsAlign && isSeq && productArity > 0 && elementArity > 0) warn(
Expand Down
4 changes: 2 additions & 2 deletions src/compiler/scala/tools/nsc/typechecker/Typers.scala
Expand Up @@ -1245,8 +1245,8 @@ trait Typers extends Adaptations with Tags with TypersTracking with PatternTyper
inferExprAlternative(tree, pt)
adaptAfterOverloadResolution(tree, mode, pt, original)
case NullaryMethodType(restpe) => // (2)
val resTpDeconst =
if (isPastTyper || (tree.symbol.isAccessor && tree.symbol.hasFlag(STABLE) && treeInfo.isExprSafeToInline(tree))) restpe
val resTpDeconst = // keep constant types when they are safe to fold. erasure eliminates constant types modulo some exceptions, so keep those.
if (isBeforeErasure && tree.symbol.isAccessor && tree.symbol.hasFlag(STABLE) && treeInfo.isExprSafeToInline(tree)) restpe
else restpe.deconst
adapt(tree setType resTpDeconst, mode, pt, original)
case TypeRef(_, ByNameParamClass, arg :: Nil) if mode.inExprMode => // (2)
Expand Down
2 changes: 1 addition & 1 deletion src/library/scala/Array.scala
Expand Up @@ -566,7 +566,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
12 changes: 1 addition & 11 deletions src/reflect/scala/reflect/internal/Symbols.scala
Expand Up @@ -3089,17 +3089,7 @@ trait Symbols extends api.Symbols { self: SymbolTable =>

override def isVarargs: Boolean = definitions.isVarArgsList(paramss.flatten)

override def returnType: Type = {
@tailrec
def loop(tpe: Type): Type =
tpe match {
case NullaryMethodType(ret) => loop(ret)
case MethodType(_, ret) => loop(ret)
case PolyType(_, tpe) => loop(tpe)
case tpe => tpe
}
loop(info)
}
override def returnType: Type = definitions.finalResultType(info)

override def exceptions = {
rawInfo match {
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-exhaustive"
}

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 know that this is exhaustive"
}

}
12 changes: 6 additions & 6 deletions test/files/neg/t4425b.check
Expand Up @@ -22,22 +22,22 @@ t4425b.scala:10: error: object X is not a case class, nor does it have a valid u
Note: def unapply(x: String)(y: String): Nothing exists in object X, but it cannot be used as an extractor due to its second non-implicit parameter list
println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" })
^
t4425b.scala:18: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:18: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println( "" match { case _ X _ => "ok" ; case _ => "fail" })
^
t4425b.scala:19: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:19: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println((X: Any) match { case _ X _ => "ok" ; case _ => "fail" })
^
t4425b.scala:20: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:20: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println( "" match { case X(_) => "ok" ; case _ => "fail" })
^
t4425b.scala:21: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:21: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println((X: Any) match { case X(_) => "ok" ; case _ => "fail" })
^
t4425b.scala:22: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:22: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println( "" match { case X(_, _) => "ok" ; case _ => "fail" })
^
t4425b.scala:23: error: The result type of an unapply method must contain a member `get` to be used as an extractor pattern, no such member exists in Nothing
t4425b.scala:23: error: object X can't be used as an extractor: The result type of an unapply method may not be Nothing
println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" })
^
t4425b.scala:31: error: too many patterns for object X offering Nothing: expected 1, found 2
Expand Down
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 was already 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"
}

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

object Test {
sealed trait Foo
final class Bar extends Foo

object Bar {
def unapply(o: Bar): true = true
}

def f(foo: Foo) = foo match {
case Bar() => println("Bar")
}
}
36 changes: 36 additions & 0 deletions test/files/run/patmat-no-inline-isEmpty.check
@@ -0,0 +1,36 @@
[[syntax trees at end of patmat]] // newSource1.scala
package <empty> {
object A extends scala.AnyRef {
def <init>(): A.type = {
A.super.<init>();
()
};
def unapplySeq(a: Int): Wrap = new Wrap(a)
};
class T extends scala.AnyRef {
def <init>(): T = {
T.super.<init>();
()
};
def t: Any = {
case <synthetic> val x1: Int(2) = 2;
case5(){
<synthetic> val o7: Wrap = A.unapplySeq(x1);
if (o7.isEmpty.unary_!)
{
val xs: Seq[Int] = o7.get.toSeq;
matchEnd4(xs)
}
else
case6()
};
case6(){
matchEnd4("other")
};
matchEnd4(x: Any){
x
}
}
}
}

0 comments on commit c475777

Please sign in to comment.