Skip to content
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

Allow name-based extractors to be irrefutable #9343

Merged
merged 4 commits into from Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -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 == 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 @@ -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 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
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 @@ -1240,8 +1240,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 @@ -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
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
}
}
}
}