Skip to content
Browse files

accommodates pull request feedback

  • Loading branch information...
1 parent ce867c7 commit 57c0e63ba18c5845772d737570342e4054af459f @xeno-by xeno-by committed Feb 8, 2013
View
31 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)
View
9 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
View
15 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)
View
4 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
View
26 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()
+ }
+}

0 comments on commit 57c0e63

Please sign in to comment.
Something went wrong with that request. Please try again.