[nomaster] typecheck(q"class C") no longer crashes

MemberDefs alone can't be typechecked as is, because namer only names contents of PackageDefs, Templates and Blocks. And, if not named, a tree can't be typed. This commit solves this problem by wrapping typecheckees in a trivial block and then unwrapping the result when it returns back from the typechecker. (cherry picked from commit 609047b)
scala · Mar 23, 2014 · 3314d76 · 3314d76
1 parent b66a396
commit 3314d76
Show file tree

Hide file tree

Showing 6 changed files with 100 additions and 62 deletions.
diff --git a/src/compiler/scala/reflect/macros/runtime/Typers.scala b/src/compiler/scala/reflect/macros/runtime/Typers.scala
@@ -14,23 +14,19 @@ trait Typers {
   def typeCheck(tree: Tree, pt: Type = universe.WildcardType, silent: Boolean = false, withImplicitViewsDisabled: Boolean = false, withMacrosDisabled: Boolean = false): Tree = {
     macroLogVerbose("typechecking %s with expected type %s, implicit views = %s, macros = %s".format(tree, pt, !withImplicitViewsDisabled, !withMacrosDisabled))
     val context = callsiteTyper.context
-    val wrapper1 = if (!withImplicitViewsDisabled) (context.withImplicitsEnabled[Tree] _) else (context.withImplicitsDisabled[Tree] _)
-    val wrapper2 = if (!withMacrosDisabled) (context.withMacrosEnabled[Tree] _) else (context.withMacrosDisabled[Tree] _)
-    def wrapper (tree: => Tree) = wrapper1(wrapper2(tree))
-    // if you get a "silent mode is not available past typer" here
-    // don't rush to change the typecheck not to use the silent method when the silent parameter is false
-    // typechecking uses silent anyways (e.g. in typedSelect), so you'll only waste your time
-    // I'd advise fixing the root cause: finding why the context is not set to report errors
-    // (also see reflect.runtime.ToolBoxes.typeCheckExpr for a workaround that might work for you)
-    wrapper(callsiteTyper.silent(_.typed(tree, universe.analyzer.EXPRmode, pt), reportAmbiguousErrors = false) match {
+    val withImplicitFlag = if (!withImplicitViewsDisabled) (context.withImplicitsEnabled[Tree] _) else (context.withImplicitsDisabled[Tree] _)
+    val withMacroFlag = if (!withMacrosDisabled) (context.withMacrosEnabled[Tree] _) else (context.withMacrosDisabled[Tree] _)
+    def withContext(tree: => Tree) = withImplicitFlag(withMacroFlag(tree))
+    def typecheckInternal(tree: Tree) = callsiteTyper.silent(_.typed(tree, universe.analyzer.EXPRmode, pt), reportAmbiguousErrors = false)
+    universe.wrappingIntoTerm(tree)(wrappedTree => withContext(typecheckInternal(wrappedTree) match {
       case universe.analyzer.SilentResultValue(result) =>
         macroLogVerbose(result)
         result
       case error @ universe.analyzer.SilentTypeError(_) =>
         macroLogVerbose(error.err.errMsg)
         if (!silent) throw new TypecheckException(error.err.errPos, error.err.errMsg)
         universe.EmptyTree
-    })
+    }))
   }
 
   def inferImplicitValue(pt: Type, silent: Boolean = true, withMacrosDisabled: Boolean = false, pos: Position = enclosingPosition): Tree = {

diff --git a/src/compiler/scala/tools/reflect/ToolBoxFactory.scala b/src/compiler/scala/tools/reflect/ToolBoxFactory.scala
@@ -63,7 +63,7 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
         try body
         finally cleanupCaches()
 
-      def verify(expr: Tree): Unit = {
+      def verify(expr: Tree): Tree = {
         // Previously toolboxes used to typecheck their inputs before compiling.
         // Actually, the initial demo by Martin first typechecked the reified tree,
         // then ran it, which typechecked it again, and only then launched the
@@ -84,14 +84,8 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
           msg += "if you have troubles tracking free type variables, consider using -Xlog-free-types"
           throw ToolBoxError(msg)
         }
-      }
-
-      def wrapIntoTerm(tree: Tree): Tree =
-        if (!tree.isTerm) Block(List(tree), Literal(Constant(()))) else tree
 
-      def unwrapFromTerm(tree: Tree): Tree = tree match {
-        case Block(List(tree), Literal(Constant(()))) => tree
-        case tree => tree
+        expr
       }
 
       def extractFreeTerms(expr0: Tree, wrapFreeTermRefs: Boolean): (Tree, scala.collection.mutable.LinkedHashMap[FreeTermSymbol, TermName]) = {
@@ -121,50 +115,51 @@ abstract class ToolBoxFactory[U <: JavaUniverse](val u: U) { factorySelf =>
       }
 
       def transformDuringTyper(expr0: Tree, withImplicitViewsDisabled: Boolean, withMacrosDisabled: Boolean)(transform: (analyzer.Typer, Tree) => Tree): Tree = {
-        verify(expr0)
-
-        // need to wrap the expr, because otherwise you won't be able to typecheck macros against something that contains free vars
-        var (expr, freeTerms) = extractFreeTerms(expr0, wrapFreeTermRefs = false)
-        val dummies = freeTerms.map{ case (freeTerm, name) => ValDef(NoMods, name, TypeTree(freeTerm.info), Select(Ident(PredefModule), newTermName("$qmark$qmark$qmark"))) }.toList
-        expr = Block(dummies, wrapIntoTerm(expr))
-
-        // [Eugene] how can we implement that?
-        // !!! Why is this is in the empty package? If it's only to make
-        // it inaccessible then please put it somewhere designed for that
-        // rather than polluting the empty package with synthetics.
-        val ownerClass    = rootMirror.EmptyPackageClass.newClassSymbol(newTypeName("<expression-owner>"))
-        build.setTypeSignature(ownerClass, ClassInfoType(List(ObjectClass.tpe), newScope, ownerClass))
-        val owner         = ownerClass.newLocalDummy(expr.pos)
-        var currentTyper  = analyzer.newTyper(analyzer.rootContext(NoCompilationUnit, EmptyTree).make(expr, owner))
-        val wrapper1      = if (!withImplicitViewsDisabled) (currentTyper.context.withImplicitsEnabled[Tree] _) else (currentTyper.context.withImplicitsDisabled[Tree] _)
-        val wrapper2      = if (!withMacrosDisabled) (currentTyper.context.withMacrosEnabled[Tree] _) else (currentTyper.context.withMacrosDisabled[Tree] _)
-        def wrapper       (tree: => Tree) = wrapper1(wrapper2(tree))
-
-        val run = new Run
-        run.symSource(ownerClass) = NoAbstractFile // need to set file to something different from null, so that currentRun.defines works
-        phase = run.typerPhase // need to set a phase to something <= typerPhase, otherwise implicits in typedSelect will be disabled
-        currentTyper.context.setReportErrors() // need to manually set context mode, otherwise typer.silent will throw exceptions
-        reporter.reset()
-
-        val expr1 = wrapper(transform(currentTyper, expr))
-        var (dummies1, unwrapped) = expr1 match {
-          case Block(dummies, unwrapped) => (dummies, unwrapped)
-          case unwrapped => (Nil, unwrapped)
-        }
-        var invertedIndex = freeTerms map (_.swap)
-        // todo. also fixup singleton types
-        unwrapped = new Transformer {
-          override def transform(tree: Tree): Tree =
-            tree match {
-              case Ident(name) if invertedIndex contains name =>
-                Ident(invertedIndex(name)) setType tree.tpe
-              case _ =>
-                super.transform(tree)
-            }
-        }.transform(unwrapped)
-        new TreeTypeSubstituter(dummies1 map (_.symbol), dummies1 map (dummy => SingleType(NoPrefix, invertedIndex(dummy.symbol.name)))).traverse(unwrapped)
-        unwrapped = if (expr0.isTerm) unwrapped else unwrapFromTerm(unwrapped)
-        unwrapped
+        wrappingIntoTerm(verify(expr0))(expr1 => {
+          // need to wrap the expr, because otherwise you won't be able to typecheck macros against something that contains free vars
+          val exprAndFreeTerms = extractFreeTerms(expr1, wrapFreeTermRefs = false)
+          var expr2 = exprAndFreeTerms._1
+          val freeTerms = exprAndFreeTerms._2
+          val dummies = freeTerms.map{ case (freeTerm, name) => ValDef(NoMods, name, TypeTree(freeTerm.info), Select(Ident(PredefModule), newTermName("$qmark$qmark$qmark"))) }.toList
+          expr2 = Block(dummies, expr2)
+
+          // [Eugene] how can we implement that?
+          // !!! Why is this is in the empty package? If it's only to make
+          // it inaccessible then please put it somewhere designed for that
+          // rather than polluting the empty package with synthetics.
+          val ownerClass    = rootMirror.EmptyPackageClass.newClassSymbol(newTypeName("<expression-owner>"))
+          build.setTypeSignature(ownerClass, ClassInfoType(List(ObjectClass.tpe), newScope, ownerClass))
+          val owner         = ownerClass.newLocalDummy(expr2.pos)
+          var currentTyper  = analyzer.newTyper(analyzer.rootContext(NoCompilationUnit, EmptyTree).make(expr2, owner))
+          val wrapper1      = if (!withImplicitViewsDisabled) (currentTyper.context.withImplicitsEnabled[Tree] _) else (currentTyper.context.withImplicitsDisabled[Tree] _)
+          val wrapper2      = if (!withMacrosDisabled) (currentTyper.context.withMacrosEnabled[Tree] _) else (currentTyper.context.withMacrosDisabled[Tree] _)
+          def wrapper       (tree: => Tree) = wrapper1(wrapper2(tree))
+
+          val run = new Run
+          run.symSource(ownerClass) = NoAbstractFile // need to set file to something different from null, so that currentRun.defines works
+          phase = run.typerPhase // need to set a phase to something <= typerPhase, otherwise implicits in typedSelect will be disabled
+          currentTyper.context.setReportErrors() // need to manually set context mode, otherwise typer.silent will throw exceptions
+          reporter.reset()
+
+          val expr3 = wrapper(transform(currentTyper, expr2))
+          var (dummies1, result) = expr3 match {
+            case Block(dummies, result) => (dummies, result)
+            case result => (Nil, result)
+          }
+          var invertedIndex = freeTerms map (_.swap)
+          // todo. also fixup singleton types
+          result = new Transformer {
+            override def transform(tree: Tree): Tree =
+              tree match {
+                case Ident(name) if invertedIndex contains name =>
+                  Ident(invertedIndex(name)) setType tree.tpe
+                case _ =>
+                  super.transform(tree)
+              }
+          }.transform(result)
+          new TreeTypeSubstituter(dummies1 map (_.symbol), dummies1 map (dummy => SingleType(NoPrefix, invertedIndex(dummy.symbol.name)))).traverse(result)
+          result
+        })
       }
 
       def typeCheck(expr: Tree, pt: Type, silent: Boolean, withImplicitViewsDisabled: Boolean, withMacrosDisabled: Boolean): Tree =

diff --git a/src/reflect/scala/reflect/internal/Trees.scala b/src/reflect/scala/reflect/internal/Trees.scala
@@ -1538,6 +1538,21 @@ trait Trees extends api.Trees { self: SymbolTable =>
 
   def duplicateAndKeepPositions(tree: Tree) = new Duplicator(focusPositions = false) transform tree
 
+  def wrapIntoTerm(tree: Tree): Tree = {
+    if (!tree.isTerm) Block(List(tree), Literal(Constant(()))) else tree
+  }
+
+  // this is necessary to avoid crashes like https://github.com/scalamacros/paradise/issues/1
+  // when someone tries to c.typecheck a naked MemberDef
+  def wrappingIntoTerm(tree0: Tree)(op: Tree => Tree): Tree = {
+    val neededWrapping = !tree0.isTerm
+    val tree1 = wrapIntoTerm(tree0)
+    op(tree1) match {
+      case Block(tree2 :: Nil, Literal(Constant(()))) if neededWrapping => tree2
+      case tree2 => tree2
+    }
+  }
+
   // ------ copiers -------------------------------------------
 
   def copyDefDef(tree: Tree)(

diff --git a/test/files/run/typecheck.check b/test/files/run/typecheck.check
diff --git a/test/files/run/typecheck/Macros_1.scala b/test/files/run/typecheck/Macros_1.scala
@@ -0,0 +1,17 @@
+import scala.reflect.macros.Context
+import scala.language.experimental.macros
+
+object Macros {
+  def impl(c: Context) = {
+    import c.universe._
+    val classDef = ClassDef(
+      Modifiers(), newTypeName("C"), List(),
+      Template(
+        List(Select(Ident(newTermName("scala")), newTypeName("AnyRef"))), emptyValDef,
+        List(DefDef(Modifiers(), nme.CONSTRUCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(tpnme.EMPTY), tpnme.EMPTY), nme.CONSTRUCTOR), List())), Literal(Constant(())))))))
+    c.typeCheck(classDef)
+    c.Expr[Any](Literal(Constant(())))
+  }
+
+  def foo: Any = macro impl
+}
diff --git a/test/files/run/typecheck/Test_2.scala b/test/files/run/typecheck/Test_2.scala
@@ -0,0 +1,15 @@
+import scala.reflect.runtime.universe._
+import scala.reflect.runtime.{currentMirror => cm}
+import scala.tools.reflect.ToolBox
+
+object Test extends App {
+  Macros.foo
+
+  val tb = cm.mkToolBox()
+  val classDef = ClassDef(
+    Modifiers(), newTypeName("C"), List(),
+    Template(
+      List(Select(Ident(newTermName("scala")), newTypeName("AnyRef"))), emptyValDef,
+      List(DefDef(Modifiers(), nme.CONSTRUCTOR, List(), List(List()), TypeTree(), Block(List(Apply(Select(Super(This(tpnme.EMPTY), tpnme.EMPTY), nme.CONSTRUCTOR), List())), Literal(Constant(())))))))
+  tb.typeCheck(classDef)
+}