Update ReTyper to Ycheck patterns & inline fixes #3143
Conversation
| case TypeApply(fn, targs) => | ||
| val (meth, Nil, Nil) = decomposeCall(fn) | ||
| (meth, targs, Nil) | ||
| val (meth, targss, args) = decomposeCall(fn) |
There was a problem hiding this comment.
I don't understand the change. Can you give an example where this matters?
There was a problem hiding this comment.
The test added in the same commit (e199fc8) breaks with a match error before the change
There was a problem hiding this comment.
I think it would be much more efficient to implement this methods as:
def decomposeCall(tree: Tree): (Tree, List[Tree], List[List[Tree]]) = {
@tailrec def loop(tree: Tree, targs: List[Tree], argss: List[List[Tree]]): (Tree, List[Tree], List[List[Tree]]) =
tree match {
case Apply(fn, args) =>
loop(fn, targs, args :: argss)
case TypeApply(fn, targs) =>
loop(fn, targs, argss)
case _ =>
(tree, targs, argss)
}
loop(tree, Nil, Nil)
}| val unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil))) | ||
| val unapplyImplicits = unapplyApp match { | ||
| def unapplyImplicits(unapp: Tree = unapplyApp): List[Tree] = unapp match { | ||
| case Apply(Apply(unapply, `dummyArg` :: Nil), args2) => assert(args2.nonEmpty); args2 |
There was a problem hiding this comment.
I think it's safer in that case to not have a default argument for unapp.
|
|
||
| override def typedTyped(tree: untpd.Typed, pt: Type)(implicit ctx: Context): Tree = { | ||
| assertTyped(tree) | ||
| val type1 = checkSimpleKinded(typedType(tree.tpt)) |
There was a problem hiding this comment.
Rename type1 -> tpt1, tree1 -> expr1 to conform to naming conventions used elsewhere. (tpt stands for "type tree")
OlivierBlanvillain
force-pushed
the
ycheck-patterns
branch
from
26a240b to
b9db9f6
Compare
|
Completely forgot about this PR 😅. I've addressed the review and rebased, so it should be good to go! |
| final class Foo(val value: Int) | ||
|
|
||
| object Foo { | ||
| inline def unapplySeq(foo: Foo): Some[Seq[Int]] = Some(List(foo.value)) |
There was a problem hiding this comment.
Should this be changed to inline def unapply(foo: Foo) after #3747?
There was a problem hiding this comment.
I would say not for now, if/when unapplySeq is effectively not supported in Dotty then we should turn this tests into a scala2 test.
OlivierBlanvillain
force-pushed
the
ycheck-patterns
branch
from
2d846fc to
9c70648
Compare
See i3248.scala for an example of a non case class pattern leaking scala.annotation.internal.Repeated.
OlivierBlanvillain
force-pushed
the
ycheck-patterns
branch
from
9c70648 to
a506b50
Compare
odersky
left a comment
odersky
left a comment
There was a problem hiding this comment.
Just needs another rebase. LGTM.
3 participants
This PR does minor adjustments to Typer and ReTyper such that patterns pass Ycheck. This is required to be able to inline pattern matching expressions.