Skip to content

Update ReTyper to Ycheck patterns & inline fixes #3143

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

Merged
merged 9 commits into from
Feb 12, 2018
4 changes: 2 additions & 2 deletions compiler/src/dotty/tools/dotc/ast/TreeInfo.scala
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,8 @@ trait TypedTreeInfo extends TreeInfo[Type] { self: Trees.Instance[Type] =>
val (meth, targs, argss) = decomposeCall(fn)
(meth, targs, argss :+ args)
case TypeApply(fn, targs) =>
val (meth, Nil, Nil) = decomposeCall(fn)
(meth, targs, Nil)
val (meth, targss, args) = decomposeCall(fn)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand the change. Can you give an example where this matters?

Copy link
Contributor Author

@OlivierBlanvillain OlivierBlanvillain Jan 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test added in the same commit (e199fc8) breaks with a match error before the change

Copy link
Contributor

@allanrenucci allanrenucci Jan 22, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
  }

(meth, targs ++ targss, args)
case _ =>
(tree, Nil, Nil)
}
Expand Down
5 changes: 1 addition & 4 deletions compiler/src/dotty/tools/dotc/transform/TreeChecker.scala
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,6 @@ class TreeChecker extends Phase with SymTransformer {

override def typedUnadapted(tree: untpd.Tree, pt: Type)(implicit ctx: Context): tpd.Tree = {
val res = tree match {
case _: untpd.UnApply =>
// can't recheck patterns
tree.asInstanceOf[tpd.Tree]
case _: untpd.TypedSplice | _: untpd.Thicket | _: EmptyValDef[_] =>
super.typedUnadapted(tree)
case _ if tree.isType =>
Expand Down Expand Up @@ -291,7 +288,7 @@ class TreeChecker extends Phase with SymTransformer {
}

def checkNotRepeated(tree: Tree)(implicit ctx: Context): tree.type = {
def allowedRepeated = (tree.symbol.flags is Case) && tree.tpe.widen.isRepeatedParam
def allowedRepeated = tree.tpe.widen.isRepeatedParam

assert(!tree.tpe.widen.isRepeatedParam || allowedRepeated, i"repeated parameter type not allowed here: $tree")
tree
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/typer/Applications.scala
Original file line number Diff line number Diff line change
Expand Up @@ -955,9 +955,10 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
}
val dummyArg = dummyTreeOfType(ownType)
val unapplyApp = typedExpr(untpd.TypedSplice(Apply(unapplyFn, dummyArg :: Nil)))
val unapplyImplicits = unapplyApp match {
def unapplyImplicits(unapp: Tree): List[Tree] = unapp match {
case Apply(Apply(unapply, `dummyArg` :: Nil), args2) => assert(args2.nonEmpty); args2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's safer in that case to not have a default argument for unapp.

case Apply(unapply, `dummyArg` :: Nil) => Nil
case Inlined(u, _, _) => unapplyImplicits(u)
}

var argTypes = unapplyArgs(unapplyApp.tpe, unapplyFn, args, tree.pos)
Expand All @@ -981,7 +982,7 @@ trait Applications extends Compatibility { self: Typer with Dynamic =>
List.fill(argTypes.length - args.length)(WildcardType)
}
val unapplyPatterns = (bunchedArgs, argTypes).zipped map (typed(_, _))
val result = assignType(cpy.UnApply(tree)(unapplyFn, unapplyImplicits, unapplyPatterns), ownType)
val result = assignType(cpy.UnApply(tree)(unapplyFn, unapplyImplicits(unapplyApp), unapplyPatterns), ownType)
unapp.println(s"unapply patterns = $unapplyPatterns")
if ((ownType eq selType) || ownType.isError) result
else tryWithClassTag(Typed(result, TypeTree(ownType)), selType)
Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/typer/Inliner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,7 @@ class Inliner(call: tpd.Tree, rhs: tpd.Tree)(implicit ctx: Context) {
paramProxy.get(tree.tpe) match {
case Some(t) if tree.isTerm && t.isSingleton => singleton(t).withPos(tree.pos)
case Some(t) if tree.isType => TypeTree(t).withPos(tree.pos)
case None => tree
case _ => tree
}
case _ => tree
}}
Expand Down
26 changes: 22 additions & 4 deletions compiler/src/dotty/tools/dotc/typer/ReTyper.scala
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import core._
import Contexts._
import Types._
import Symbols._
import StdNames._
import Decorators._
import typer.ProtoTypes._
import ast.{tpd, untpd}
Expand All @@ -24,18 +25,20 @@ import config.Printers.typr
class ReTyper extends Typer {
import tpd._

private def assertTyped(tree: untpd.Tree)(implicit ctx: Context): Unit =
assert(tree.hasType, i"$tree ${tree.getClass} ${tree.uniqueId}")

/** Checks that the given tree has been typed */
protected def promote(tree: untpd.Tree)(implicit ctx: Context): tree.ThisTree[Type] = {
assert(tree.hasType, i"$tree ${tree.getClass} ${tree.uniqueId}")
assertTyped(tree)
tree.withType(tree.typeOpt)
}

override def typedIdent(tree: untpd.Ident, pt: Type)(implicit ctx: Context): Tree =
promote(tree)

override def typedSelect(tree: untpd.Select, pt: Type)(implicit ctx: Context): Tree = {
assert(tree.hasType, tree)
// a qualifier cannot be a pattern
assertTyped(tree)
val qual1 = typed(tree.qualifier, AnySelectionProto)(ctx.retractMode(Mode.Pattern))
untpd.cpy.Select(tree)(qual1, tree.name).withType(tree.typeOpt)
}
Expand All @@ -49,11 +52,22 @@ class ReTyper extends Typer {
override def typedSuper(tree: untpd.Super, pt: Type)(implicit ctx: Context): Tree =
promote(tree)

override def typedTyped(tree: untpd.Typed, pt: Type)(implicit ctx: Context): Tree = {
assertTyped(tree)
val tpt1 = checkSimpleKinded(typedType(tree.tpt))
val expr1 = tree.expr match {
case id: untpd.Ident if (ctx.mode is Mode.Pattern) && untpd.isVarPattern(id) && (id.name == nme.WILDCARD || id.name == nme.WILDCARD_STAR) =>
tree.expr.withType(tpt1.tpe)
case _ => typed(tree.expr)
}
untpd.cpy.Typed(tree)(expr1, tpt1).withType(tree.typeOpt)
}

override def typedTypeTree(tree: untpd.TypeTree, pt: Type)(implicit ctx: Context): TypeTree =
promote(tree)

override def typedBind(tree: untpd.Bind, pt: Type)(implicit ctx: Context): Bind = {
assert(tree.hasType)
assertTyped(tree)
val body1 = typed(tree.body, pt)
untpd.cpy.Bind(tree)(tree.name, body1).withType(tree.typeOpt)
}
Expand All @@ -65,6 +79,10 @@ class ReTyper extends Typer {
untpd.cpy.UnApply(tree)(fun1, implicits1, patterns1).withType(tree.tpe)
}

override def typedUnApply(tree: untpd.Apply, selType: Type)(implicit ctx: Context): Tree = {
typedApply(tree, selType)
}

override def localDummy(cls: ClassSymbol, impl: untpd.Template)(implicit ctx: Context) = impl.symbol

override def retrieveSym(tree: untpd.Tree)(implicit ctx: Context): Symbol = tree.symbol
Expand Down
1 change: 0 additions & 1 deletion compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ trait TypeAssigner {
* - typed child trees it needs to access to cpmpute that type,
* - any further information it needs to access to compute that type.
*/

def assignType(tree: untpd.Ident, tp: Type)(implicit ctx: Context) =
tree.withType(tp)

Expand Down
12 changes: 2 additions & 10 deletions compiler/src/dotty/tools/dotc/typer/Typer.scala
Original file line number Diff line number Diff line change
Expand Up @@ -664,13 +664,7 @@ class Typer extends Namer

def typedBlock(tree: untpd.Block, pt: Type)(implicit ctx: Context) = track("typedBlock") {
val (exprCtx, stats1) = typedBlockStats(tree.stats)
val ept =
if (tree.isInstanceOf[untpd.InfixOpBlock])
// Right-binding infix operations are expanded to InfixBlocks, which may be followed by arguments.
// Example: `(a /: bs)(op)` expands to `{ val x = a; bs./:(x) } (op)` where `{...}` is an InfixBlock.
pt
else pt.notApplied
val expr1 = typedExpr(tree.expr, ept)(exprCtx)
val expr1 = typedExpr(tree.expr, pt.notApplied)(exprCtx)
ensureNoLocalRefs(
cpy.Block(tree)(stats1, expr1).withType(expr1.tpe), pt, localSyms(stats1))
}
Expand Down Expand Up @@ -1001,9 +995,7 @@ class Typer extends Namer
cases mapconserve (typedCase(_, pt, selType, gadtSyms))
}

/** Type a case. Overridden in ReTyper, that's why it's separate from
* typedCases.
*/
/** Type a case. */
def typedCase(tree: untpd.CaseDef, pt: Type, selType: Type, gadtSyms: Set[Symbol])(implicit ctx: Context): CaseDef = track("typedCase") {
val originalCtx = ctx

Expand Down
14 changes: 14 additions & 0 deletions tests/pos/inline-i1773.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
object Test {
implicit class Foo(sc: StringContext) {
object q {
inline def unapply(arg: Any): Option[(Any, Any)] =
Some((sc.parts(0), sc.parts(1)))
}
}

def main(args: Array[String]): Unit = {
val q"class $name extends $parent" = new Object
println(name)
println(parent)
}
}
4 changes: 4 additions & 0 deletions tests/pos/inline-i2570.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
object Test {
inline def sum2(ys: List[Int]): Int = (1 /: ys)(_ + _)
val h1: ((List[Int]) => Int) = sum2
}
5 changes: 5 additions & 0 deletions tests/pos/inline-named-typeargs.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
object t1 {
inline def construct[Elem, Coll[_]](xs: List[Elem]): Coll[Elem] = ???

val xs3 = construct[Coll = List](List(1, 2, 3))
}
8 changes: 8 additions & 0 deletions tests/pos/inline-t2425.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
object Test extends App {
inline def foo[T](bar: T) = {
bar match {
case _ => ()
}
}
foo(Array(1, 2))
}
11 changes: 11 additions & 0 deletions tests/pos/inline-t9232a.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
final class Foo(val value: Int)

object Foo {
inline def unapply(foo: Foo): Some[Int] = Some(foo.value)
}

object Test {
def transformTree(f: Foo): Any = f match {
case Foo(_) => ???
}
}
23 changes: 23 additions & 0 deletions tests/pos/inline-t9232b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
final class Foo(val value: Int)

object Foo {
inline def unapplySeq(foo: Foo): Some[Seq[Int]] = Some(List(foo.value))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be changed to inline def unapply(foo: Foo) after #3747?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}

sealed trait Tree
case class Node1(foo: Foo) extends Tree
case class Node2() extends Tree

object Test {
def transformTree(tree: Tree): Any = tree match {
case Node1(Foo(_: _*)) => ???
}

def transformTree2(tree: Tree): Any = tree match {
case Node1(Foo(1, _: _*)) => ???
}

def transformTree3(tree: Tree): Any = tree match {
case Node1(Foo(x, _: _*)) => ???
}
}