-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[WIP] Take typed identifiers into account when optimizing matches #24972
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
base: main
Are you sure you want to change the base?
[WIP] Take typed identifiers into account when optimizing matches #24972
Conversation
1296cdb to
495187a
Compare
c4103a3 to
39116e9
Compare
| sel | ||
| end match | ||
|
|
||
| case class TuplePatternInfo(arity: Int, varNum: Int, wildcardNum: Int) |
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.
Review in split diff, this is a rewrite
| def makeLambda(gen: GenFrom, body: Tree): Tree = gen.pat match { | ||
| case IdPattern(named, tpt) if gen.checkMode != GenCheckMode.FilterAlways => | ||
| Function(derivedValDef(gen.pat, named, tpt, EmptyTree, Modifiers(Param)) :: Nil, body) | ||
| case IdPattern(named, tpt) if named.name != nme.WILDCARD && gen.checkMode != GenCheckMode.FilterAlways => |
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 found it quite confusing that IdPattern ignored wildcards, and in my case I wanted "any (un)typed ID, even a wildcard", so I moved the check to the existing uses and removed it from IdPattern
| case _ => Modifiers() | ||
| makePatDef(valeq, mods, defpat, rhs) | ||
| val pdefs = valeqs.lazyZip(defpats).map { case (valeq @ GenAlias(_, rhs), defpat) => | ||
| makePatDef(defpat, rhs, valeq.span, givenPatternsAllowed = true) |
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.
No behavior change here (hopefully), just cleaning up some rather messy maps and zips
| case _ => Modifiers() | ||
| makePatDef(valeq, mods, defpat, rhs) | ||
| val pdefs = valeqs.map { case valeq @ GenAlias(pat, rhs) => | ||
| makePatDef(makeIdPat(pat)._1, rhs, valeq.span, givenPatternsAllowed = true) |
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.
Ditto
| @@ -1,3 +1,3 @@ | |||
| object Foo { | |||
| val (A, B) = () // error // error | |||
| val (A, B) = () // error | |||
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.
Not quite sure why there were 2 errors before but I think 1 is quite enough :)
| def test = | ||
| val (x: "x", y: "y") = (("x", "y"): ("x", "y")) | ||
| println(x) | ||
| } |
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.
This is the original test from the issue this PR is fixing
|
|
||
| def test = | ||
| val (x: List[B @unchecked], y: List[C @unchecked]) = foo(): @unchecked | ||
| bar(x, y) |
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.
This is a simplified version of code in Typer.scala that failed to compile in my initial attempt at fixing this
Fixes #24435
makePatDefwas quite hard to understand and also wrong in edge cases related to types and `@unchecked conversions.I rewrote it with (1) detailed comments of what the optimizations are, (2) support for more patterns such as
Typed(Tuple(_), _), and (3) aprintingtest that enshrines what we expect from these optimizations.