diff options
author | Jason Zaugg <jzaugg@gmail.com> | 2015-09-29 13:21:44 +1000 |
---|---|---|
committer | Jason Zaugg <jzaugg@gmail.com> | 2015-09-29 13:21:44 +1000 |
commit | b81d2b21a70cfd23fe343ff0e58a9db83548b395 (patch) | |
tree | 22e914ea43b07dcb2524d135bd2934970ba465b6 | |
parent | 27da46343cd545534819300235bc64ab74958c92 (diff) | |
parent | 3b396884e6c153588ba25123d82eb4ed703fd844 (diff) | |
download | scala-b81d2b21a70cfd23fe343ff0e58a9db83548b395.tar.gz scala-b81d2b21a70cfd23fe343ff0e58a9db83548b395.tar.bz2 scala-b81d2b21a70cfd23fe343ff0e58a9db83548b395.zip |
Merge pull request #4720 from retronym/ticket/9029
SI-9029 Fix regression in extractor patterns
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/UnCurry.scala | 7 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/patmat/PatternExpander.scala | 18 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala | 50 | ||||
-rw-r--r-- | test/files/neg/t4425b.check | 22 | ||||
-rw-r--r-- | test/files/neg/t8127a.check | 4 | ||||
-rw-r--r-- | test/files/neg/t8127a.scala | 12 | ||||
-rw-r--r-- | test/files/neg/t8989.check | 4 | ||||
-rw-r--r-- | test/files/neg/t8989.scala | 14 | ||||
-rw-r--r-- | test/files/run/t7850c.scala | 11 | ||||
-rw-r--r-- | test/files/run/t7850d.scala | 17 | ||||
-rw-r--r-- | test/files/run/t9029.flags | 1 | ||||
-rw-r--r-- | test/files/run/t9029.scala | 15 | ||||
-rw-r--r-- | test/files/run/t9029b.check | 1 | ||||
-rw-r--r-- | test/files/run/t9029b.scala | 31 | ||||
-rw-r--r-- | test/files/run/t9029c.scala | 21 |
15 files changed, 192 insertions, 36 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/UnCurry.scala b/src/compiler/scala/tools/nsc/transform/UnCurry.scala index 163c44822e..ee7c839de9 100644 --- a/src/compiler/scala/tools/nsc/transform/UnCurry.scala +++ b/src/compiler/scala/tools/nsc/transform/UnCurry.scala @@ -475,13 +475,6 @@ abstract class UnCurry extends InfoTransform withNeedLift(needLift = true) { super.transform(tree) } else super.transform(tree) - case UnApply(fn, args) => - val fn1 = transform(fn) - val args1 = fn.symbol.name match { - case nme.unapplySeq => transformArgs(tree.pos, fn.symbol, args, patmat.alignPatterns(global.typer.context, tree).expectedTypes) - case _ => args - } - treeCopy.UnApply(tree, fn1, args1) case Apply(fn, args) => val needLift = needTryLift || !fn.symbol.isLabel // SI-6749, no need to lift in args to label jumps. diff --git a/src/compiler/scala/tools/nsc/transform/patmat/PatternExpander.scala b/src/compiler/scala/tools/nsc/transform/patmat/PatternExpander.scala index e84ccbf754..1916050dd8 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/PatternExpander.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/PatternExpander.scala @@ -86,9 +86,25 @@ trait PatternExpander[Pattern, Type] { * @param fixed The non-sequence types which are extracted * @param repeated The sequence type which is extracted */ - final case class Extractor(whole: Type, fixed: List[Type], repeated: Repeated) { + final case class Extractor(whole: Type, fixed: List[Type], repeated: Repeated, typeOfSinglePattern: Type) { require(whole != NoType, s"expandTypes($whole, $fixed, $repeated)") + /** A pattern with arity-1 that doesn't match the arity of the Product-like result of the `get` method, + * will match that result in its entirety. Example: + * + * {{{ + * warning: there was one deprecation warning; re-run with -deprecation for details + * scala> object Extractor { def unapply(a: Any): Option[(Int, String)] = Some((1, "2")) } + * defined object Extractor + * + * scala> "" match { case Extractor(x: Int, y: String) => } + * + * scala> "" match { case Extractor(xy : (Int, String)) => } + * warning: there was one deprecation warning; re-run with -deprecation for details + * }}} + * */ + def asSinglePattern: Extractor = copy(fixed = List(typeOfSinglePattern)) + def productArity = fixed.length def hasSeq = repeated.exists def elementType = repeated.elementType diff --git a/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala b/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala index b1783dc81f..d4f44303bb 100644 --- a/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala +++ b/src/compiler/scala/tools/nsc/transform/patmat/ScalacPatternExpanders.scala @@ -43,8 +43,9 @@ trait ScalacPatternExpanders { orElse definitions.elementType(ArrayClass, seq) ) } - def newExtractor(whole: Type, fixed: List[Type], repeated: Repeated): Extractor = - logResult(s"newExtractor($whole, $fixed, $repeated")(Extractor(whole, fixed, repeated)) + def newExtractor(whole: Type, fixed: List[Type], repeated: Repeated, typeOfSinglePattern: Type): Extractor = + logResult(s"newExtractor($whole, $fixed, $repeated, $typeOfSinglePattern")(Extractor(whole, fixed, repeated, typeOfSinglePattern)) + def newExtractor(whole: Type, fixed: List[Type], repeated: Repeated): Extractor = newExtractor(whole, fixed, repeated, tupleType(fixed)) // Turn Seq[A] into Repeated(Seq[A], A, A*) def repeatedFromSeq(seqType: Type): Repeated = { @@ -73,26 +74,27 @@ trait ScalacPatternExpanders { * Unfortunately the MethodType does not carry the information of whether * it was unapplySeq, so we have to funnel that information in separately. */ - def unapplyMethodTypes(whole: Type, result: Type, isSeq: Boolean): Extractor = { - val expanded = ( - if (result =:= BooleanTpe) Nil - else typeOfMemberNamedGet(result) match { + def unapplyMethodTypes(context: Context, whole: Type, result: Type, isSeq: Boolean): Extractor = { + if (result =:= BooleanTpe) newExtractor(whole, Nil, NoRepeated) + else { + val getResult = typeOfMemberNamedGet(result) + def noGetError() = { + val name = "unapply" + (if (isSeq) "Seq" else "") + context.error(context.tree.pos, s"The result type of an $name method must contain a member `get` to be used as an extractor pattern, no such member exists in ${result}") + } + val expanded = getResult match { + case global.NoType => noGetError(); Nil case rawGet if !hasSelectors(rawGet) => rawGet :: Nil case rawGet => typesOfSelectors(rawGet) } - ) - expanded match { - case init :+ last if isSeq => newExtractor(whole, init, repeatedFromSeq(last)) - case tps => newExtractor(whole, tps, NoRepeated) + expanded match { + case init :+ last if isSeq => newExtractor(whole, init, repeatedFromSeq(last), getResult) + case tps => newExtractor(whole, tps, NoRepeated, getResult) + } } } } object alignPatterns extends ScalacPatternExpander { - /** Converts a T => (A, B, C) extractor to a T => ((A, B, CC)) extractor. - */ - def tupleExtractor(extractor: Extractor): Extractor = - extractor.copy(fixed = tupleType(extractor.fixed) :: Nil) - private def validateAligned(context: Context, tree: Tree, aligned: Aligned): Aligned = { import aligned._ @@ -129,8 +131,8 @@ trait ScalacPatternExpanders { val isUnapply = sel.symbol.name == nme.unapply val extractor = sel.symbol.name match { - case nme.unapply => unapplyMethodTypes(firstParamType(fn.tpe), sel.tpe, isSeq = false) - case nme.unapplySeq => unapplyMethodTypes(firstParamType(fn.tpe), sel.tpe, isSeq = true) + case nme.unapply => unapplyMethodTypes(context, firstParamType(fn.tpe), sel.tpe, isSeq = false) + case nme.unapplySeq => unapplyMethodTypes(context, firstParamType(fn.tpe), sel.tpe, isSeq = true) case _ => applyMethodTypes(fn.tpe) } @@ -142,12 +144,14 @@ trait ScalacPatternExpanders { def acceptMessage = if (extractor.isErroneous) "" else s" to hold ${extractor.offeringString}" val requiresTupling = isUnapply && patterns.totalArity == 1 && productArity > 1 - if (requiresTupling && effectivePatternArity(args) == 1) { - val sym = sel.symbol.owner - currentRun.reporting.deprecationWarning(sel.pos, sym, s"${sym} expects $productArity patterns$acceptMessage but crushing into $productArity-tuple to fit single pattern (SI-6675)") - } - - val normalizedExtractor = if (requiresTupling) tupleExtractor(extractor) else extractor + val normalizedExtractor = if (requiresTupling) { + val tupled = extractor.asSinglePattern + if (effectivePatternArity(args) == 1 && isTupleType(extractor.typeOfSinglePattern)) { + val sym = sel.symbol.owner + currentRun.reporting.deprecationWarning(sel.pos, sym, s"${sym} expects $productArity patterns$acceptMessage but crushing into $productArity-tuple to fit single pattern (SI-6675)") + } + tupled + } else extractor validateAligned(context, fn, Aligned(patterns, normalizedExtractor)) } diff --git a/test/files/neg/t4425b.check b/test/files/neg/t4425b.check index 8418b4fd12..a204467586 100644 --- a/test/files/neg/t4425b.check +++ b/test/files/neg/t4425b.check @@ -22,16 +22,28 @@ t4425b.scala:10: error: object X is not a case class, nor does it have an unappl 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: too many patterns for object X: expected 1, found 2 +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 + println( "" match { case _ X _ => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:18: error: too many patterns for object X offering Boolean: expected 0, found 2 println( "" match { case _ X _ => "ok" ; case _ => "fail" }) ^ -t4425b.scala:19: error: too many patterns for object X: expected 1, found 2 +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 + println((X: Any) match { case _ X _ => "ok" ; case _ => "fail" }) + ^ +t4425b.scala:19: error: too many patterns for object X offering Boolean: expected 0, found 2 println((X: Any) match { case _ X _ => "ok" ; case _ => "fail" }) ^ -t4425b.scala:22: error: too many patterns for object X: expected 1, found 2 +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 + 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 + 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 println( "" match { case X(_, _) => "ok" ; case _ => "fail" }) ^ -t4425b.scala:23: error: too many patterns for object X: expected 1, found 2 +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 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 @@ -46,4 +58,4 @@ t4425b.scala:35: error: too many patterns for object X offering Nothing: expecte t4425b.scala:36: error: too many patterns for object X offering Nothing: expected 1, found 2 println((X: Any) match { case X(_, _) => "ok" ; case _ => "fail" }) ^ -14 errors found +18 errors found diff --git a/test/files/neg/t8127a.check b/test/files/neg/t8127a.check new file mode 100644 index 0000000000..5a30574861 --- /dev/null +++ b/test/files/neg/t8127a.check @@ -0,0 +1,4 @@ +t8127a.scala:7: error: The result type of an unapplySeq method must contain a member `get` to be used as an extractor pattern, no such member exists in Seq[_$1] + case H(v) => + ^ +one error found diff --git a/test/files/neg/t8127a.scala b/test/files/neg/t8127a.scala new file mode 100644 index 0000000000..c05facdac1 --- /dev/null +++ b/test/files/neg/t8127a.scala @@ -0,0 +1,12 @@ +object H { + def unapplySeq(m: Any): Seq[_] = List("") +} + +object Test { + def unapply(m: Any) = m match { + case H(v) => + case _ => + } + // now: too many patterns for object H offering Boolean: expected 0, found 1 + // was: result type Seq[_$2] of unapplySeq defined in method unapplySeq in object H does not conform to Option[_] +} diff --git a/test/files/neg/t8989.check b/test/files/neg/t8989.check new file mode 100644 index 0000000000..4e89b862bd --- /dev/null +++ b/test/files/neg/t8989.check @@ -0,0 +1,4 @@ +t8989.scala:11: 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 A + val f = p match {case d(1) => true; case _ => false} + ^ +one error found diff --git a/test/files/neg/t8989.scala b/test/files/neg/t8989.scala new file mode 100644 index 0000000000..8ed6a901cd --- /dev/null +++ b/test/files/neg/t8989.scala @@ -0,0 +1,14 @@ +class A extends Product1[Int] { + def _1 = 1 + def isEmpty = false // used by scalac + def isDefined = !isEmpty // used by dotty + def canEqual(a: Any) = true +} + +object d{ + def unapply(a: Any) = new A + val p: Any = ??? + val f = p match {case d(1) => true; case _ => false} +} + + diff --git a/test/files/run/t7850c.scala b/test/files/run/t7850c.scala new file mode 100644 index 0000000000..25b9c0028d --- /dev/null +++ b/test/files/run/t7850c.scala @@ -0,0 +1,11 @@ +// Testing that isEmpty and get are viewed with `memberType` from `Casey1`. +trait T[A, B >: Null] { def isEmpty: A = false.asInstanceOf[A]; def get: B = null} +class Casey1() extends T[Boolean, String] +object Casey1 { def unapply(a: Casey1) = a } + +object Test { + def main(args: Array[String]) { + val c @ Casey1(x) = new Casey1() + assert(x == c.get) + } +} diff --git a/test/files/run/t7850d.scala b/test/files/run/t7850d.scala new file mode 100644 index 0000000000..ccc98f1bcc --- /dev/null +++ b/test/files/run/t7850d.scala @@ -0,0 +1,17 @@ +// Testing that the ad-hoc overload resolution of isEmpty/get discards +// parameter-accepting variants +trait T[A, B >: Null] { def isEmpty: A = false.asInstanceOf[A]; def get: B = null} +class Casey1(val a: Int) { + def isEmpty: Boolean = false + def isEmpty(x: Int): Boolean = ??? + def get: Int = a + def get(x: Int): String = ??? +} +object Casey1 { def unapply(a: Casey1) = a } + +object Test { + def main(args: Array[String]) { + val c @ Casey1(x) = new Casey1(0) + assert(x == c.get) + } +} diff --git a/test/files/run/t9029.flags b/test/files/run/t9029.flags new file mode 100644 index 0000000000..dcc59ebe32 --- /dev/null +++ b/test/files/run/t9029.flags @@ -0,0 +1 @@ +-deprecation diff --git a/test/files/run/t9029.scala b/test/files/run/t9029.scala new file mode 100644 index 0000000000..c01033b76e --- /dev/null +++ b/test/files/run/t9029.scala @@ -0,0 +1,15 @@ +class Y(val _2: Int, val _1: String) + +object X { def unapply(u: Unit): Option[Y] = Some(new Y(42, "!")) } + +object Test { + def test1 = { + val X(y) = () + val yy: Y = y + assert(yy._1 == "!") + assert(yy._2 == 42) + } + def main(args: Array[String]): Unit = { + test1 + } +} diff --git a/test/files/run/t9029b.check b/test/files/run/t9029b.check new file mode 100644 index 0000000000..aeb2d5e239 --- /dev/null +++ b/test/files/run/t9029b.check @@ -0,0 +1 @@ +Some(1) diff --git a/test/files/run/t9029b.scala b/test/files/run/t9029b.scala new file mode 100644 index 0000000000..764d0771ec --- /dev/null +++ b/test/files/run/t9029b.scala @@ -0,0 +1,31 @@ +class Foo(val x: Bar) { + def isEmpty = false + def get = x +} + +object Foo { + def unapply(x: Foo) = x +} + +class Bar(val x: Option[Int], val y: Option[Int]) { + def isEmpty = false + def get = this + def _1 = x + def _2 = y +} + +object Bar { + def unapply(x: Bar) = x +} + +object Test { + def nameBased: Unit = { + val x: AnyRef = new Foo(new Bar(Some(1), Some(2))) + x match { + case Foo(Bar(x1, x2)) => println(x1) + } + } + def main(args: Array[String]): Unit = { + nameBased + } +} diff --git a/test/files/run/t9029c.scala b/test/files/run/t9029c.scala new file mode 100644 index 0000000000..ccb51e23ae --- /dev/null +++ b/test/files/run/t9029c.scala @@ -0,0 +1,21 @@ +object Extractor { + def unapply(a: Any): Option[Product2[Int, String]] = Some(new P2(1, "2")) +} +class P2[A, B](val _1: A, val _2: B) extends Product2[A, B] { + def canEqual(other: Any) = true + def isP2 = true +} + +object Test { + def main(args: Array[String]): Unit = { + "" match { + case Extractor(p) => + val pp: Product2[Int, String] = p + } + "" match { + case Extractor(x, y) => + val xx: Int = x + val yy: String = y + } + } +} |