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

Unexpected semantics of non-nested varargs pattern #17443

Closed
prolativ opened this issue May 9, 2023 · 3 comments · Fixed by #18055
Closed

Unexpected semantics of non-nested varargs pattern #17443

prolativ opened this issue May 9, 2023 · 3 comments · Fixed by #18055

Comments

@prolativ
Copy link
Contributor

prolativ commented May 9, 2023

Compiler version

3.3.1-RC1-bin-20230508-830230f-NIGHTLY and before

Minimized code

@main def run() =
  val x = List(1) match { case (xs*) => xs }
  val y = x.head
@main def run() =
  val x = List(1) match { case (xs*) => xs; case _ => Seq.empty }
  val y = x.head

Output

Compiling project (Scala 3.3.1-RC1-bin-20230508-830230f-NIGHTLY, JVM)
[warn] /var/folders/rp/f9y80jvs54lcy26rxnt2y2kw0000gn/T/Repro.scala:2:11
[warn] match may not be exhaustive.
[warn] 
[warn] It would fail on pattern case: List(_, _*), Nil
[warn]   val x = List(1) match { case (xs*) => xs }
[warn]           ^^^^^^^
Compiled project (Scala 3.3.1-RC1-bin-20230508-830230f-NIGHTLY, JVM)
Exception in thread "main" java.lang.ClassCastException: java.lang.Integer cannot be cast to scala.collection.immutable.List
        at Repro$package$.run(Repro.scala:3)
        at run.main(Repro.scala:1)
[warn] /var/folders/rp/f9y80jvs54lcy26rxnt2y2kw0000gn/T/Repro.scala:2:33
[warn] Unreachable case
[warn]   val x = List(1) match { case (xs*) => xs; case _ => Seq.empty }
[warn]                                 ^^^
Exception in thread "main" java.lang.ClassCastException: java.lang.Integer cannot be cast to scala.collection.immutable.List
        at Repro$package$.run(Repro.scala:3)
        at run.main(Repro.scala:1)

Expectation

Most probably patterns like case (xs*) => should be illegal, just as case xs* => is. @odersky Can you confirm?

In the code snippets above the type of x gets inferred to Seq[List[Int]] but its value is List(1), which doesn't make sense and causes the ClassCastException.
Also, the warning from the second snippet is wrong at the moment as the case reported as unreachable is actually the one that gets matched.

@sjrd
Copy link
Member

sjrd commented May 9, 2023

Scala 2 says:

scala> val x = List(1) match { case (xs @ _*) => xs }
                                            ^
       error: bad simple pattern: bad use of _* (sequence pattern not allowed)

@prolativ
Copy link
Contributor Author

prolativ commented May 9, 2023

To give some context: I found the problem while trying to understand the current syntax specification of pattern matching.
syntax.md says:

Pattern           ::=  Pattern1 { ‘|Pattern1 }
Pattern1          ::=  PatVar:RefinedType
                    | [‘-’] integerLiteral ‘:RefinedType
                    | [‘-’] floatingPointLiteral ‘:RefinedType
                    |  Pattern2
Pattern2          ::=  [id ‘@’] InfixPattern [‘*’]
InfixPattern      ::=  SimplePattern { id [nl] SimplePattern }
SimplePattern     ::=  PatVar
                    |  Literal
                    |  ‘(’ [Patterns] ‘)’
                    |  Quoted
                    |  SimplePattern1 [TypeArgs] [ArgumentPatterns]
                    |givenRefinedType
SimplePattern1    ::=  SimpleRef
                    |  SimplePattern1 ‘.’ id
PatVar            ::=  varid
                    |  ‘_’
Patterns          ::=  Pattern {‘,’ Pattern}
ArgumentPatterns  ::=  ‘(’ [Patterns] ‘)’
                    |  ‘(’ [Patterns ‘,’] PatVar*’ ‘)’

Unless I got something wrong, this seems to allow not only case (xs*) => but also case xs* =>.

This specification also seems to be out of sync with the comments in Parsers.scala:

Pattern     ::= Pattern1 { ‘|Pattern1 }
Pattern1    ::= PatVar Ascription
              | [‘-’] integerLiteral Ascription
              | [‘-’] floatingPointLiteral Ascription
              | Pattern2
Pattern2    ::= [id ‘@’] Pattern3
Pattern3    ::= InfixPattern
              | PatVar*

Beside the fact that Pattern3 is inlined in syntax.md, the comments in the implementation say that an infix pattern cannot be followed by *.
What would be the right syntax specification to place in both locations then?

For reference the syntax for scala 2.13.10 is as follows:

Pattern ::= Pattern1 { ‘|Pattern1 }
Pattern1 ::= boundvarid ‘:TypePat
| ‘_’ ‘:TypePat
| Pattern2
Pattern2 ::= id [‘@Pattern3]
| Pattern3
Pattern3 ::= SimplePattern
| SimplePattern {id [nl] SimplePattern}
SimplePattern ::= ‘_’
| varid
| Literal
| StableId
| StableId ‘(’ [Patterns] ‘)’
| StableId ‘(’ [Patterns ‘,’] [id ‘@’] ‘_’ ‘*’ ‘)’
| ‘(’ [Patterns] ‘)’
| XmlPattern
Patterns ::= Pattern {‘,’ Patterns}

@odersky
Copy link
Contributor

odersky commented Jun 25, 2023

I agree, xs* and (xs*) should both be illegal. And the discrepancies between parser grammar and syntax.md need to be fixed.

odersky added a commit to dotty-staging/dotty that referenced this issue Jun 25, 2023
Syntax: Remove outdated `*` after InfixPattern
Parsing: Only allow vararg `*` in ArgumentPatterns

Fixes scala#17443
odersky added a commit to dotty-staging/dotty that referenced this issue Jun 26, 2023
Syntax: Remove outdated `*` after InfixPattern
Parsing: Only allow vararg `*` in ArgumentPatterns

Fixes scala#17443
prolativ added a commit that referenced this issue Jun 27, 2023
Syntax: Remove outdated `*` after InfixPattern
Parsing: Only allow vararg `*` in ArgumentPatterns

Fixes #17443
@Kordyjan Kordyjan added this to the 3.4.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants