pattern matcher could do a better job positioning trees

[retronym]

I guess I've gotten used to the flip-flop of the debugger with pattern matches over the years. But after fixing #186, I've taken a fresh look at what we generate and believe there are some obvious flaws to rectify.

class Patmat1 {
  def foo(a: Any) = a match {
    case a: String if false =>
    case _: String =>
       ""
    case _ =>
       ""
  }
}

[[syntax trees at end of                    patmat]] // test.scala
[6]package [6]<empty> {
  [6]class Patmat1 extends [14][138]scala.AnyRef {
    [138]def <init>(): [14]Patmat1 = [138]{
      [138][138][138]Patmat1.super.<init>();
      [14]()
    };
    [22]def foo([26]a: [29]<type: [29]scala.Any>): [22]Any = [36]{
      [55]<synthetic> var rc5: [55]Boolean = [55]false;
      [55]<synthetic> var x2: [55]String = [55][55][55]null.asInstanceOf[[55]String];
      [36]{
        [36]case <synthetic> val x1: [36]Any = [36]a;
        [36]case7(){
          [55]if ([55][55]x1.isInstanceOf[[55]String])
            [55]{
              [55][55]rc5 = [55]true;
              [55][55]x2 = [55]([55][55]x1.asInstanceOf[[55]String]: [55]String);
              [55]if ([68]false)
                [74][74]matchEnd6([74]())
              else
                [55][55]case8()
            }
          else
            [55][55]case8()
        };
        [36]case8(){
          [36]if ([36]rc5)
            [106][106]matchEnd6([106]"")
          else
            [36][36]case9()
        };
        [36]case9(){
          [130][130]matchEnd6([130]"")
        };
        [36]matchEnd6(x: [NoPosition]Any){
          [36]x
        }
      }
    }
  }
}

• initialization to zero of temp vars x2 and rc5 should be positioned at the match, not at the pattern that gave rise to them.
• def case8 and the if it contains should not be positioned back at the match, but rather at the pattern on line 4.
• Not sure where the matchEnd6 should be positioned. Could it be moved to the last line of the match { }? What does javac do with switch-es?

Here's the bytecode positions that this all leads to.

  public java.lang.Object foo(java.lang.Object);
    descriptor: (Ljava/lang/Object;)Ljava/lang/Object;
    flags: ACC_PUBLIC
    Code:
      stack=1, locals=6, args_size=2
         0: iconst_0
         1: istore_3
         2: aconst_null
         3: astore        4
         5: aload_1
         6: astore        5
         8: aload         5
        10: instanceof    #13                 // class java/lang/String
        13: ifeq          28
        16: iconst_1
        17: istore_3
        18: aload         5
        20: checkcast     #13                 // class java/lang/String
        23: astore        4
        25: goto          31
        28: goto          31
        31: iload_3
        32: ifeq          41
        35: ldc           #15                 // String
        37: astore_2
        38: goto          50
        41: goto          44
        44: ldc           #15                 // String
        46: astore_2
        47: goto          50
        50: aload_2
        51: areturn
      LocalVariableTable:
        Start  Length  Slot  Name   Signature
            0      52     0  this   LPatmat1;
            0      52     1     a   Ljava/lang/Object;
      LineNumberTable:
        line 3: 0
        line 2: 5
        line 3: 8
        line 5: 35
        line 2: 41
        line 7: 44

[retronym]

I'm not actually able to reproduce a bad debugging experience in IntelliJ for that sample. Maybe it has some smarts to avoid the problem? Or maybe there isn't a problem. I'll play around with more complex examples to try to remind myself of what my actual complaint is...

[retronym]

Maybe I did enough in https://issues.scala-lang.org/browse/SI-6288 to fix the experience.

[retronym]

Just checked with JetBrains; they've worked around the dodgy positions. We should still pursue these clean ups

See customizeCaseClauses in:

https://github.com/JetBrains/intellij-scala/blob/e3730efcad13069c4dc36d61a3675bfc32d75a45/src/org/jetbrains/plugins/scala/debugger/LocationLineManager.scala

[retronym]

WIP: scala/scala@2.12.x...retronym:ticket/SD-187

Demo of improved positions: https://gist.github.com/retronym/71cc7c20663b743a81a95bea50ddb208