From b97d44b2d813c1bf482b23efb353e4550818700c Mon Sep 17 00:00:00 2001 From: Den Shabalin Date: Sun, 8 Dec 2013 20:18:56 +0100 Subject: SI-8047 change fresh name encoding to avoid owner corruption MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously a following encoding was used to represent fresh names that should be created at runtime of the quasiquote: build.withFreshTermName(prefix1) { name$1 => ... build.withFreshTermName(prefixN) { name$N => tree } ... } It turned out that this encoding causes symbol corruption when tree defines symbols of its own. After being spliced into anonymous functions, the owner chain of those symbols will become corrupted. Now a simpler and probably better performing alternative is used instead: { val name$1 = universe.build.freshTermName(prefix1) ... val name$N = universe.build.freshTermName(prefixN) tree } Here owner stays the same and doesn’t need any adjustment. --- .../scala/tools/reflect/quasiquotes/Reifiers.scala | 29 +++++++++++++------- src/reflect/scala/reflect/api/BuildUtils.scala | 4 +-- .../scala/reflect/internal/BuildUtils.scala | 4 +-- src/reflect/scala/reflect/internal/StdNames.scala | 2 ++ test/files/run/t8047.check | 7 +++++ test/files/run/t8047.scala | 31 ++++++++++++++++++++++ 6 files changed, 64 insertions(+), 13 deletions(-) create mode 100644 test/files/run/t8047.check create mode 100644 test/files/run/t8047.scala diff --git a/src/compiler/scala/tools/reflect/quasiquotes/Reifiers.scala b/src/compiler/scala/tools/reflect/quasiquotes/Reifiers.scala index b28c85cfc2..a6330e95b2 100644 --- a/src/compiler/scala/tools/reflect/quasiquotes/Reifiers.scala +++ b/src/compiler/scala/tools/reflect/quasiquotes/Reifiers.scala @@ -33,8 +33,16 @@ trait Reifiers { self: Quasiquotes => var nameMap = collection.mutable.HashMap.empty[Name, Set[TermName]].withDefault { _ => Set() } /** Wraps expressions into: - * a sequence of nested withFreshTermName/withFreshTypeName calls which are required - * to force regeneration of randomly generated names on every evaluation of quasiquote. + * a block which starts with a sequence of vals that correspond + * to fresh names that has to be created at evaluation of the quasiquote + * and ends with reified tree: + * + * { + * val name$1: universe.TermName = universe.build.freshTermName(prefix1) + * ... + * val name$N: universe.TermName = universe.build.freshTermName(prefixN) + * tree + * } * * Wraps patterns into: * a call into anonymous class' unapply method required by unapply macro expansion: @@ -51,15 +59,18 @@ trait Reifiers { self: Quasiquotes => */ def wrap(tree: Tree) = if (isReifyingExpressions) { - nameMap.foldLeft(tree) { - case (t, (origname, names)) => + val freshdefs = nameMap.iterator.map { + case (origname, names) => assert(names.size == 1) val FreshName(prefix) = origname - val ctor = TermName("withFresh" + (if (origname.isTermName) "TermName" else "TypeName")) - // q"$u.build.$ctor($prefix) { ${names.head} => $t }" - Apply(Apply(Select(Select(u, nme.build), ctor), List(Literal(Constant(prefix)))), - List(Function(List(ValDef(Modifiers(PARAM), names.head, TypeTree(), EmptyTree)), t))) - } + val nameTypeName = if (origname.isTermName) tpnme.TermName else tpnme.TypeName + val freshName = if (origname.isTermName) nme.freshTermName else nme.freshTypeName + // q"val ${names.head}: $u.$nameTypeName = $u.build.$freshName($prefix)" + ValDef(NoMods, names.head, Select(u, nameTypeName), + Apply(Select(Select(u, nme.build), freshName), Literal(Constant(prefix)) :: Nil)) + }.toList + // q"..$freshdefs; $tree" + SyntacticBlock(freshdefs :+ tree) } else { val freevars = holeMap.toList.map { case (name, _) => Ident(name) } val isVarPattern = tree match { case Bind(name, Ident(nme.WILDCARD)) => true case _ => false } diff --git a/src/reflect/scala/reflect/api/BuildUtils.scala b/src/reflect/scala/reflect/api/BuildUtils.scala index cf05aefe72..83521e04f3 100644 --- a/src/reflect/scala/reflect/api/BuildUtils.scala +++ b/src/reflect/scala/reflect/api/BuildUtils.scala @@ -90,9 +90,9 @@ private[reflect] trait BuildUtils { self: Universe => def RefTree(qual: Tree, sym: Symbol): Tree - def withFreshTermName[T](prefix: String)(f: TermName => T): T + def freshTermName(prefix: String): TermName - def withFreshTypeName[T](prefix: String)(f: TypeName => T): T + def freshTypeName(prefix: String): TypeName val ScalaDot: ScalaDotExtractor diff --git a/src/reflect/scala/reflect/internal/BuildUtils.scala b/src/reflect/scala/reflect/internal/BuildUtils.scala index 8fc1869dd2..d701743993 100644 --- a/src/reflect/scala/reflect/internal/BuildUtils.scala +++ b/src/reflect/scala/reflect/internal/BuildUtils.scala @@ -130,9 +130,9 @@ trait BuildUtils { self: SymbolTable => def RefTree(qual: Tree, sym: Symbol) = self.RefTree(qual, sym.name) setSymbol sym - def withFreshTermName[T](prefix: String)(f: TermName => T): T = f(freshTermName(prefix)) + def freshTermName(prefix: String): TermName = self.freshTermName(prefix) - def withFreshTypeName[T](prefix: String)(f: TypeName => T): T = f(freshTypeName(prefix)) + def freshTypeName(prefix: String): TypeName = self.freshTypeName(prefix) protected implicit def fresh: FreshNameCreator = self.currentFreshNameCreator diff --git a/src/reflect/scala/reflect/internal/StdNames.scala b/src/reflect/scala/reflect/internal/StdNames.scala index c26e815df1..d3bf79287b 100644 --- a/src/reflect/scala/reflect/internal/StdNames.scala +++ b/src/reflect/scala/reflect/internal/StdNames.scala @@ -669,6 +669,8 @@ trait StdNames { val find_ : NameType = "find" val flatMap: NameType = "flatMap" val foreach: NameType = "foreach" + val freshTermName: NameType = "freshTermName" + val freshTypeName: NameType = "freshTypeName" val get: NameType = "get" val hashCode_ : NameType = "hashCode" val hash_ : NameType = "hash" diff --git a/test/files/run/t8047.check b/test/files/run/t8047.check new file mode 100644 index 0000000000..a6b83a4a16 --- /dev/null +++ b/test/files/run/t8047.check @@ -0,0 +1,7 @@ +doWhile$1(){ + 1; + if (true) + doWhile$1() + else + () +} diff --git a/test/files/run/t8047.scala b/test/files/run/t8047.scala new file mode 100644 index 0000000000..f5660541e8 --- /dev/null +++ b/test/files/run/t8047.scala @@ -0,0 +1,31 @@ +object Test extends App { + import scala.reflect.runtime.universe._ + // + // x's owner is outer Test scope. Previosly the quasiquote expansion + // looked like: + // + // object Test { + // build.withFreshTermName("doWhile")(n => + // LabelDef(n, List(), + // Block( + // List({ val x = 1; x }), + // If(Literal(Constant(true)), Apply(Ident(n), List()), Literal(Constant(()))))) + // } + // + // Here the proper owner is anonymous function, not the Test. Hence + // symbol corruption. In new encoding this is represented as: + // + // object Test { + // { + // val n = build.freshTermName("doWhile") + // LabelDef(n, List(), + // Block( + // List({ val x = 1; x }), + // If(Literal(Constant(true)), Apply(Ident(n), List()), Literal(Constant(())))) + // } + // } + // + // Owner stays the same and life is good again. + // + println(q"do ${ val x = 1; x } while(true)") +} -- cgit v1.2.3