Merge pull request #3080 from dotty-staging/improve-#2924

Fix #2924: make TreeChecker.typedBlock iterative

Author: nicolasstucki

compiler/src/dotty/tools/dotc/core/Decorators.scala

@@ -112,14 +112,6 @@ object Decorators {
         else x1 :: xs1
       }
 
-    def foldRightBN[U](z: => U)(op: (T, => U) => U): U = {
-      @tailrec def foldLeftBN(xs: List[T], acc: => U): U = xs match {
-        case x :: xs1 => foldLeftBN(xs1, op(x, acc))
-        case Nil => acc
-      }
-      foldLeftBN(xs.reverse, z)
-    }
-
     final def hasSameLengthAs[U](ys: List[U]): Boolean = {
       @tailrec def loop(xs: List[T], ys: List[U]): Boolean =
         if (xs.isEmpty) ys.isEmpty

compiler/src/dotty/tools/dotc/transform/FirstTransform.scala

@@ -92,26 +92,33 @@ class FirstTransform extends MiniPhaseTransform with InfoTransformer with Annota
   private def reorderAndComplete(stats: List[Tree])(implicit ctx: Context): List[Tree] = {
     val moduleClassDefs, singleClassDefs = mutable.Map[Name, Tree]()
 
-    def reorder(stats: List[Tree]): List[Tree] = stats match {
+    /* Returns the result of reordering stats and prepending revPrefix in reverse order to it.
+     * The result of reorder is equivalent to reorder(stats, revPrefix) = revPrefix.reverse ::: reorder(stats, Nil).
+     * This implementation is tail recursive as long as the element is not a module TypeDef.
+     */
+    def reorder(stats: List[Tree], revPrefix: List[Tree]): List[Tree] = stats match {
       case (stat: TypeDef) :: stats1 if stat.symbol.isClass =>
         if (stat.symbol is Flags.Module) {
+          def pushOnTop(xs: List[Tree], ys: List[Tree]): List[Tree] =
+            (ys /: xs)((ys, x) => x :: ys)
           moduleClassDefs += (stat.name -> stat)
           singleClassDefs -= stat.name.stripModuleClassSuffix
-          val stats1r = reorder(stats1)
-          if (moduleClassDefs contains stat.name) stat :: stats1r else stats1r
+          val stats1r = reorder(stats1, Nil)
+          pushOnTop(revPrefix, if (moduleClassDefs contains stat.name) stat :: stats1r else stats1r)
         } else {
-          def stats1r = reorder(stats1)
-          val normalized = moduleClassDefs remove stat.name.moduleClassName match {
-            case Some(mcdef) =>
-              mcdef :: stats1r
-            case None =>
-              singleClassDefs += (stat.name -> stat)
-              stats1r
-          }
-          stat :: normalized
+          reorder(
+            stats1,
+            moduleClassDefs remove stat.name.moduleClassName match {
+              case Some(mcdef) =>
+                mcdef :: stat :: revPrefix
+              case None =>
+                singleClassDefs += (stat.name -> stat)
+                stat :: revPrefix
+            }
+          )
         }
-      case stat :: stats1 => stat :: reorder(stats1)
-      case Nil => Nil
+      case stat :: stats1 => reorder(stats1, stat :: revPrefix)
+      case Nil => revPrefix.reverse
     }
 
     def registerCompanion(name: TermName, forClass: Symbol): TermSymbol = {
@@ -136,7 +143,7 @@ class FirstTransform extends MiniPhaseTransform with InfoTransformer with Annota
       case stat => stat
     }
 
-    addMissingCompanions(reorder(stats))
+    addMissingCompanions(reorder(stats, Nil))
   }
 
   private def newCompanion(name: TermName, forClass: Symbol)(implicit ctx: Context) = {

compiler/src/dotty/tools/dotc/transform/TreeChecker.scala

@@ -147,44 +147,41 @@ class TreeChecker extends Phase with SymTransformer {
     // don't check value classes after typer, as the constraint about constructors doesn't hold after transform
     override def checkDerivedValueClass(clazz: Symbol, stats: List[Tree])(implicit ctx: Context) = ()
 
-    def withDefinedSym[T](tree: untpd.Tree)(op: => T)(implicit ctx: Context): T = tree match {
-      case tree: untpd.DefTree =>
+    def withDefinedSyms[T](trees: List[untpd.Tree])(op: => T)(implicit ctx: Context) = {
+      var locally = List.empty[Symbol]
+      for (tree <- trees) {
         val sym = tree.symbol
-        assert(isValidJVMName(sym.name.encode), s"${sym.name.debugString} name is invalid on jvm")
-        everDefinedSyms.get(sym) match {
-          case Some(t)  =>
-            if (t ne tree)
-              ctx.warning(i"symbol ${sym.fullName} is defined at least twice in different parts of AST")
-            // should become an error
-          case None =>
-            everDefinedSyms(sym) = tree
-        }
-        assert(!nowDefinedSyms.contains(sym), i"doubly defined symbol: ${sym.fullName} in $tree")
-
-        if (ctx.settings.YcheckMods.value) {
-          tree match {
-            case t: untpd.MemberDef =>
-              if (t.name ne sym.name) ctx.warning(s"symbol ${sym.fullName} name doesn't correspond to AST: ${t}")
-            // todo: compare trees inside annotations
-            case _ =>
-          }
+        tree match {
+          case tree: untpd.DefTree =>
+            assert(isValidJVMName(sym.name.encode), s"${sym.name.debugString} name is invalid on jvm")
+            everDefinedSyms.get(sym) match {
+              case Some(t)  =>
+                if (t ne tree)
+                  ctx.warning(i"symbol ${sym.fullName} is defined at least twice in different parts of AST")
+              // should become an error
+              case None =>
+                everDefinedSyms(sym) = tree
+            }
+            assert(!nowDefinedSyms.contains(sym), i"doubly defined symbol: ${sym.fullName} in $tree")
+
+            if (ctx.settings.YcheckMods.value) {
+              tree match {
+                case t: untpd.MemberDef =>
+                  if (t.name ne sym.name) ctx.warning(s"symbol ${sym.fullName} name doesn't correspond to AST: ${t}")
+                // todo: compare trees inside annotations
+                case _ =>
+              }
+            }
+            locally = sym :: locally
+            nowDefinedSyms += sym
+          case _ =>
         }
-
-        nowDefinedSyms += tree.symbol
-        //ctx.echo(i"defined: ${tree.symbol}")
-        val res = op
-        nowDefinedSyms -= tree.symbol
-        //ctx.echo(i"undefined: ${tree.symbol}")
-        res
-      case _ => op
+      }
+      val res = op
+      nowDefinedSyms --= locally
+      res
     }
 
-    def withDefinedSyms[T](trees: List[untpd.Tree])(op: => T)(implicit ctx: Context) =
-      trees.foldRightBN(op)(withDefinedSym(_)(_))
-
-    def withDefinedSymss[T](vparamss: List[List[untpd.ValDef]])(op: => T)(implicit ctx: Context): T =
-      vparamss.foldRightBN(op)(withDefinedSyms(_)(_))
-
     def assertDefined(tree: untpd.Tree)(implicit ctx: Context) =
       if (
         tree.symbol.maybeOwner.isTerm &&
@@ -388,7 +385,7 @@ class TreeChecker extends Phase with SymTransformer {
 
     override def typedDefDef(ddef: untpd.DefDef, sym: Symbol)(implicit ctx: Context) =
       withDefinedSyms(ddef.tparams) {
-        withDefinedSymss(ddef.vparamss) {
+        withDefinedSyms(ddef.vparamss.flatten) {
           if (!sym.isClassConstructor && !(sym.name eq nme.STATIC_CONSTRUCTOR))
             assert(isValidJVMMethodName(sym.name.encode), s"${sym.name.debugString} name is invalid on jvm")