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

In 3.4 make refutable patterns in a for comprehension an error #18842

Merged
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/parsing/Parsers.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2771,7 +2771,7 @@ object Parsers {
atSpan(startOffset(pat), accept(LARROW)) {
val checkMode =
if casePat then GenCheckMode.FilterAlways
else if sourceVersion.isAtLeast(`future`) then GenCheckMode.Check
else if sourceVersion.isAtLeast(`3.4`) then GenCheckMode.Check
else if sourceVersion.isAtLeast(`3.2`) then GenCheckMode.CheckAndFilter
else GenCheckMode.FilterNow // filter on source version < 3.2, for backward compat
GenFrom(pat, subExpr(), checkMode)
Expand Down
8 changes: 7 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/Checking.scala
Original file line number Diff line number Diff line change
Expand Up @@ -923,7 +923,13 @@ trait Checking {
|
|If $usage is intentional, this can be communicated by $fix,
|which $addendum.$rewriteMsg"""),
pos, warnFrom = `3.2`, errorFrom = `future`)
pos,
warnFrom = `3.2`,
// we tighten for-comprehension without `case` to error in 3.4,
// but we keep pat-defs as warnings for now ("@unchecked"),
// until we propose an alternative way to assert exhaustivity to the typechecker.
errorFrom = if isPatDef then `future` else `3.4`
)
false
}

Expand Down
1 change: 1 addition & 0 deletions project/Build.scala
Original file line number Diff line number Diff line change
Expand Up @@ -1490,6 +1490,7 @@ object Build {
(
(dir / "shared/src/test/scala" ** (("*.scala": FileFilter)
-- "ReflectiveCallTest.scala" // uses many forms of structural calls that are not allowed in Scala 3 anymore
-- "UTF16Test.scala" // refutable pattern match
)).get

++ (dir / "shared/src/test/require-sam" ** "*.scala").get
Expand Down
12 changes: 12 additions & 0 deletions tests/neg/irrefutable.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
-- [E008] Not Found Error: tests/neg/irrefutable.scala:27:29 -----------------------------------------------------------
27 | for (case Foo(x: Int) <- xs) yield x // error
| ^^
| value withFilter is not a member of Lst[Foo[Any]]
-- Error: tests/neg/irrefutable.scala:30:16 ----------------------------------------------------------------------------
30 | for (Foo(x: Int) <- xs) yield x // error
| ^^^
| pattern's type Int is more specialized than the right hand side expression's type Any
|
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
| which will result in a filtering for expression (using `withFilter`).
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
42 changes: 42 additions & 0 deletions tests/neg/irrefutable.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// This tests that A.f1 is recognized as an irrefutable pattern and A.f2_nocase is not, and therefore A.f2 solves this
// by adding a case to the pattern, which results in withFilter being inserted.
// see also: tests/run/irrefutable.scala for an example that exercises the insertion of withFilter.

class Lst[+T](val id: String, val underlying: List[T]) {
def map[U](f: T => U): Lst[U] = new Lst(id, underlying.map(f))

// hide the withFilter so that there is a compile error
// def withFilter(f: T => Boolean): Lst.WithFilter[T] = new Lst.WithFilter(this, f)
}

// object Lst:
// class WithFilter[+T](lst: Lst[T], filter: T => Boolean):
// def forwardingFilter[T1](filter: T1 => Boolean): T1 => Boolean = t =>
// println(s"filtering $t in ${lst.id}")
// filter(t)

// def map[U](f: T => U): Lst[U] = Lst(lst.id, lst.underlying.withFilter(forwardingFilter(filter)).map(f))

case class Foo[T](x: T)

object A {
def f1(xs: Lst[Foo[Int]]): Lst[Int] = {
for (Foo(x: Int) <- xs) yield x
}
def f2(xs: Lst[Foo[Any]]): Lst[Int] = {
for (case Foo(x: Int) <- xs) yield x // error
}
def f2_nocase(xs: Lst[Foo[Any]]): Lst[Int] = {
for (Foo(x: Int) <- xs) yield x // error
}
}

@main def Test =
val xs = new Lst("xs", List(Foo(1), Foo(2), Foo(3)))
println("=== mapping xs with A.f1 ===")
val xs1 = A.f1(xs)
assert(xs1.underlying == List(1, 2, 3))
val ys = new Lst("ys", List(Foo(1: Any), Foo(2: Any), Foo(3: Any)))
println("=== mapping ys with A.f2 ===")
val ys1 = A.f2(ys)
assert(ys1.underlying == List(1, 2, 3))
32 changes: 16 additions & 16 deletions tests/neg/refutable-pattern-binding-messages.check
Original file line number Diff line number Diff line change
@@ -1,11 +1,3 @@
-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------
5 | val Positive(p) = 5 // error: refutable extractor
| ^^^^^^^^^^^^^^^
| pattern binding uses refutable extractor `Test.Positive`
|
| If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression,
| which may result in a MatchError at runtime.
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:6:14 ------------------------------------------------------
6 | for Positive(i) <- List(1, 2, 3) do () // error: refutable extractor
| ^^^^^^^^^^^
Expand All @@ -14,14 +6,6 @@
| If this usage is intentional, this can be communicated by adding the `case` keyword before the full pattern,
| which will result in a filtering for expression (using `withFilter`).
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 -----------------------------------------------------
10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized
| ^^^^^^^^^^^^^
| pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int]
|
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
| which may result in a MatchError at runtime.
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:11:11 -----------------------------------------------------
11 | for ((x: String) <- xs) do () // error: pattern type more specialized
| ^^^^^^
Expand All @@ -38,6 +22,22 @@
| If the narrowing is intentional, this can be communicated by adding the `case` keyword before the full pattern,
| which will result in a filtering for expression (using `withFilter`).
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:5:14 ------------------------------------------------------
5 | val Positive(p) = 5 // error: refutable extractor
| ^^^^^^^^^^^^^^^
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did these error messages move? It seems non-optimal, as the messages are not in source order anymore.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all that was changed was the warning became an error, so perhaps under -Werror true errors print last?

| pattern binding uses refutable extractor `Test.Positive`
|
| If this usage is intentional, this can be communicated by adding `: @unchecked` after the expression,
| which may result in a MatchError at runtime.
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:10:20 -----------------------------------------------------
10 | val i :: is = List(1, 2, 3) // error: pattern type more specialized
| ^^^^^^^^^^^^^
| pattern's type ::[Int] is more specialized than the right hand side expression's type List[Int]
|
| If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression,
| which may result in a MatchError at runtime.
| This patch can be rewritten automatically under -rewrite -source 3.2-migration.
-- Error: tests/neg/refutable-pattern-binding-messages.scala:16:10 -----------------------------------------------------
16 | val 1 = 2 // error: pattern type does not match
| ^
Expand Down
22 changes: 0 additions & 22 deletions tests/pos/irrefutable.scala

This file was deleted.

2 changes: 1 addition & 1 deletion tests/pos/t6205.scala
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
class A[T]
class Test1 {
def x(backing: Map[A[_], Any]) =
for( (k: A[kt], v) <- backing)
for(case (k: A[kt], v) <- backing)
yield (k: A[kt])
}

Expand Down
4 changes: 2 additions & 2 deletions tests/run/ReplacementMatching.scala
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ object Test {

def groupsMatching: Unit = {
val Date = """(\d+)/(\d+)/(\d+)""".r
for (Regex.Groups(a, b, c) <- Date findFirstMatchIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.") {
for (case Regex.Groups(a, b, c) <- Date findFirstMatchIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.") {
assert(a == "1")
assert(b == "1")
assert(c == "2001")
}
for (Regex.Groups(a, b, c) <- (Date findAllIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.").matchData) {
for (case Regex.Groups(a, b, c) <- (Date findAllIn "1/1/2001 marks the start of the millennium. 31/12/2000 doesn't.").matchData) {
assert(a == "1" || a == "31")
assert(b == "1" || b == "12")
assert(c == "2001" || c == "2000")
Expand Down
5 changes: 5 additions & 0 deletions tests/run/irrefutable.check
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
=== mapping xs with A.f1 ===
=== mapping ys with A.f2 ===
filtering Foo(1) in ys
filtering Foo(2) in ys
filtering Foo(3) in ys
36 changes: 36 additions & 0 deletions tests/run/irrefutable.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
// This tests that A.f1 does not filter its inputs, whereas A.f2 does.
// see also: tests/neg/irrefutable.scala for an example that exercises the requirement to insert case.

class Lst[+T](val id: String, val underlying: List[T]) {
def map[U](f: T => U): Lst[U] = new Lst(id, underlying.map(f))
def withFilter(f: T => Boolean): Lst.WithFilter[T] = new Lst.WithFilter(this, f)
}

object Lst:
class WithFilter[+T](lst: Lst[T], filter: T => Boolean):
def forwardingFilter[T1](filter: T1 => Boolean): T1 => Boolean = t =>
println(s"filtering $t in ${lst.id}")
filter(t)

def map[U](f: T => U): Lst[U] = Lst(lst.id, lst.underlying.withFilter(forwardingFilter(filter)).map(f))

case class Foo[T](x: T)

object A {
def f1(xs: Lst[Foo[Int]]): Lst[Int] = {
for (Foo(x: Int) <- xs) yield x
}
def f2(xs: Lst[Foo[Any]]): Lst[Int] = {
for (case Foo(x: Int) <- xs) yield x
}
}

@main def Test =
val xs = new Lst("xs", List(Foo(1), Foo(2), Foo(3)))
println("=== mapping xs with A.f1 ===")
val xs1 = A.f1(xs)
assert(xs1.underlying == List(1, 2, 3))
val ys = new Lst("ys", List(Foo(1: Any), Foo(2: Any), Foo(3: Any)))
println("=== mapping ys with A.f2 ===")
val ys1 = A.f2(ys)
assert(ys1.underlying == List(1, 2, 3))
2 changes: 1 addition & 1 deletion tests/run/patmat-bind-typed.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
object Test {
def f(xs: List[Any]) = for (key @ (dummy: String) <- xs) yield key
def f(xs: List[Any]) = for (case key @ (dummy: String) <- xs) yield key

def main(args: Array[String]): Unit = {
f("abc" :: Nil) foreach println
Expand Down
8 changes: 4 additions & 4 deletions tests/run/quoted-sematics-1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -82,24 +82,24 @@ def typeChecks(g: Gamma)(level: 0 | 1)(term: Term): Option[Type] =
yield LambdaType(t, res)
case App(fun, arg) => // T-App
for
LambdaType(t1, t2) <- typeChecks(g)(level)(fun)
case LambdaType(t1, t2) <- typeChecks(g)(level)(fun)
`t1` <- typeChecks(g)(level)(arg)
yield t2
case Box(body) if level == 0 => // T-Box
for t <- typeChecks(g)(1)(body) yield BoxType(t)
case Lift(body) if level == 0 => // T-Lift
for NatType <- typeChecks(g)(0)(body) yield BoxType(NatType)
case Splice(body) if level == 1 => // T-Unbox
for BoxType(t) <- typeChecks(g)(0)(body) yield t
for case BoxType(t) <- typeChecks(g)(0)(body) yield t
case Match(scrutinee, pat, thenp, elsep) => // T-Pat
for
BoxType(t1) <- typeChecks(g)(0)(scrutinee)
case BoxType(t1) <- typeChecks(g)(0)(scrutinee)
delta <- typePatChecks(g, t1)(pat)
t <- typeChecks(g ++ delta)(0)(thenp)
`t` <- typeChecks(g)(0)(elsep)
yield t
case Fix(t) if level == 0 =>
for LambdaType(t1, t2) <- typeChecks(g)(0)(t) yield t2 // T-Fix
for case LambdaType(t1, t2) <- typeChecks(g)(0)(t) yield t2 // T-Fix
case _ => None
if res.isEmpty then
println(s"Failed to type $term at level $level with environment $g")
Expand Down
4 changes: 2 additions & 2 deletions tests/run/t6406-regextract.scala
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,10 @@ object Test extends App {
val t = "Last modified 2011-07-15"
val p1 = """(\d\d\d\d)-(\d\d)-(\d\d)""".r
val y1: Option[String] = for {
p1(year, month, day) <- p1 findFirstIn t
case p1(year, month, day) <- p1 findFirstIn t
} yield year
val y2: Option[String] = for {
p1(year, month, day) <- p1 findFirstMatchIn t
case p1(year, month, day) <- p1 findFirstMatchIn t
} yield year
println(s"$y1 $y2")

Expand Down
6 changes: 3 additions & 3 deletions tests/run/t6646.scala
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ object Test {
val l = List(PrimaryKey, NoNull, lower)

// withFilter must be generated in these
for (option @ NoNull <- l) println("Found " + option)
for (option @ `lower` <- l) println("Found " + option)
for ((`lower`, i) <- l.zipWithIndex) println("Found " + i)
for (case option @ NoNull <- l) println("Found " + option)
for (case option @ `lower` <- l) println("Found " + option)
for (case (`lower`, i) <- l.zipWithIndex) println("Found " + i)

// no withFilter
for (X <- List("A single ident is always a pattern")) println(X)
Expand Down
2 changes: 1 addition & 1 deletion tests/run/t6968.scala
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
object Test {
def main(args: Array[String]): Unit = {
val mixedList = List(1,(1,2),4,(3,1),(5,4),6)
val as = for((a,b) <- mixedList) yield a
val as = for(case (a,b) <- mixedList) yield a
println(as.mkString(", "))
}
}
Loading