Fixed local variable table scoping

5 files changed, 173 insertions, 49 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
index 1e1fe8f629..8a6d480ca3 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
@@ -9,7 +9,7 @@ package scala.tools.nsc.backend.icode
 
 import compat.StringBuilder
 import scala.tools.nsc.ast._
-import scala.collection.mutable.Map
+import scala.collection.mutable.{Map, Set, HashSet}
 import scala.tools.nsc.util.Position
 import scala.tools.nsc.backend.icode.analysis.ProgramPoint
 
@@ -36,6 +36,11 @@ trait BasicBlocks requires ICodes {
     /** Is this block the head of a while? */
     var loopHeader = false
 
+    /** Local variables that are in scope at entry of this basic block. Used
+     *  for debugging information.
+     */
+    var varsInScope: Set[Local] = HashSet.empty
+
     /** ICode instructions, used as temporary storage while emitting code.
      * Once closed is called, only the `instrs' array should be used.
      */
@@ -353,10 +358,10 @@ trait BasicBlocks requires ICodes {
       preds
     }
 
-    override def equals(other: Any): Boolean =
-      other.isInstanceOf[BasicBlock] &&
-      other.asInstanceOf[BasicBlock].label == label &&
-      other.asInstanceOf[BasicBlock].code  == code
+    override def equals(other: Any): Boolean = other match {
+      case that: BasicBlock => that.label == label && that.code == code
+      case _ => false
+    }
 
     // Instead of it, rather use a printer
     def print() : unit = print(java.lang.System.out)
diff --git a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
index 3855438e6e..6b1a2024f1 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/GenICode.scala
@@ -8,7 +8,7 @@
 package scala.tools.nsc.backend.icode
 
 import compat.StringBuilder
-import scala.collection.mutable.{Map, HashMap}
+import scala.collection.mutable.{Map, HashMap, ListBuffer, Buffer, Set, HashSet}
 import scala.tools.nsc.symtab._
 
 
@@ -69,6 +69,9 @@ abstract class GenICode extends SubComponent  {
       ctx1
     }
 
+    /** The maximum line number seen so far in the current method. */
+    private var maxLineNr = 0
+
     /////////////////// Code generation ///////////////////////
 
     def gen(tree: Tree, ctx: Context): Context = tree match {
@@ -100,6 +103,7 @@ abstract class GenICode extends SubComponent  {
         m.returnType = if (tree.symbol.isConstructor) UNIT
                        else toTypeKind(tree.symbol.info.resultType)
         ctx.clazz.addMethod(m)
+        maxLineNr = unit.position(tree.pos).line
 
         var ctx1 = ctx.enterMethod(m, tree.asInstanceOf[DefDef])
         addMethodParams(ctx1, vparamss)
@@ -152,32 +156,33 @@ abstract class GenICode extends SubComponent  {
      * @return a new context. This is necessary for control flow instructions
      *         which may change the current basic block.
      */
-    private def genStat(tree: Tree, ctx: Context): Context = tree match {
-      case Assign(lhs @ Select(_, _), rhs) =>
-        if (isStaticSymbol(lhs.symbol)) {
+    private def genStat(tree: Tree, ctx: Context): Context = {
+      maxLineNr = compat.Math.max(unit.position(tree.pos).line, maxLineNr)
+
+      tree match {
+        case Assign(lhs @ Select(_, _), rhs) =>
+          if (isStaticSymbol(lhs.symbol)) {
+            val ctx1 = genLoad(rhs, ctx, toTypeKind(lhs.symbol.info))
+            ctx1.bb.emit(STORE_FIELD(lhs.symbol, true), tree.pos)
+            ctx1
+          } else {
+            var ctx1 = genLoadQualifier(lhs, ctx)
+            ctx1 = genLoad(rhs, ctx1, toTypeKind(lhs.symbol.info))
+            ctx1.bb.emit(STORE_FIELD(lhs.symbol, false), tree.pos)
+            ctx1
+          }
+
+        case Assign(lhs, rhs) =>
           val ctx1 = genLoad(rhs, ctx, toTypeKind(lhs.symbol.info))
-          ctx1.bb.emit(STORE_FIELD(lhs.symbol, true), tree.pos)
+          val Some(l) = ctx.method.lookupLocal(lhs.symbol)
+          ctx1.bb.emit(STORE_LOCAL(l), tree.pos)
           ctx1
-        } else {
-          var ctx1 = genLoadQualifier(lhs, ctx)
-          ctx1 = genLoad(rhs, ctx1, toTypeKind(lhs.symbol.info))
-          ctx1.bb.emit(STORE_FIELD(lhs.symbol, false), tree.pos)
-          ctx1
-        }
-
-      case Assign(lhs, rhs) =>
-//          assert(ctx.method.locals.contains(lhs.symbol) |
-//                 ctx.clazz.fields.contains(lhs.symbol),
-//                 "Assignment to inexistent local or field: " + lhs.symbol)
-        val ctx1 = genLoad(rhs, ctx, toTypeKind(lhs.symbol.info))
-        val Some(l) = ctx.method.lookupLocal(lhs.symbol)
-        ctx1.bb.emit(STORE_LOCAL(l), tree.pos)
-        ctx1
 
-      case _ =>
-        if (settings.debug.value)
-          log("Passing " + tree + " to genLoad")
-        genLoad(tree, ctx, UNIT)
+        case _ =>
+          if (settings.debug.value)
+            log("Passing " + tree + " to genLoad")
+          genLoad(tree, ctx, UNIT)
+      }
     }
 
     /**
@@ -192,6 +197,7 @@ abstract class GenICode extends SubComponent  {
      */
     private def genLoad(tree: Tree, ctx: Context, expectedType: TypeKind): Context = {
       var generatedType = expectedType
+      maxLineNr = compat.Math.max(unit.position(tree.pos).line, maxLineNr)
 
       /**
        * Generate code for primitive arithmetic operations.
@@ -402,6 +408,8 @@ abstract class GenICode extends SubComponent  {
           val sym = tree.symbol
           var local = new Local(sym, toTypeKind(sym.info), false)
           local = ctx.method.addLocal(local)
+          ctx.scope.add(local)
+          ctx.bb.emit(SCOPE_ENTER(local))
 
           if (rhs == EmptyTree) {
             if (settings.debug.value)
@@ -865,9 +873,12 @@ abstract class GenICode extends SubComponent  {
           ctx
 
         case Block(stats, expr) =>
-          assert(!(ctx.method eq null), "Block outside method")
-          val ctx1 = genStat(stats, ctx)
-          genLoad(expr, ctx1, expectedType)
+//          assert(!(ctx.method eq null), "Block outside method")
+          ctx.enterScope
+          var ctx1 = genStat(stats, ctx)
+          ctx1 = genLoad(expr, ctx1, expectedType)
+          ctx1.exitScope
+          ctx1
 
         case Typed(expr, _) =>
           genLoad(expr, ctx, expectedType)
@@ -1381,6 +1392,8 @@ abstract class GenICode extends SubComponent  {
           eqEqTempVar.setInfo(definitions.AnyRefClass.typeConstructor)
           val local = new Local(eqEqTempVar, REFERENCE(definitions.AnyRefClass), false)
           ctx.method.addLocal(local)
+          local.start = unit.position(l.pos).line
+          local.end   = unit.position(r.pos).line
           local
       }
 
@@ -1453,8 +1466,12 @@ abstract class GenICode extends SubComponent  {
         case Nil => ()
 
         case vparams :: Nil =>
-          for (val p <- vparams)
-            ctx.method.addParam(new Local(p.symbol, toTypeKind(p.symbol.info), true));
+          for (val p <- vparams) {
+            val lv = new Local(p.symbol, toTypeKind(p.symbol.info), true)
+            ctx.method.addParam(lv)
+            ctx.scope.add(lv)
+            ctx.bb.varsInScope += lv
+          }
           ctx.method.params = ctx.method.params.reverse
 
         case _ =>
@@ -1623,6 +1640,9 @@ abstract class GenICode extends SubComponent  {
       /** The current exception handler, when we generate code for one. */
       var currentExceptionHandler: Option[ExceptionHandler] = None
 
+      /** The current local variable scope. */
+      var scope: Scope = EmptyScope
+
       var handlerCount = 0
 
       override def toString(): String = {
@@ -1634,6 +1654,7 @@ abstract class GenICode extends SubComponent  {
         buf.append("\tlabels: ").append(labels).append('\n')
         buf.append("\texception handlers: ").append(handlers).append('\n')
         buf.append("\tcleanups: ").append(cleanups).append('\n')
+        buf.append("\tscope: ").append(scope).append('\n')
         buf.toString()
       }
 
@@ -1649,6 +1670,7 @@ abstract class GenICode extends SubComponent  {
         this.handlerCount = other.handlerCount
         this.cleanups = other.cleanups
         this.currentExceptionHandler = other.currentExceptionHandler
+        this.scope = other.scope
       }
 
       def setPackage(p: Name): this.type = {
@@ -1702,6 +1724,9 @@ abstract class GenICode extends SubComponent  {
         ctx1.method.code = new Code(m.symbol.simpleName.toString())
         ctx1.bb = ctx1.method.code.startBlock
         ctx1.defdef = d
+//        assert(ctx1.scope == EmptyScope, ctx1.scope)
+        ctx1.scope = EmptyScope
+        ctx1.enterScope
         ctx1
       }
 
@@ -1713,9 +1738,22 @@ abstract class GenICode extends SubComponent  {
           case Some(e) => e.addBlock(block)
           case None    => ()
         }
+        block.varsInScope = new HashSet()
+        block.varsInScope ++= scope.varsInScope
         new Context(this) setBasicBlock block;
       }
 
+      def enterScope = {
+        scope = new Scope(scope)
+      }
+
+      def exitScope = {
+        if (bb.size > 0) {
+          scope.locals foreach { lv => bb.emit(SCOPE_EXIT(lv)) }
+        }
+        scope = scope.outer
+      }
+
       /** Create a new exception handler and adds it in the list
        * of current exception handlers. All new blocks will be
        * 'covered' by this exception handler (in addition to the
@@ -1931,4 +1969,25 @@ abstract class GenICode extends SubComponent  {
         failure.addCallingInstruction(this)
     }
 
+  /** Local variable scopes. Keep track of line numbers for debugging info. */
+  class Scope(val outer: Scope) {
+    val locals: ListBuffer[Local] = new ListBuffer
+
+    def add(l: Local) =
+      locals += l
+
+    def remove(l: Local) =
+      locals -= l
+
+    /** Return all locals that are in scope. */
+    def varsInScope: Buffer[Local] = outer.varsInScope ++ locals
+
+    override def toString() =
+      outer.toString() + locals.mkString("[", ", ", "]")
+  }
+
+  object EmptyScope extends Scope(null) {
+    override def toString() = "[]"
+    override def varsInScope: Buffer[Local] = new ListBuffer
+  }
 }
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Members.scala b/src/compiler/scala/tools/nsc/backend/icode/Members.scala
index ac503bd1e2..6116b73d1e 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Members.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Members.scala
@@ -222,6 +222,15 @@ trait Members requires ICodes {
   class Local(val sym: Symbol, val kind: TypeKind, val arg: Boolean) {
     var index: Int = -1;
 
+    /** Starting PC for this local's visbility range. */
+    var start: Int = _
+
+    /** Ending PC for this local's visbility range. */
+    var end: Int = _
+
+    /** PC-based ranges for this local variable's visibility range */
+    var ranges: List[{Int, Int}] = Nil
+
     override def equals(other: Any): Boolean = (
       other.isInstanceOf[Local] &&
       other.asInstanceOf[Local].sym == this.sym
diff --git a/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala b/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala
index f6bed76977..e50bc65cc4 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/Opcodes.scala
@@ -510,6 +510,26 @@ trait Opcodes requires ICodes {
       override def produced = 0;
     }
 
+    /** A local variable becomes visible at this point in code.
+     *  Used only for generating precise local variable tables as
+     *  debugging information.
+     */
+    case class SCOPE_ENTER(lv: Local) extends Instruction {
+      override def toString(): String = "SCOPE_ENTER " + lv
+      override def consumed = 0
+      override def produced = 0
+    }
+
+    /** A local variable leaves its scope at this point in code.
+     *  Used only for generating precise local variable tables as
+     *  debugging information.
+     */
+    case class SCOPE_EXIT(lv: Local) extends Instruction {
+      override def toString(): String = "SCOPE_EXIT " + lv
+      override def consumed = 0
+      override def produced = 0
+    }
+
     /** This class represents a method invocation style. */
     sealed abstract class InvokeStyle {
 
diff --git a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
index c43e9069a9..f6797e1ba7 100644
--- a/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
+++ b/src/compiler/scala/tools/nsc/backend/jvm/GenJVM.scala
@@ -11,7 +11,7 @@ import java.io.File
 import java.nio.ByteBuffer
 
 import scala.collection.immutable.{Set, ListSet}
-import scala.collection.mutable.{Map, HashMap}
+import scala.collection.mutable.{Map, HashMap, HashSet}
 import scala.tools.nsc.symtab._
 import scala.tools.nsc.util.Position
 
@@ -637,6 +637,9 @@ abstract class GenJVM extends SubComponent {
       });
     }
 
+    /** local variables whose scope appears in this block. */
+    var varsInBlock: collection.mutable.Set[Local] = new HashSet
+
     def genBlock(b: BasicBlock): Unit = {
       labels(b).anchorToNext()
 
@@ -645,6 +648,7 @@ abstract class GenJVM extends SubComponent {
       var lastMappedPC = 0
       var lastLineNr = 0
       var crtPC = 0
+      varsInBlock.clear
 
       b traverse ( instr => {
         class CompilationError(msg: String) extends Error {
@@ -978,6 +982,20 @@ abstract class GenJVM extends SubComponent {
 
           case MONITOR_EXIT() =>
             jcode.emitMONITOREXIT()
+
+          case SCOPE_ENTER(lv) =>
+            varsInBlock += lv
+            lv.start = jcode.getPC()
+
+          case SCOPE_EXIT(lv) =>
+            if (varsInBlock contains lv) {
+              lv.ranges = {lv.start, jcode.getPC()} :: lv.ranges
+              varsInBlock -= lv
+            } else if (b.varsInScope contains lv) {
+              lv.ranges = {labels(b).getAnchor(), jcode.getPC()} :: lv.ranges
+              b.varsInScope -= lv
+            } else
+              assert(false, "Illegal local var nesting: " + method)
         }
 
         crtPC = jcode.getPC()
@@ -1000,7 +1018,15 @@ abstract class GenJVM extends SubComponent {
           lastLineNr   = crtLine
         }
 
-      });
+      }); // b.traverse
+
+      // local vars that survived this basic block
+      for (val lv <- varsInBlock) {
+        lv.ranges = {lv.start, jcode.getPC()} :: lv.ranges
+      }
+      for (val lv <- b.varsInScope) {
+        lv.ranges = {labels(b).getAnchor(), jcode.getPC()} :: lv.ranges
+      }
     }
 
     /**
@@ -1395,9 +1421,11 @@ abstract class GenJVM extends SubComponent {
         val pool = jclass.getConstantPool()
         val pc = jcode.getPC()
         var anonCounter = 0
-        val locals = if (jmethod.isStatic()) vars.length else 1 + vars.length
+        var entries = 0
+        vars.foreach { lv => lv.ranges = mergeEntries(lv.ranges.reverse); entries = entries + lv.ranges.length }
+        if (!jmethod.isStatic()) entries = entries + 1
 
-        val lvTab = ByteBuffer.allocate(2 + 10 * locals)
+        val lvTab = ByteBuffer.allocate(2 + 10 * entries)
         def emitEntry(name: String, signature: String, idx: Short, start: Short, end: Short): Unit = {
           lvTab.putShort(start)
           lvTab.putShort(end)
@@ -1406,7 +1434,7 @@ abstract class GenJVM extends SubComponent {
           lvTab.putShort(idx)
         }
 
-        lvTab.putShort(locals.asInstanceOf[Short])
+        lvTab.putShort(entries.asInstanceOf[Short])
 
         if (!jmethod.isStatic()) {
           emitEntry("this", jclass.getType().getSignature(), 0, 0.asInstanceOf[Short], pc.asInstanceOf[Short])
@@ -1418,17 +1446,11 @@ abstract class GenJVM extends SubComponent {
               "<anon" + anonCounter + ">"
             } else javaName(lv.sym)
 
-            val startPC = 0.asInstanceOf[Short]
-            var endPC   = pc
-            if (startPC > endPC) {
-//              Console.println("startPC: " + startPC + " endPC: " + endPC + " start: " + lv.start + " end: " + lv.end)
-              endPC = pc
+            val index = indexOf(lv).asInstanceOf[Short]
+            val tpe   = javaType(lv.kind).getSignature()
+            for (val {start, end} <- lv.ranges) {
+              emitEntry(name, tpe, index, start.asInstanceOf[Short], (end - start).asInstanceOf[Short])
             }
-//            log(lv + " start: " + lv.start + " end: " + lv.end)
-
-            emitEntry(name, javaType(lv.kind).getSignature(),
-                      indexOf(lv).asInstanceOf[Short],
-                      startPC, (endPC - startPC).asInstanceOf[Short])
         }
         val attr =
             fjbgContext.JOtherAttribute(jclass,
@@ -1438,6 +1460,15 @@ abstract class GenJVM extends SubComponent {
         jcode.addAttribute(attr)
     }
 
+
+    /** Merge adjacent ranges. */
+    private def mergeEntries(ranges: List[{Int, Int}]): List[{Int, Int}] =
+      (ranges.foldLeft(Nil: List[{Int, Int}]) { (collapsed: List[{Int, Int}], p: {Int, Int}) => {collapsed, p} match {
+        case { Nil, _ } => List(p)
+        case { {s1, e1} :: rest, {s2, e2} } if (e1 == s2) => {s1, e2} :: rest
+        case _ => p :: collapsed
+      }}).reverse
+
     def assert(cond: Boolean, msg: String) = if (!cond) {
       dump(method)
       throw new Error(msg + "\nMethod: " + method)