Merge pull request #1754 from retronym/ticket/6288

SI-6288 Perfecting positions

8 files changed, 252 insertions, 14 deletions

diff --git a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
index 068836fe4f..d50d4cd125 100644
--- a/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
+++ b/src/compiler/scala/tools/nsc/backend/icode/BasicBlocks.scala
@@ -320,7 +320,12 @@ trait BasicBlocks {
       else
         instrs.zipWithIndex collect {
           case (oldInstr, i) if map contains oldInstr =>
-            code.touched |= replaceInstruction(i, map(oldInstr))
+            // SI-6288 clone important here because `replaceInstruction` assigns
+            // a position to `newInstr`. Without this, a single instruction can
+            // be added twice, and the position last position assigned clobbers
+            // all previous positions in other usages.
+            val newInstr = map(oldInstr).clone()
+            code.touched |= replaceInstruction(i, newInstr)
         }
 
     ////////////////////// Emit //////////////////////
diff --git a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
index 834c64aaae..fa8aff5cdd 100644
--- a/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/PatternMatching.scala
@@ -801,8 +801,8 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
       protected def spliceApply(binder: Symbol): Tree = {
         object splice extends Transformer {
           override def transform(t: Tree) = t match {
-            case Apply(x, List(Ident(nme.SELECTOR_DUMMY))) =>
-              treeCopy.Apply(t, x, List(CODE.REF(binder)))
+            case Apply(x, List(i @ Ident(nme.SELECTOR_DUMMY))) =>
+              treeCopy.Apply(t, x, List(CODE.REF(binder).setPos(i.pos)))
             case _ => super.transform(t)
           }
         }
@@ -879,7 +879,7 @@ trait PatternMatching extends Transform with TypingTransformers with ast.TreeDSL
           override def transform(tree: Tree): Tree = {
             def subst(from: List[Symbol], to: List[Tree]): Tree =
               if (from.isEmpty) tree
-              else if (tree.symbol == from.head) typedIfOrigTyped(to.head.shallowDuplicate, tree.tpe)
+              else if (tree.symbol == from.head) typedIfOrigTyped(to.head.shallowDuplicate.setPos(tree.pos), tree.tpe)
               else subst(from.tail, to.tail)
 
             tree match {
diff --git a/src/partest/scala/tools/partest/DirectTest.scala b/src/partest/scala/tools/partest/DirectTest.scala
index 8c18809ad6..483cb491a1 100644
--- a/src/partest/scala/tools/partest/DirectTest.scala
+++ b/src/partest/scala/tools/partest/DirectTest.scala
@@ -39,6 +39,10 @@ abstract class DirectTest extends App {
   // new compiler
   def newCompiler(args: String*): Global = {
     val settings = newSettings((CommandLineParser tokenize ("-d \"" + testOutput.path + "\" " + extraSettings)) ++ args.toList)
+    newCompiler(settings)
+  }
+
+  def newCompiler(settings: Settings): Global = {
     if (settings.Yrangepos.value) new Global(settings, reporter(settings)) with interactive.RangePositions
     else new Global(settings, reporter(settings))
   }
diff --git a/test/files/run/inline-ex-handlers.check b/test/files/run/inline-ex-handlers.check
index e786c780d6..45db7c3a15 100644
--- a/test/files/run/inline-ex-handlers.check
+++ b/test/files/run/inline-ex-handlers.check
@@ -47,7 +47,7 @@
 <     106	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     106	CALL_METHOD MyException.message (dynamic)
 502c504
 <   blocks: [1,2,3,4,6,7,8,9,10]
 ---
@@ -162,12 +162,12 @@
 <     176	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     176	CALL_METHOD MyException.message (dynamic)
 783c833,834
 <     177	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     177	CALL_METHOD MyException.message (dynamic)
 785c836,837
 <     177	THROW(MyException)
 ---
@@ -194,12 +194,12 @@
 <     181	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     181	CALL_METHOD MyException.message (dynamic)
 822c878,879
 <     182	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     182	CALL_METHOD MyException.message (dynamic)
 824c881,882
 <     182	THROW(MyException)
 ---
@@ -260,7 +260,7 @@
 <     127	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     127	CALL_METHOD MyException.message (dynamic)
 966c1042
 <     catch (IllegalArgumentException) in ArrayBuffer(6, 7, 8, 11, 14, 16, 17, 19) starting at: 3
 ---
@@ -299,7 +299,7 @@
 <     154	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     154	CALL_METHOD MyException.message (dynamic)
 1275c1354
 <   blocks: [1,2,3,4,5,7]
 ---
@@ -354,22 +354,23 @@
 <     213	LOAD_LOCAL(value message)
 ---
 >     ?	LOAD_LOCAL(value x5)
->     ?	CALL_METHOD MyException.message (dynamic)
+>     213	CALL_METHOD MyException.message (dynamic)
 1470c1560
 <   blocks: [1,2,3,4,5,7]
 ---
 >   blocks: [1,2,3,4,5,7,8]
-1494c1584,1591
+1494c1584,1585
 <     58	THROW(IllegalArgumentException)
 ---
 >     ?	STORE_LOCAL(value e)
 >     ?	JUMP 8
->     
+1495a1587,1592
 >   8: 
 >     62	LOAD_MODULE object Predef
 >     62	CONSTANT("RuntimeException")
 >     62	CALL_METHOD scala.Predef.println (dynamic)
 >     62	JUMP 2
+>     
 1543c1640
 <   blocks: [1,2,3,4]
 ---
diff --git a/test/files/run/t6288.check b/test/files/run/t6288.check
new file mode 100644
index 0000000000..af6bd5d269
--- /dev/null
+++ b/test/files/run/t6288.check
@@ -0,0 +1,85 @@
+[[syntax trees at end of                    patmat]] // newSource1
+[7]package [7]<empty> {
+  [7]object Case3 extends [13][106]scala.AnyRef {
+    [106]def <init>(): [13]Case3.type = [106]{
+      [106][106][106]Case3.super.<init>();
+      [13]()
+    };
+    [21]def unapply([29]z: [32]<type: [32]scala.Any>): [21]Option[Int] = [56][52][52]scala.Some.apply[[52]Int]([58]-1);
+    [64]{
+      [64]case <synthetic> val x1: [64]Any = [64]"";
+      [64]case5()[84]{
+        [84]<synthetic> val o7: [84]Option[Int] = [84][84]Case3.unapply([84]x1);
+        [84]if ([84]o7.isEmpty.unary_!)
+          [84]{
+            [90]val nr: [90]Int = [90]o7.get;
+            [97][97]matchEnd4([97]())
+          }
+        else
+          [84][84]case6()
+      };
+      [64]case6(){
+        [64][64]matchEnd4([64]throw [64][64][64]new [64]MatchError([64]x1))
+      };
+      [64]matchEnd4(x: [NoPosition]Unit){
+        [64]x
+      }
+    }
+  };
+  [113]object Case4 extends [119][217]scala.AnyRef {
+    [217]def <init>(): [119]Case4.type = [217]{
+      [217][217][217]Case4.super.<init>();
+      [119]()
+    };
+    [127]def unapplySeq([138]z: [141]<type: [141]scala.Any>): [127]Option[List[Int]] = [167]scala.None;
+    [175]{
+      [175]case <synthetic> val x1: [175]Any = [175]"";
+      [175]case5()[195]{
+        [195]<synthetic> val o7: [195]Option[List[Int]] = [195][195]Case4.unapplySeq([195]x1);
+        [195]if ([195]o7.isEmpty.unary_!)
+          [195]if ([195][195][195][195]o7.get.!=([195]null).&&([195][195][195][195]o7.get.lengthCompare([195]1).==([195]0)))
+            [195]{
+              [201]val nr: [201]Int = [201][201]o7.get.apply([201]0);
+              [208][208]matchEnd4([208]())
+            }
+          else
+            [195][195]case6()
+        else
+          [195][195]case6()
+      };
+      [175]case6(){
+        [175][175]matchEnd4([175]throw [175][175][175]new [175]MatchError([175]x1))
+      };
+      [175]matchEnd4(x: [NoPosition]Unit){
+        [175]x
+      }
+    }
+  };
+  [224]object Case5 extends [230][312]scala.AnyRef {
+    [312]def <init>(): [230]Case5.type = [312]{
+      [312][312][312]Case5.super.<init>();
+      [230]()
+    };
+    [238]def unapply([246]z: [249]<type: [249]scala.Any>): [238]Boolean = [265]true;
+    [273]{
+      [273]case <synthetic> val x1: [273]Any = [273]"";
+      [273]case5()[293]{
+        [293]<synthetic> val o7: [293]Option[List[Int]] = [293][293]Case4.unapplySeq([293]x1);
+        [293]if ([293]o7.isEmpty.unary_!)
+          [293]if ([293][293][293][293]o7.get.!=([293]null).&&([293][293][293][293]o7.get.lengthCompare([293]0).==([195]0)))
+            [304][304]matchEnd4([304]())
+          else
+            [293][293]case6()
+        else
+          [293][293]case6()
+      };
+      [273]case6(){
+        [273][273]matchEnd4([273]throw [273][273][273]new [273]MatchError([273]x1))
+      };
+      [273]matchEnd4(x: [NoPosition]Unit){
+        [273]x
+      }
+    }
+  }
+}
+
diff --git a/test/files/run/t6288.scala b/test/files/run/t6288.scala
new file mode 100644
index 0000000000..cf5865e95a
--- /dev/null
+++ b/test/files/run/t6288.scala
@@ -0,0 +1,41 @@
+import scala.tools.partest._
+import java.io.{Console => _, _}
+
+object Test extends DirectTest {
+
+  override def extraSettings: String = "-usejavacp -Xprint:patmat -Xprint-pos -d " + testOutput.path
+
+  override def code =
+    """
+      |object Case3 {
+      |  def unapply(z: Any): Option[Int] = Some(-1)
+      |
+      |  "" match {
+      |    case Case3(nr) => ()
+      |  }
+      |}
+      |object Case4 {
+      |  def unapplySeq(z: Any): Option[List[Int]] = None
+      |
+      |  "" match {
+      |    case Case4(nr) => ()
+      |  }
+      |}
+      |object Case5 {
+      |  def unapply(z: Any): Boolean = true
+      |
+      |  "" match {
+      |    case Case4() => ()
+      |  }
+      |}
+      |
+      |""".stripMargin.trim
+
+  override def show(): Unit = {
+    // Now: [84][84]Case3.unapply([84]x1);
+    // Was: [84][84]Case3.unapply([64]x1);
+    Console.withErr(System.out) {
+      compile()
+    }
+  }
+}
diff --git a/test/files/run/t6288b-jump-position.check b/test/files/run/t6288b-jump-position.check
new file mode 100644
index 0000000000..45ec31c308
--- /dev/null
+++ b/test/files/run/t6288b-jump-position.check
@@ -0,0 +1,80 @@
+object Case3 extends Object {
+  // fields:
+  
+  // methods
+  def unapply(z: Object (REF(class Object))): Option {
+  locals: value z
+  startBlock: 1
+  blocks: [1]
+  
+  1: 
+    2	NEW REF(class Some)
+    2	DUP(REF(class Some))
+    2	CONSTANT(-1)
+    2	BOX INT
+    2	CALL_METHOD scala.Some.<init> (static-instance)
+    2	RETURN(REF(class Option))
+    
+  }
+  Exception handlers: 
+    
+  def main(args: Array[String] (ARRAY[REF(class String)])): Unit {
+  locals: value args, value x1, value x2, value x
+  startBlock: 1
+  blocks: [1,2,3,6,7]
+  
+  1: 
+    4	CONSTANT("")
+    4	STORE_LOCAL(value x1)
+    4	SCOPE_ENTER value x1
+    4	JUMP 2
+    
+  2: 
+    5	LOAD_LOCAL(value x1)
+    5	IS_INSTANCE REF(class String)
+    5	CZJUMP (BOOL)NE ? 3 : 6
+    
+  3: 
+    5	LOAD_LOCAL(value x1)
+    5	CHECK_CAST REF(class String)
+    5	STORE_LOCAL(value x2)
+    5	SCOPE_ENTER value x2
+    6	LOAD_MODULE object Predef
+    6	CONSTANT("case 0")
+    6	CALL_METHOD scala.Predef.println (dynamic)
+    6	LOAD_FIELD scala.runtime.BoxedUnit.UNIT
+    6	STORE_LOCAL(value x)
+    6	JUMP 7
+    
+  6: 
+    8	LOAD_MODULE object Predef
+    8	CONSTANT("default")
+    8	CALL_METHOD scala.Predef.println (dynamic)
+    8	LOAD_FIELD scala.runtime.BoxedUnit.UNIT
+    8	STORE_LOCAL(value x)
+    8	JUMP 7
+    
+  7: 
+    10	LOAD_MODULE object Predef
+    10	CONSTANT("done")
+    10	CALL_METHOD scala.Predef.println (dynamic)
+    10	RETURN(UNIT)
+    
+  }
+  Exception handlers: 
+    
+  def <init>(): Case3.type {
+  locals: 
+  startBlock: 1
+  blocks: [1]
+  
+  1: 
+    12	THIS(Case3)
+    12	CALL_METHOD java.lang.Object.<init> (super())
+    12	RETURN(UNIT)
+    
+  }
+  Exception handlers: 
+    
+  
+}
diff --git a/test/files/run/t6288b-jump-position.scala b/test/files/run/t6288b-jump-position.scala
new file mode 100644
index 0000000000..e22a1ab120
--- /dev/null
+++ b/test/files/run/t6288b-jump-position.scala
@@ -0,0 +1,22 @@
+import scala.tools.partest.IcodeTest
+
+object Test extends IcodeTest {
+  override def code =
+    """object Case3 {                                // 01
+     |  def unapply(z: Any): Option[Int] = Some(-1)  // 02
+     |  def main(args: Array[String]) {              // 03
+     |    ("": Any) match {                          // 04
+     |      case x : String =>                       // 05 Read: <linenumber> JUMP <target basic block id>
+     |        println("case 0")                      // 06 expecting "6 JUMP 7", was "8 JUMP 7"
+     |      case _ =>                                // 07
+     |        println("default")                     // 08 expecting "8 JUMP 7"
+     |    }                                          // 09
+     |    println("done")                            // 10
+     |  }
+     |}""".stripMargin
+
+  override def show() {
+    val lines1 = collectIcode("")
+    println(lines1 mkString "\n")
+  }
+}