Scala Programming Language
  1. Scala Programming Language
  2. SI-7915

Wrong Tree returned by `askTypeAt` (wrong hyperlink)

    Details

      Description

      Foo.scala
      package foo
      
      class Foo {
        def foo() {
          new Bar().bar()  // <-- try hyperlinking on `bar` method, you'll jump to the enclosing `foo` method definition
        }
      }
      
      class Bar {
        def bar(b: Int = 2) {}
      }
      

      I've debugged the problem inside the Scala IDE, and the reason why this happen is that `askTypeAt` returns the enclosing `foo` DefDef method symbol, instead of the expected Select `Bar.bar` node. The enclosing method is returned becuase all Trees inside the method block have a TransparentPosition, all the way down. The impelmentation of `askTypeAt` relies on `RangePositions.Locator.traverse` to return the deepest, non-transparent, tree that includes the passed position. And, because all positions are transparent inside the `foo` method's body, then the `foo` symbol is returned.

      An additional interesting fact is that it looks like one of the transformation
      happening during typechecking is to blame for updating the Tree with the wrong kind of positions. In fact, if the source file isn't typechecked, the correct symbol is returned.

      Attached, you can find a patch that demonstrates the issue by running the presentation compiler in isolation. To run the test, simply type: ./test/partest test/files/presentation/t0000

        Activity

        Hide
        Mirco Dotta added a comment -

        Alright, let's see if I can fix it myself.

        Show
        Mirco Dotta added a comment - Alright, let's see if I can fix it myself.
        Hide
        Jason Zaugg added a comment -

        Here are some of the tools you need:

        scalac-hash origin/master -Xprint:typer -Yrangepos -Xprint-pos sandbox/test.scala
        [info] downloading http://scala-webapps.epfl.ch/artifacts/2c1e9af40e252bd9ed5bfe4305991ee6f7d25636/pack.tgz ...done.
        [info] scala revision from 2013-10-29 05:39:42 -0700 downloaded to /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af
        Password:
        [info] origin/mas => /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af
        [[syntax trees at end of                     typer]] // test.scala
        [0:201]package [8:11]foo {
          [14:160]class Foo extends [24:160][163]scala.AnyRef {
            [163]def <init>(): [24]foo.Foo = [163]{
              [163][163][163]Foo.super.<init>();
              [24]()
            };
            [28:158]def foo(): [32]Unit = <44:59>{
              [44:53]<artifact> val qual$1: [44]foo.Bar = [44:53][44:53][44:53]new [48:51]Bar();
              [54]<artifact> val x$1: [54]Int = [54]qual$1.bar$default$1;
              <44:59><44:57>qual$1.bar([54]x$1)
            }
          };
          [163:201]class Bar extends [173:201][201]scala.AnyRef {
            [201]def <init>(): [173]foo.Bar = [201]{
              [201][201][201]Bar.super.<init>();
              [173]()
            };
            [177:199]def bar([185:195]b: [188:191]<type: [188:191]scala.Int> = [194:195]2): [181]Unit = [197:199]();
            [185]<synthetic> def bar$default$1: [185]Int = [194]2
          }
        }
        
        % ack  makeTrans /Users/jason/code/scala3/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala
                val baseFunTransformed = atPos(baseFun.pos.makeTransparent) {
                      res.setPos(res.pos.makeTransparent)
                      val block = Block(stats ::: valDefs.flatten, res).setType(res.tpe).setPos(tree.pos.makeTransparent)
        
        Show
        Jason Zaugg added a comment - Here are some of the tools you need: scalac-hash origin/master -Xprint:typer -Yrangepos -Xprint-pos sandbox/test.scala [info] downloading http://scala-webapps.epfl.ch/artifacts/2c1e9af40e252bd9ed5bfe4305991ee6f7d25636/pack.tgz ...done. [info] scala revision from 2013-10-29 05:39:42 -0700 downloaded to /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af Password: [info] origin/mas => /Users/jason/usr/scala-v2.11.0-M6-134-g2c1e9af [[syntax trees at end of typer]] // test.scala [0:201]package [8:11]foo { [14:160]class Foo extends [24:160][163]scala.AnyRef { [163]def <init>(): [24]foo.Foo = [163]{ [163][163][163]Foo.super.<init>(); [24]() }; [28:158]def foo(): [32]Unit = <44:59>{ [44:53]<artifact> val qual$1: [44]foo.Bar = [44:53][44:53][44:53]new [48:51]Bar(); [54]<artifact> val x$1: [54]Int = [54]qual$1.bar$default$1; <44:59><44:57>qual$1.bar([54]x$1) } }; [163:201]class Bar extends [173:201][201]scala.AnyRef { [201]def <init>(): [173]foo.Bar = [201]{ [201][201][201]Bar.super.<init>(); [173]() }; [177:199]def bar([185:195]b: [188:191]<type: [188:191]scala.Int> = [194:195]2): [181]Unit = [197:199](); [185]<synthetic> def bar$default$1: [185]Int = [194]2 } } % ack makeTrans /Users/jason/code/scala3/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala val baseFunTransformed = atPos(baseFun.pos.makeTransparent) { res.setPos(res.pos.makeTransparent) val block = Block(stats ::: valDefs.flatten, res).setType(res.tpe).setPos(tree.pos.makeTransparent)
        Hide
        Iulian Dragos added a comment -

        `scalac-hash` looks like a very handy script...

        Show
        Iulian Dragos added a comment - `scalac-hash` looks like a very handy script...
        Hide
        Jason Zaugg added a comment -
        Show
        Jason Zaugg added a comment - https://github.com/paulp/libscala
        Hide
        Mirko Stocker added a comment -

        Great! This might also fix SI-4287.

        Show
        Mirko Stocker added a comment - Great! This might also fix SI-4287 .
        Show
        Mirco Dotta added a comment - Fixed in https://github.com/scala/scala/pull/3136 Backport to 2.10.x https://github.com/scala/scala/pull/3157
        Hide
        Mirco Dotta added a comment -

        @MirkoStocker Unfortunately no, my fix doesn't touch default argument in constructors.

        Show
        Mirco Dotta added a comment - @MirkoStocker Unfortunately no, my fix doesn't touch default argument in constructors.
        Hide
        Mirko Stocker added a comment -

        Oh, too bad, but I have faith in you

        Show
        Mirko Stocker added a comment - Oh, too bad, but I have faith in you
        Show
        James Iry added a comment - https://github.com/scala/scala/pull/3157

          People

          • Assignee:
            Mirco Dotta
            Reporter:
            Mirco Dotta
          • Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development