Skip to content

Returning a non-tuple into a val pattern match compiles #14472

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

Closed
Daenyth opened this issue Feb 14, 2022 · 10 comments
Closed

Returning a non-tuple into a val pattern match compiles #14472

Daenyth opened this issue Feb 14, 2022 · 10 comments

Comments

@Daenyth
Copy link

Daenyth commented Feb 14, 2022

Compiler version

scala 3.1.1

Minimized code

This code compiles, and fails at runtime with a MatchError:

val (n, m) = (1, 2) match {
  case _ => 1
  case null => (1,2)
}
val (n, m) = (1, 2) match {
  case pair if false => (1,2)
  case other => 1
}

Output

Fails with scala.MatchError: 1 (of class java.lang.Integer)

Expectation

The code should not compile; the return 1 cannot be destructured into a Tuple2

For comparison, the following code does not compile:

// cannot test if value of type (1 : Int) | (2 : Int) is a reference of class Tuple2
val (n, m) = (1, 2) match {
  case _ => 1
  case null => 2
}
// cannot test if value of type Int is a reference of class Tuple2
val (n, m) = (1, 2) match {
  case _ => 1
  case null => ???
}
// cannot test if value of type Int is a reference of class Tuple2
val (n, m) = (1, 2) match {
  case _ => 1
}
@Daenyth Daenyth added itype:bug stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 14, 2022
@SethTisue
Copy link
Member

SethTisue commented Feb 14, 2022

looks like typer is inserting an @unchecked:

[[syntax trees at end of                     typer]]
        val $1$: (Int, Int) = 
          (Tuple2.apply[Int, Int](1, 2) match 
            {
              case _ => 
                1
              case null => 
                Tuple2.apply[Int, Int](1, 2)
            }
          ):((1 : Int) | (Int, Int)) @unchecked match 
            {
              case Tuple2.unapply[Int, Int](n @ _, m @ _):(Int, Int) => 
                Tuple2.apply[Int, Int](n, m)
            }
        val n: Int = $1$._1
        val m: Int = $1$._2

@dwijnand dwijnand added area:pattern-matching area:typer and removed stat:needs triage Every issue needs to have an "area" and "itype" label labels Feb 14, 2022
@dwijnand dwijnand self-assigned this Feb 14, 2022
@SethTisue
Copy link
Member

SethTisue commented Feb 15, 2022

Note that it's reproducible without a guard, without null, and without any unreachable cases:

scala> val (n, m) = (0, 2) match {
     |   case (0, _) => 1
     |   case _ => (1, 2)
     | }
scala.MatchError: 1 (of class java.lang.Integer)

and it doesn't matter which prong has the Int and which has the Tuple2:

scala> val (n, m) = (0, 2) match {
     |   case (1, _) => (1, 2)
     |   case _ => 1
     | }
scala.MatchError: 1 (of class java.lang.Integer)

@SethTisue
Copy link
Member

SethTisue commented Feb 15, 2022

The val <pattern> = ... desugaring automatically slaps @unchecked on it, but the true-positive cases (the ones the compiler correctly rejects) are handled properly anyway. One next step here might to be understand why those cases do work. Dale and I intend to dig into it further.

@prolativ
Copy link
Contributor

This is not a new issue actually https://www.lihaoyi.com/post/WartsoftheScalaProgrammingLanguage.html#conflating-total-destructuring-with-partial-pattern-matching
Personally I really like the suggestion to introduce a distinction between vals and case vals

@dwijnand
Copy link
Member

I need to check, but it might be this is all implemented, it's just under the "-strict" flag: #6389

@dwijnand dwijnand removed their assignment Feb 23, 2022
@odersky
Copy link
Contributor

odersky commented Mar 13, 2022

It is under the -source future flag:

sc test.scala -source future
-- Warning: test.scala:2:20 ----------------------------------------------------
2 |val (n, m) = (1, 2) match {
  |             ^
  |pattern's type (Int, Int) is more specialized than the right hand side expression's type (1 : Int) | (Int, Int)
  |
  |If the narrowing is intentional, this can be communicated by adding `: @unchecked` after the expression.
3 |  case _ => 1
4 |  case null => (1,2)
5 |}

@odersky odersky closed this as completed Mar 13, 2022
@som-snytt
Copy link
Contributor

It's under -Xlint on scala 2, for any flag-challenged cross-compilers. Unreachable warnings are free.

@Daenyth
Copy link
Author

Daenyth commented Mar 15, 2022

I thought -source future was for syntax and language features, not lints? It seems like a flag that not all codebases will have.
What does it enable exactly?

@som-snytt
Copy link
Contributor

@Daenyth #13832

@bishabosha
Copy link
Member

bishabosha commented Mar 16, 2022

I thought -source future was for syntax and language features, not lints? It seems like a flag that not all codebases will have.
What does it enable exactly?

It is more than a lint (you are required to insert the @unchecked annotation) - it also removes implicitly inserting withFilter when a pattern appears in a for comprehension, (instead there would be MatchError). Adding withFilter under future is now made explicit by requiring case before the pattern.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants