Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

SI-6968 Simple Tuple patterns aren't irrefutable #1893

Closed
wants to merge 1 commit into from

3 participants

@retronym
Owner

Reverts part of c82ecab. The parser can't assume that
a pattern (a, b) will match, as results of
.isInstanceOf[Tuple2] can't be statically known until
after the typer.

The reopens SI-1336 and SI-5589, in exchange for fixing
this regression SI-6968. Keeping all three fixed will require
some major surgery.

Review by @paulp

@retronym retronym SI-6968 Simple Tuple patterns aren't irrefutable
Reverts part of c82ecab. The parser can't assume that
a pattern `(a, b)` will match, as results of
`.isInstanceOf[Tuple2]` can't be statically known until
after the typer.

The reopens SI-1336 and SI-5589, in exchange for fixing
this regression SI-6968. Keeping all three fixed will require
some major surgery.
c2bc229
@scala-jenkins

Started jenkins job pr-scala-testsuite-linux-opt at https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2085/

@scala-jenkins

jenkins job pr-scala-testsuite-linux-opt: Failed - https://scala-webapps.epfl.ch/jenkins/job/pr-scala-testsuite-linux-opt/2085/

@adriaanm
Owner
testing: [...]/files/neg/t5589neg2.scala
t5589neg2.scala:7: error: constructor cannot be instantiated to expected type;
 found   : (T1, T2)
 required: String
    for (((((a, (b, (c, (d1, d2)))), es), fs), gs) <- x) yield (d :: es).mkString(", ") // not ok
                        ^
one error found

testing: [...]/files/run/t4574.scala
java.lang.AssertionError: assertion failed: Should not succeed.
    at Test$.expectMatchError(t4574.scala:5)

    at Test$.main(t4574.scala:10)

testing: [...]/files/scalacheck/parallel-collections
ParallelMapCheck1.scala:23: error: constructor cannot be instantiated to expected type;
 found   : (T1, T2)
 required: scala.collection.parallel.ParMap[K,V]
    val containsSelf = for ((k, v) <- coll) yield (coll.get(k) == Some(v))
                            ^
one error found
@retronym
Owner

Closing this, back in a day or two. There is another problem: the refutability checking in RefChecks, albeit somewhat ineffectual and now sometimes obviated by the (slightly overeager) checking in the parser, no longer works at all.

It suffers from two problems:

     case Apply(
        Select(qual, nme.filter | nme.withFilter),
        List(Function(
          List(ValDef(_, pname, tpt, _)),
          Match(_, CaseDef(pat1, _, _) :: _))))
        if ((pname startsWith nme.CHECK_IF_REFUTABLE_STRING) &&
            isIrrefutable(pat1, tpt.tpe) && (qual.tpe <:< tree.tpe)) =>

          transform(qual)
  • It doesn't handle TypeApply, the sort of problem that is now easily solved with treeInfo.Applied.
  • match nodes are eliminated by patmat under the new pattern matcher.

I've got a fix coming that uses the results of pattern matcher unreachability analysis (re-run with different assumptions about nullability) to make this work again.

@retronym retronym closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Jan 13, 2013
  1. @retronym

    SI-6968 Simple Tuple patterns aren't irrefutable

    retronym authored
    Reverts part of c82ecab. The parser can't assume that
    a pattern `(a, b)` will match, as results of
    `.isInstanceOf[Tuple2]` can't be statically known until
    after the typer.
    
    The reopens SI-1336 and SI-5589, in exchange for fixing
    this regression SI-6968. Keeping all three fixed will require
    some major surgery.
This page is out of date. Refresh to see the latest.
View
4 src/reflect/scala/reflect/internal/TreeInfo.scala
@@ -291,10 +291,6 @@ abstract class TreeInfo {
tree match {
case Bind(name, pat) => isVarPatternDeep0(pat)
case Ident(name) => isVarPattern(tree)
- case Apply(sel, args) =>
- ( isReferenceToScalaMember(sel, TupleClass(args.size).name.toTermName)
- && (args forall isVarPatternDeep0)
- )
case _ => false
}
}
View
19 test/files/neg/t5589neg.check
@@ -14,24 +14,21 @@ t5589neg.scala:3: error: constructor cannot be instantiated to expected type;
required: String
def f6(x: Either[Int, String]) = for ((y1, y2: Any) <- x.right) yield ((y1, y2))
^
+t5589neg.scala:4: warning: `withFilter' method does not yet exist on scala.util.Either.RightProjection[Int,(String, Int)], using `filter' method instead
+ def f7(x: Either[Int, (String, Int)]) = for (y1 @ Tuple1(y2) <- x.right) yield ((y1, y2))
+ ^
t5589neg.scala:4: error: constructor cannot be instantiated to expected type;
found : (T1,)
required: (String, Int)
def f7(x: Either[Int, (String, Int)]) = for (y1 @ Tuple1(y2) <- x.right) yield ((y1, y2))
^
-t5589neg.scala:4: error: not found: value y2
- def f7(x: Either[Int, (String, Int)]) = for (y1 @ Tuple1(y2) <- x.right) yield ((y1, y2))
- ^
+t5589neg.scala:5: warning: `withFilter' method does not yet exist on scala.util.Either.RightProjection[Int,(String, Int)], using `filter' method instead
+ def f8(x: Either[Int, (String, Int)]) = for ((y1, y2, y3) <- x.right) yield ((y1, y2))
+ ^
t5589neg.scala:5: error: constructor cannot be instantiated to expected type;
found : (T1, T2, T3)
required: (String, Int)
def f8(x: Either[Int, (String, Int)]) = for ((y1, y2, y3) <- x.right) yield ((y1, y2))
^
-t5589neg.scala:5: error: not found: value y1
- def f8(x: Either[Int, (String, Int)]) = for ((y1, y2, y3) <- x.right) yield ((y1, y2))
- ^
-t5589neg.scala:5: error: not found: value y2
- def f8(x: Either[Int, (String, Int)]) = for ((y1, y2, y3) <- x.right) yield ((y1, y2))
- ^
-two warnings found
-7 errors found
+four warnings found
+four errors found
View
1  test/files/run/t6968.check
@@ -0,0 +1 @@
+1, 3, 5
View
7 test/files/run/t6968.scala
@@ -0,0 +1,7 @@
+object Test {
+ def main(args: Array[String]) {
+ val mixedList = List(1,(1,2),4,(3,1),(5,4),6)
+ val as = for((a,b) <- mixedList) yield a
+ println(as.mkString(", "))
+ }
+}
View
0  test/files/pos/t1336.scala → test/pending/pos/t1336.scala
File renamed without changes
View
0  test/files/pos/t5589.scala → test/pending/pos/t5589.scala
File renamed without changes
Something went wrong with that request. Please try again.