-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Suppress warnings in comprehensions with 22+ binds #23590
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
Conversation
Fixes scala#23164 At that point, we emit a TupleXXL pattern case bundling all the variable bindings so far. The type tests in these patterns cannot be checked at runtime due to erasure. But we know these are safe, so we mark them as `@unchecked`.
makeCaseLambda(CaseDef(gen.pat, EmptyTree, body) :: Nil, matchCheckMode) | ||
val pat = gen.pat.match | ||
case Tuple(pats) if pats.length > Definitions.MaxImplementedFunctionArity => | ||
/* The pattern case is a tupleXXL, because we have bound > 21 variables in the comprehension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/* The pattern case is a tupleXXL, because we have bound > 21 variables in the comprehension. | |
/* The pattern case is a tupleXXL, because we have bound > 22 variables in the comprehension. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Depends on how you look at it. If the source program has > 21 binds, then we'll get the TupleXXL. For <= 21 we won't. The tuple in the pattern case being generated here will always carry one variable more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There exists a Tuple22
class: https://dotty.epfl.ch/api/scala/Tuple22.html#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rule is if we have n
binds, then the generated case
will destructure a tuple of length n + 1
. The example in the issue has 22, which is > 21, ergo we will match on a 23 tuple and get a TupleXXL.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall looks good to me.
/* The pattern case is a tupleXXL, because we have bound > 21 variables in the comprehension. | ||
* In this case, we need to mark all the typed patterns as @unchecked, or get loads of warnings. | ||
* Cf. warn test i23164.scala */ | ||
Tuple: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would just annotating the whole pattern work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure. Before this PR, the generated case would look like
val xxl = scala.runtime.TupleXXL(1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23)
(xxl: @unchecked).match // <- @unchecked inserted here
case scala.runtime.TupleXXL(a1,a2,a3,a4,a5,a6:List[Int],a7,a8,a9,a10,a11,a12,a13,a14,a15,a16,a17,a18,a19,a20,a21,a22,a23) => true
// ^ gives a warning
case _ => false
I'm unable to surround the whole scala.runtime.TupleXXL(a1,a2,...)
with @unchecked
and get it to compile.
Edit: Also, surrounding the entire match with @unchecked
does not suppress the warning for the :List[Int]
type test.
Fixes #23164
At that point, we emit a TupleXXL pattern case bundling all the variable bindings so far. The type tests in these patterns cannot be checked at runtime due to erasure. But we know these are safe, so we mark them as
@unchecked
.