From d27adbe6db7930533a0a9a6f3a27ed7391c5eddc Mon Sep 17 00:00:00 2001 From: Miles Sabin Date: Mon, 21 Jan 2019 16:40:52 +0000 Subject: [PATCH 1/3] Correct owner chains when mixing byname implicits and inlining When constructing the dictionary for a byname implicit resolution we have to ensure that the owners of any definitions which have been hoisted into the dictionary are correct. Prior to this PR this was only done fully for implicit expansions which didn't involve inlining. We fix that here by ensuring that all definitions under a given dictionary entry are owned directly or indirectly by that dictionary entry. Fixes #5766. --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 22 +++++++++++++ .../dotty/tools/dotc/typer/Implicits.scala | 2 +- tests/pos/i5766.scala | 32 +++++++++++++++++++ 3 files changed, 55 insertions(+), 1 deletion(-) create mode 100644 tests/pos/i5766.scala diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 0e577a5ac774..a11a18076724 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -730,6 +730,28 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { if (from == to) tree else loop(from, Nil, to :: Nil) } + /** + * Set the owner of every definition in this tree which is not itself contained in this + * tree to be `newowner` + */ + def changeNonLocalOwners(newowner: Symbol)(implicit ctx: Context): Tree = { + val localOwners = mutable.HashSet[Symbol]() + val traverser = new TreeTraverser { + + def traverse(tree: Tree)(implicit ctx: Context): Unit = { + tree match { + case _: DefTree => if(tree.symbol ne NoSymbol) localOwners += tree.symbol + case _ => + } + traverseChildren(tree) + } + } + traverser.traverse(tree) + val nonLocalOwners = localOwners.filter(sym => !localOwners.contains(sym.owner)).toList.map(_.owner) + val newOwners = nonLocalOwners.map(_ => newowner) + new TreeTypeMap(oldOwners = nonLocalOwners, newOwners = newOwners).apply(tree) + } + /** After phase `trans`, set the owner of every definition in this tree that was formerly * owner by `from` to `to`. */ diff --git a/compiler/src/dotty/tools/dotc/typer/Implicits.scala b/compiler/src/dotty/tools/dotc/typer/Implicits.scala index 85430730c76e..6bb41b258f91 100644 --- a/compiler/src/dotty/tools/dotc/typer/Implicits.scala +++ b/compiler/src/dotty/tools/dotc/typer/Implicits.scala @@ -1577,7 +1577,7 @@ final class SearchRoot extends SearchHistory { val nrhss = rhss.map(rhsMap(_)) val vdefs = (nsyms zip nrhss) map { - case (nsym, nrhs) => ValDef(nsym.asTerm, nrhs) + case (nsym, nrhs) => ValDef(nsym.asTerm, nrhs.changeNonLocalOwners(nsym)) } val constr = ctx.newConstructor(classSym, Synthetic, Nil, Nil).entered diff --git a/tests/pos/i5766.scala b/tests/pos/i5766.scala new file mode 100644 index 000000000000..a33468e8a4fe --- /dev/null +++ b/tests/pos/i5766.scala @@ -0,0 +1,32 @@ +object Test1 { + trait Foo { def next: Foo } + + inline implicit def foo(implicit loop: => Foo): Foo = new Foo { def next = loop } + + def summon(implicit f: Foo) = () + summon +} + +object Test2 { + trait Foo { def next: Bar } + trait Bar { def next: Foo } + + implicit def foo(implicit loop: => Bar): Foo = new Foo { def next = loop } + inline implicit def bar(implicit loop: Foo): Bar = new Bar { def next = loop } + + def summon(implicit f: Foo) = () + summon +} + +object Test3 { + trait Foo { def next: Bar } + trait Bar { def next: Baz } + trait Baz { def next: Foo } + + implicit def foo(implicit loop: Bar): Foo = new Foo { def next = loop } + inline implicit def bar(implicit loop: Baz): Bar = new Bar { def next = loop } + inline implicit def baz(implicit loop: => Foo): Baz = new Baz { def next = loop } + + def summon(implicit f: Foo) = () + summon +} From cfaf6f0b7b513d5b310d7cfabaac7eb9300a44bb Mon Sep 17 00:00:00 2001 From: Miles Sabin Date: Wed, 23 Jan 2019 12:09:56 +0000 Subject: [PATCH 2/3] Don't visit DefTree subtrees in changeNonLocalOwners --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index a11a18076724..296bcd36b3cd 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -741,9 +741,8 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { def traverse(tree: Tree)(implicit ctx: Context): Unit = { tree match { case _: DefTree => if(tree.symbol ne NoSymbol) localOwners += tree.symbol - case _ => + case _ => traverseChildren(tree) } - traverseChildren(tree) } } traverser.traverse(tree) From 98a4f14f76f871bbeafdbf859e0c92dc33cda0a7 Mon Sep 17 00:00:00 2001 From: Miles Sabin Date: Thu, 24 Jan 2019 11:48:44 +0000 Subject: [PATCH 3/3] Simplifications from review --- compiler/src/dotty/tools/dotc/ast/tpd.scala | 26 ++++++++++----------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/compiler/src/dotty/tools/dotc/ast/tpd.scala b/compiler/src/dotty/tools/dotc/ast/tpd.scala index 296bcd36b3cd..491d71e37dd0 100644 --- a/compiler/src/dotty/tools/dotc/ast/tpd.scala +++ b/compiler/src/dotty/tools/dotc/ast/tpd.scala @@ -10,7 +10,7 @@ import core._ import util.Spans._, Types._, Contexts._, Constants._, Names._, Flags._, NameOps._ import Symbols._, StdNames._, Annotations._, Trees._, Symbols._ import Decorators._, DenotTransformers._ -import collection.mutable +import collection.{immutable, mutable} import util.{Property, SourceFile, NoSource} import NameKinds.{TempResultName, OuterSelectName} import typer.ConstFold @@ -734,21 +734,19 @@ object tpd extends Trees.Instance[Type] with TypedTreeInfo { * Set the owner of every definition in this tree which is not itself contained in this * tree to be `newowner` */ - def changeNonLocalOwners(newowner: Symbol)(implicit ctx: Context): Tree = { - val localOwners = mutable.HashSet[Symbol]() - val traverser = new TreeTraverser { - - def traverse(tree: Tree)(implicit ctx: Context): Unit = { - tree match { - case _: DefTree => if(tree.symbol ne NoSymbol) localOwners += tree.symbol - case _ => traverseChildren(tree) - } + def changeNonLocalOwners(newOwner: Symbol)(implicit ctx: Context): Tree = { + val ownerAcc = new TreeAccumulator[immutable.Set[Symbol]] { + def apply(ss: immutable.Set[Symbol], tree: Tree)(implicit ctx: Context) = tree match { + case tree: DefTree => + if (tree.symbol.exists) ss + tree.symbol.owner + else ss + case _ => + foldOver(ss, tree) } } - traverser.traverse(tree) - val nonLocalOwners = localOwners.filter(sym => !localOwners.contains(sym.owner)).toList.map(_.owner) - val newOwners = nonLocalOwners.map(_ => newowner) - new TreeTypeMap(oldOwners = nonLocalOwners, newOwners = newOwners).apply(tree) + val owners = ownerAcc(immutable.Set.empty[Symbol], tree).toList + val newOwners = List.fill(owners.size)(newOwner) + new TreeTypeMap(oldOwners = owners, newOwners = newOwners).apply(tree) } /** After phase `trans`, set the owner of every definition in this tree that was formerly