diff options
author | Eugene Burmako <xeno.by@gmail.com> | 2013-02-08 14:08:30 +0100 |
---|---|---|
committer | Eugene Burmako <xeno.by@gmail.com> | 2013-02-08 15:40:16 +0100 |
commit | 57c0e63ba18c5845772d737570342e4054af459f (patch) | |
tree | d912c790cea5a732abf3f1a748096789611ff7ea | |
parent | ce867c74572b51cfcb6ac3e3bfa9dce36cc0b638 (diff) | |
download | scala-57c0e63ba18c5845772d737570342e4054af459f.tar.gz scala-57c0e63ba18c5845772d737570342e4054af459f.tar.bz2 scala-57c0e63ba18c5845772d737570342e4054af459f.zip |
accommodates pull request feedback
https://github.com/scala/scala/pull/2072
-rw-r--r-- | src/compiler/scala/reflect/reify/codegen/GenSymbols.scala | 31 | ||||
-rw-r--r-- | src/compiler/scala/tools/reflect/ToolBoxFactory.scala | 9 | ||||
-rw-r--r-- | src/reflect/scala/reflect/internal/TreeInfo.scala | 15 | ||||
-rw-r--r-- | test/files/run/t6591_7.check | 4 | ||||
-rw-r--r-- | test/files/run/t6591_7.scala | 26 |
5 files changed, 68 insertions, 17 deletions
diff --git a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala index 0d795e3783..47c966ea24 100644 --- a/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala +++ b/src/compiler/scala/reflect/reify/codegen/GenSymbols.scala @@ -1,6 +1,8 @@ package scala.reflect.reify package codegen +import scala.reflect.internal.Flags._ + trait GenSymbols { self: Reifier => @@ -100,8 +102,33 @@ trait GenSymbols { reifyIntoSymtab(binding.symbol) { sym => if (reifyDebug) println("Free term" + (if (sym.isCapturedVariable) " (captured)" else "") + ": " + sym + "(" + sym.accurateKindString + ")") val name = newTermName(nme.REIFY_FREE_PREFIX + sym.name + (if (sym.isType) nme.REIFY_FREE_THIS_SUFFIX else "")) - // Flag <stable> is set here to make reified free value pass stability test during reflective compilation - if (!sym.isMutable) sym setFlag reflect.internal.Flags.STABLE + // We need to note whether the free value being reified is stable or not to guide subsequent reflective compilation. + // Here's why reflection compilation needs our help. + // + // When dealing with a tree, which contain free values, toolboxes extract those and wrap the entire tree in a Function + // having parameters defined for every free values in the tree. For example, evaluating + // + // Ident(setTypeSignature(newFreeTerm("x", 2), <Int>)) + // + // Will generate something like + // + // object wrapper { + // def wrapper(x: () => Int) = { + // x() + // } + // } + // + // Note that free values get transformed into, effectively, by-name parameters. This is done to make sure + // that evaluation order is kept intact. And indeed, we cannot just evaluate all free values at once in order + // to obtain arguments for wrapper.wrapper, because if some of the free values end up being unused during evaluation, + // we might end up doing unnecessary calculations. + // + // So far, so good - we didn't need any flags at all. However, if the code being reified contains path-dependent types, + // we're in trouble, because valid code like `free.T` ends up being transformed into `free.apply().T`, which won't compile. + // + // To overcome this glitch, we note whether a given free term is stable or not (because vars can also end up being free terms). + // Then, if a free term is stable, we tell the compiler to treat `free.apply()` specially and assume that it's stable. + if (!sym.isMutable) sym setFlag STABLE if (sym.isCapturedVariable) { assert(binding.isInstanceOf[Ident], showRaw(binding)) val capturedBinding = referenceCapturedVariable(sym) diff --git a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala index fdc790a920..c05c59d5ff 100644 --- a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala +++ b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala @@ -8,7 +8,7 @@ import scala.tools.nsc.typechecker.Modes import scala.tools.nsc.io.VirtualDirectory import scala.tools.nsc.interpreter.AbstractFileClassLoader import scala.tools.nsc.util.FreshNameCreator -import scala.reflect.internal.Flags +import scala.reflect.internal.Flags._ import scala.reflect.internal.util.{BatchSourceFile, NoSourceFile, NoFile} import java.lang.{Class => jClass} import scala.compat.Platform.EOL @@ -211,12 +211,9 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf => val meth = obj.moduleClass.newMethod(newTermName(wrapperMethodName)) def makeParam(schema: (FreeTermSymbol, TermName)) = { + // see a detailed explanation of the STABLE trick in `GenSymbols.reifyFreeTerm` val (fv, name) = schema - /* Free term symbol `fv` can contain flag <stable> which was set by - * `scala.reflect.reify.codegen.GenSymbols.reifyFreeTerm` method. - * It is recovered here, so value parameter can pass `isExprSafeToInline` - * test in `scala.reflect.internal.TreeInfo`. */ - meth.newValueParameter(name, newFlags = if (fv.hasStableFlag) Flags.STABLE else 0) setInfo appliedType(definitions.FunctionClass(0).tpe, List(fv.tpe.resultType)) + meth.newValueParameter(name, newFlags = if (fv.hasStableFlag) STABLE else 0) setInfo appliedType(definitions.FunctionClass(0).tpe, List(fv.tpe.resultType)) } meth setInfo MethodType(freeTerms.map(makeParam).toList, AnyClass.tpe) minfo.decls enter meth diff --git a/src/reflect/scala/reflect/internal/TreeInfo.scala b/src/reflect/scala/reflect/internal/TreeInfo.scala index f83106aaef..3040486076 100644 --- a/src/reflect/scala/reflect/internal/TreeInfo.scala +++ b/src/reflect/scala/reflect/internal/TreeInfo.scala @@ -89,18 +89,15 @@ abstract class TreeInfo { tree.symbol.isStable && isExprSafeToInline(qual) case TypeApply(fn, _) => isExprSafeToInline(fn) - /* Special case for reified free terms. During reflective compilation, - * reified value recovered flag <stable> from free term and wrapped into a - * Function object, so it can pass stability check here to become a stable - * identifier.*/ case Apply(Select(free @ Ident(_), nme.apply), _) if free.symbol.name endsWith nme.REIFY_FREE_VALUE_SUFFIX => + // see a detailed explanation of this trick in `GenSymbols.reifyFreeTerm` free.symbol.hasStableFlag && isExprSafeToInline(free) case Apply(fn, List()) => - /* Note: After uncurry, field accesses are represented as Apply(getter, Nil), - * so an Apply can also be pure. - * However, before typing, applications of nullary functional values are also - * Apply(function, Nil) trees. To prevent them from being treated as pure, - * we check that the callee is a method. */ + // Note: After uncurry, field accesses are represented as Apply(getter, Nil), + // so an Apply can also be pure. + // However, before typing, applications of nullary functional values are also + // Apply(function, Nil) trees. To prevent them from being treated as pure, + // we check that the callee is a method. fn.symbol.isMethod && !fn.symbol.isLazy && isExprSafeToInline(fn) case Typed(expr, _) => isExprSafeToInline(expr) diff --git a/test/files/run/t6591_7.check b/test/files/run/t6591_7.check new file mode 100644 index 0000000000..e21a3669b6 --- /dev/null +++ b/test/files/run/t6591_7.check @@ -0,0 +1,4 @@ +name = x, stable = true +name = y, stable = true +name = z, stable = false +name = C, stable = true diff --git a/test/files/run/t6591_7.scala b/test/files/run/t6591_7.scala new file mode 100644 index 0000000000..b6c8d399a0 --- /dev/null +++ b/test/files/run/t6591_7.scala @@ -0,0 +1,26 @@ +import scala.reflect.runtime.universe._ +import scala.tools.reflect.Eval + +object Test extends App { + locally { + val x = 2 + def y = 3 + var z = 4 + class C { + var w = 5 + locally { + val expr = reify(x + y + z + w) + // blocked by SI-7103, though it's not the focus of this test + // therefore I'm just commenting out the evaluation + // println(expr.eval) + expr.tree.freeTerms foreach (ft => { + // blocked by SI-7104, though it's not the focus of this test + // therefore I'm just commenting out the call to typeSignature + // println(s"name = ${ft.name}, sig = ${ft.typeSignature}, stable = ${ft.isStable}") + println(s"name = ${ft.name}, stable = ${ft.isStable}") + }) + } + } + new C() + } +}
\ No newline at end of file |