From 7187197f3d338ad6523bb56b16f1f3168e5bd867 Mon Sep 17 00:00:00 2001 From: Nicolas Stucki Date: Fri, 17 Aug 2018 18:14:19 +0200 Subject: [PATCH 1/7] Fix #1023: Test that TypeTrees are not in an unexpected tree --- .../tools/dotc/transform/TreeChecker.scala | 100 ++++++++++++++++++ 1 file changed, 100 insertions(+) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index bd3f4f44984b..48db34a98393 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -306,6 +306,7 @@ class TreeChecker extends Phase with SymTransformer { sym.isEffectivelyErased && sym.is(Private) && !sym.initial.is(Private) override def typed(tree: untpd.Tree, pt: Type = WildcardType)(using Context): Tree = { + checkTreeNode(tree) val tpdTree = super.typed(tree, pt) Typer.assertPositioned(tree) if (ctx.erasedTypes) @@ -646,4 +647,103 @@ object TreeChecker { tp } }.apply(tp0) + + private val PrivateErased = allOf(Private, Erased) + + /** Check that the tree only contains legal children trees */ + def checkTreeNode(tree: untpd.Tree)(implicit ctx: Context): Unit = { + def assertNotTypeTree(t: untpd.Tree): Unit = assert(!t.isInstanceOf[untpd.TypeTree], tree) + + // TODO add many more sanity checks + tree match { +// case Ident(name) => + case Select(qualifier, name) => + // FIXME this assertion fails only in tests/run/scala2trait-lazyval.scala + // assertNotTypeTree(qual) + // case This(qual) => +// +// case Super(qual, mix) => + case Apply(fun, args) => + assertNotTypeTree(fun) + for (arg <- args) { + assertNotTypeTree(arg) + } + case TypeApply(fun, args) => + assertNotTypeTree(fun) + // case Literal(const) => +// +// case New(tpt) => + case Typed(expr, tpt) => + assertNotTypeTree(expr) + // case NamedArg(name, arg) => + case Assign(lhs, rhs) => + assertNotTypeTree(lhs) + assertNotTypeTree(rhs) + case Block(stats, expr) => + for (stat <- stats) { + assertNotTypeTree(stat) + } + assertNotTypeTree(expr) + case If(cond, thenp, elsep) => + assertNotTypeTree(cond) + assertNotTypeTree(thenp) + assertNotTypeTree(elsep) + // case Closure(env, meth, tpt) => + case Match(selector, cases) => + assertNotTypeTree(selector) + case CaseDef(pat, guard, body) => + assertNotTypeTree(guard) + assertNotTypeTree(body) + case Return(expr, from) => + assertNotTypeTree(expr) + case Try(block, handler, finalizer) => + assertNotTypeTree(block) + assertNotTypeTree(finalizer) + case SeqLiteral(elems, elemtpt) => + for (elem <- elems) { + assertNotTypeTree(elem) + } + case Inlined(call, bindings, expansion) => + assertNotTypeTree(call) + // case TypeTree() => +// +// case SingletonTypeTree(ref) => +// +// case AndTypeTree(left, right) => +// +// case OrTypeTree(left, right) => +// +// case RefinedTypeTree(tpt, refinements) => +// +// case AppliedTypeTree(tpt, args) => +// +// case LambdaTypeTree(tparams, body) => +// +// case ByNameTypeTree(result) => +// +// case TypeBoundsTree(lo, hi) => +// +// case Bind(name, body) => +// +// case Alternative(trees) => +// +// case UnApply(fun, implicits, patterns) => + case tree @ ValDef(name, tpt, _) => + assertNotTypeTree(tree.rhs) + case tree @ DefDef(name, tparams, vparamss, tpt, _) => + assertNotTypeTree(tree.rhs) + // case TypeDef(name, rhs) => +// +// case tree @ Template(constr, parents, self, _) => +// +// case Import(expr, selectors) => +// +// case PackageDef(pid, stats) => +// +// case Annotated(arg, annot) => +// +// case Thicket(ts) => + case _ => + } + } } From 543b1338a3d2500afca75724d98f000bd5a37b7a Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 22 Mar 2022 22:17:13 +0000 Subject: [PATCH 2/7] Reuse TreeTraverser to traverse all tree children, with explicit exceptions --- compiler/src/dotty/tools/dotc/ast/Trees.scala | 1 + .../tools/dotc/transform/TreeChecker.scala | 115 +++--------------- 2 files changed, 18 insertions(+), 98 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/Trees.scala b/compiler/src/dotty/tools/dotc/ast/Trees.scala index f65154984f50..34d3a0bc1ca8 100644 --- a/compiler/src/dotty/tools/dotc/ast/Trees.scala +++ b/compiler/src/dotty/tools/dotc/ast/Trees.scala @@ -1638,6 +1638,7 @@ object Trees { abstract class TreeTraverser extends TreeAccumulator[Unit] { def traverse(tree: Tree)(using Context): Unit + def traverse(trees: List[Tree])(using Context) = apply((), trees) def apply(x: Unit, tree: Tree)(using Context): Unit = traverse(tree) protected def traverseChildren(tree: Tree)(using Context): Unit = foldOver((), tree) } diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 48db34a98393..ac743116bd48 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -306,7 +306,7 @@ class TreeChecker extends Phase with SymTransformer { sym.isEffectivelyErased && sym.is(Private) && !sym.initial.is(Private) override def typed(tree: untpd.Tree, pt: Type = WildcardType)(using Context): Tree = { - checkTreeNode(tree) + TreeNodeChecker.run(tree) val tpdTree = super.typed(tree, pt) Typer.assertPositioned(tree) if (ctx.erasedTypes) @@ -648,102 +648,21 @@ object TreeChecker { } }.apply(tp0) - private val PrivateErased = allOf(Private, Erased) - /** Check that the tree only contains legal children trees */ - def checkTreeNode(tree: untpd.Tree)(implicit ctx: Context): Unit = { - def assertNotTypeTree(t: untpd.Tree): Unit = assert(!t.isInstanceOf[untpd.TypeTree], tree) - - // TODO add many more sanity checks - tree match { -// case Ident(name) => - case Select(qualifier, name) => - // FIXME this assertion fails only in tests/run/scala2trait-lazyval.scala - // assertNotTypeTree(qual) - // case This(qual) => -// -// case Super(qual, mix) => - case Apply(fun, args) => - assertNotTypeTree(fun) - for (arg <- args) { - assertNotTypeTree(arg) - } - case TypeApply(fun, args) => - assertNotTypeTree(fun) - // case Literal(const) => -// -// case New(tpt) => - case Typed(expr, tpt) => - assertNotTypeTree(expr) - // case NamedArg(name, arg) => - case Assign(lhs, rhs) => - assertNotTypeTree(lhs) - assertNotTypeTree(rhs) - case Block(stats, expr) => - for (stat <- stats) { - assertNotTypeTree(stat) - } - assertNotTypeTree(expr) - case If(cond, thenp, elsep) => - assertNotTypeTree(cond) - assertNotTypeTree(thenp) - assertNotTypeTree(elsep) - // case Closure(env, meth, tpt) => - case Match(selector, cases) => - assertNotTypeTree(selector) - case CaseDef(pat, guard, body) => - assertNotTypeTree(guard) - assertNotTypeTree(body) - case Return(expr, from) => - assertNotTypeTree(expr) - case Try(block, handler, finalizer) => - assertNotTypeTree(block) - assertNotTypeTree(finalizer) - case SeqLiteral(elems, elemtpt) => - for (elem <- elems) { - assertNotTypeTree(elem) - } - case Inlined(call, bindings, expansion) => - assertNotTypeTree(call) - // case TypeTree() => -// -// case SingletonTypeTree(ref) => -// -// case AndTypeTree(left, right) => -// -// case OrTypeTree(left, right) => -// -// case RefinedTypeTree(tpt, refinements) => -// -// case AppliedTypeTree(tpt, args) => -// -// case LambdaTypeTree(tparams, body) => -// -// case ByNameTypeTree(result) => -// -// case TypeBoundsTree(lo, hi) => -// -// case Bind(name, body) => -// -// case Alternative(trees) => -// -// case UnApply(fun, implicits, patterns) => - case tree @ ValDef(name, tpt, _) => - assertNotTypeTree(tree.rhs) - case tree @ DefDef(name, tparams, vparamss, tpt, _) => - assertNotTypeTree(tree.rhs) - // case TypeDef(name, rhs) => -// -// case tree @ Template(constr, parents, self, _) => -// -// case Import(expr, selectors) => -// -// case PackageDef(pid, stats) => -// -// case Annotated(arg, annot) => -// -// case Thicket(ts) => - case _ => - } - } + object TreeNodeChecker extends untpd.TreeTraverser: + import untpd._ + def run(tree: Tree)(using Context) = if !tree.isInstanceOf[TypeTree] then traverse(tree) + def traverse(tree: Tree)(using Context) = tree match + case t: TypeTree => assert(assertion = false, t) + case t @ TypeApply(fun, _targs) => traverse(fun) + case t @ New(_tpt) => + case t @ Typed(expr, _tpt) => traverse(expr) + case t @ Closure(env, meth, _tpt) => traverse(env); traverse(meth) + case t @ SeqLiteral(elems, _elemtpt) => traverse(elems) + case t @ ValDef(_, _tpt, _) => traverse(t.rhs) + case t @ DefDef(_, paramss, _tpt, _) => for params <- paramss do traverse(params); traverse(t.rhs) + case t @ TypeDef(_, _rhs) => + case t @ Template(constr, _parents, self, _) => traverse(constr); traverse(self); traverse(t.body) + case t => traverseChildren(t) + end traverse } From e6e456533d7f97fd97cad2a9dbc3c505884654e4 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Tue, 22 Mar 2022 22:42:22 +0000 Subject: [PATCH 3/7] Move TreeNodeChecker running out of typed --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index ac743116bd48..5313e3f53d0d 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -130,6 +130,7 @@ class TreeChecker extends Phase with SymTransformer { assert(ctx.typerState.constraint.domainLambdas.isEmpty, i"non-empty constraint at end of $fusedPhase: ${ctx.typerState.constraint}, ownedVars = ${ctx.typerState.ownedVars.toList}%, %") assertSelectWrapsNew(ctx.compilationUnit.tpdTree) + TreeNodeChecker.run(ctx.compilationUnit.tpdTree) } val checkingCtx = ctx @@ -306,7 +307,6 @@ class TreeChecker extends Phase with SymTransformer { sym.isEffectivelyErased && sym.is(Private) && !sym.initial.is(Private) override def typed(tree: untpd.Tree, pt: Type = WildcardType)(using Context): Tree = { - TreeNodeChecker.run(tree) val tpdTree = super.typed(tree, pt) Typer.assertPositioned(tree) if (ctx.erasedTypes) From e133e3ff0f0898149dc870643a1c77304583d884 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 7 Apr 2022 15:32:39 +0100 Subject: [PATCH 4/7] Call TreeNodeChecker.traverse directly Now that we're not running this on every `typed` invocation, the compilation unit tree isn't going to be a top-level TypeTree. --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 5313e3f53d0d..2b8bb9e34470 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -130,7 +130,7 @@ class TreeChecker extends Phase with SymTransformer { assert(ctx.typerState.constraint.domainLambdas.isEmpty, i"non-empty constraint at end of $fusedPhase: ${ctx.typerState.constraint}, ownedVars = ${ctx.typerState.ownedVars.toList}%, %") assertSelectWrapsNew(ctx.compilationUnit.tpdTree) - TreeNodeChecker.run(ctx.compilationUnit.tpdTree) + TreeNodeChecker.traverse(ctx.compilationUnit.tpdTree) } val checkingCtx = ctx @@ -651,7 +651,6 @@ object TreeChecker { /** Check that the tree only contains legal children trees */ object TreeNodeChecker extends untpd.TreeTraverser: import untpd._ - def run(tree: Tree)(using Context) = if !tree.isInstanceOf[TypeTree] then traverse(tree) def traverse(tree: Tree)(using Context) = tree match case t: TypeTree => assert(assertion = false, t) case t @ TypeApply(fun, _targs) => traverse(fun) From f7ab15b0e848b667823541a2ac949f31103f8d14 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 7 Apr 2022 15:33:01 +0100 Subject: [PATCH 5/7] Give TreeNodeChecker a slightly better error message --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 2b8bb9e34470..8326c386d354 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -652,7 +652,7 @@ object TreeChecker { object TreeNodeChecker extends untpd.TreeTraverser: import untpd._ def traverse(tree: Tree)(using Context) = tree match - case t: TypeTree => assert(assertion = false, t) + case t: TypeTree => assert(assertion = false, i"TypeTree not expected: $t") case t @ TypeApply(fun, _targs) => traverse(fun) case t @ New(_tpt) => case t @ Typed(expr, _tpt) => traverse(expr) From 7af2560854fe66e403ffe553ad3af9efae984cf0 Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Thu, 7 Apr 2022 15:33:46 +0100 Subject: [PATCH 6/7] Check template parents in TreeNodeChecker. I'm not sure what I had hit that lead me to this exclusion, but I'm not hitting now.. --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index 8326c386d354..be0a8420cdfd 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -661,7 +661,7 @@ object TreeChecker { case t @ ValDef(_, _tpt, _) => traverse(t.rhs) case t @ DefDef(_, paramss, _tpt, _) => for params <- paramss do traverse(params); traverse(t.rhs) case t @ TypeDef(_, _rhs) => - case t @ Template(constr, _parents, self, _) => traverse(constr); traverse(self); traverse(t.body) + case t @ Template(constr, parents, self, _) => traverse(constr); traverse(parents); traverse(self); traverse(t.body) case t => traverseChildren(t) end traverse } From 8f5ee23c4debdd502ba25a6d9c760c46cb49490f Mon Sep 17 00:00:00 2001 From: Dale Wijnand Date: Wed, 20 Apr 2022 08:48:52 +0100 Subject: [PATCH 7/7] Expand on TreeNodeChecker's docs --- compiler/src/dotty/tools/dotc/transform/TreeChecker.scala | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala index be0a8420cdfd..b89af15488f2 100644 --- a/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala +++ b/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala @@ -648,7 +648,11 @@ object TreeChecker { } }.apply(tp0) - /** Check that the tree only contains legal children trees */ + /** Run some additional checks on the nodes of the trees. Specifically: + * + * - TypeTree can only appear in TypeApply args, New, Typed tpt, Closure + * tpt, SeqLiteral elemtpt, ValDef tpt, DefDef tpt, and TypeDef rhs. + */ object TreeNodeChecker extends untpd.TreeTraverser: import untpd._ def traverse(tree: Tree)(using Context) = tree match