Skip to content

Fixed positions in de-aliased special symbols.. #1251

Merged
merged 1 commit into from Sep 4, 2012
View
4 src/compiler/scala/tools/nsc/typechecker/Typers.scala
@@ -579,7 +579,7 @@ trait Typers extends Modes with Adaptations with Tags {
// to notice exhaustiveness and to generate good code when
// List extractors are mixed with :: patterns. See Test5 in lists.scala.
def dealias(sym: Symbol) =
- (atPos(tree.pos) {gen.mkAttributedRef(sym)}, sym.owner.thisType)
+ (atPos(tree.pos.makeTransparent) {gen.mkAttributedRef(sym)} setPos tree.pos, sym.owner.thisType)
@paulp
paulp added a note Sep 4, 2012

This code again! Will this hack never tire of creating new issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
sym.name match {
case nme.List => return dealias(ListModule)
case nme.Seq => return dealias(SeqModule)
@@ -1057,7 +1057,7 @@ trait Typers extends Modes with Adaptations with Tags {
case other =>
other
}
- typed(atPos(tree.pos)(Select(qual, nme.apply)), mode, pt)
+ typed(atPos(tree.pos)(Select(qual setPos tree.pos.makeTransparent, nme.apply)), mode, pt)
@misto
misto added a note Sep 11, 2012

Why is qual getting the transparent position? Shouldn't it be the Select, like Select(qual, nme.apply) setPos tree.pos.makeTransparent

@dragos
dragos added a note Sep 11, 2012

It makes sense, but if the apply was transparent there would be no way to get to that tree.

With the current solution, one can locate the Apply(qual.. tree, and it has access to both the qualifier and selection. This allowed the IDE to offer hyperlinking to both List.apply and List on that position.

If we make the apply transparent, one will always retrieve the qualifier. Since trees have no way to get to their parent, there would be no way to hyperlink or get to the apply node anymore.

Does this solution make your life much harder?

@misto
misto added a note Sep 11, 2012

Ok, I see that this is convenient for the IDE. If it's just for apply, then it should be easy to have a workaround in the refactoring library. It just doesn't feel like a good solution, abusing TransparentPositions like this :)

So, what about unapply calls? Shouldn't they get the same treatment?

@dragos
dragos added a note Sep 11, 2012

I'm open to better solutions, or I can remove this change if you prefer.

I think unapplies are translated after type-checking (there's an UnApply node)..

@misto
misto added a note Sep 11, 2012

I just checked what happens to unapply. The UnApply tree in a pattern has an Apply with an Extractor.unapply Select. Here the Extractor Ident simply has the same range position as the Extractor.unapply Select. To me that also doesn't look right..

I'm not sure what to do, in my understanding of positions, both apply and unapply should be transparent positions and their underlying Ident or Select trees should have a range position. But if that prevents the IDE from hyperlinking directly to the definition of apply, then maybe we should just leave this as it is now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
}
// begin adapt
View
25 test/files/run/t5064.check
@@ -0,0 +1,25 @@
+[12] T5064.super.<init>()
+[12] T5064.super.<init>
+[12] this
+[16:23] immutable.this.List.apply(scala.this.Predef.wrapIntArray(Array[Int]{1}))
+[16:20] immutable.this.List.apply
+<16:20> immutable.this.List
+<16:20> immutable.this
+[16:23] scala.this.Predef.wrapIntArray(Array[Int]{1})
+[20] scala.this.Predef.wrapIntArray
+[20] scala.this.Predef
+[20] scala.this
+[26:32] collection.this.Seq.apply(scala.this.Predef.wrapIntArray(Array[Int]{1}))
+[26:29] collection.this.Seq.apply
+<26:29> collection.this.Seq
+<26:29> collection.this
+[26:32] scala.this.Predef.wrapIntArray(Array[Int]{1})
+[29] scala.this.Predef.wrapIntArray
+[29] scala.this.Predef
+[29] scala.this
+[35:39] immutable.this.List
+<35:39> immutable.this
+[42:45] collection.this.Seq
+<42:45> collection.this
+[48:51] immutable.this.Nil
+<48:51> immutable.this
View
23 test/files/run/t5064.scala
@@ -0,0 +1,23 @@
+import scala.tools.partest._
+
+object Test extends CompilerTest {
+ import global._
+ override def extraSettings = super.extraSettings + " -Yrangepos"
+ override def sources = List(
+ """|class T5064 {
+ | List(1)
+ | Seq(1)
+ | List
+ | Seq
+ | Nil
+ |}""".stripMargin
+ )
+ def check(source: String, unit: CompilationUnit) {
+ for (ClassDef(_, _, _, Template(_, _, stats)) <- unit.body ; stat <- stats ; t <- stat) {
+ t match {
+ case _: Select | _: Apply | _: This => println("%-15s %s".format(t.pos.show, t))
+ case _ =>
+ }
+ }
+ }
+}
Something went wrong with that request. Please try again.