diff options
author | Mirco Dotta <mirco.dotta@typesafe.com> | 2013-10-16 13:57:13 +0200 |
---|---|---|
committer | Mirco Dotta <mirco.dotta@typesafe.com> | 2013-11-15 14:28:09 +0100 |
commit | 3009a525b58a4c7865ff524899b85518884ee5f7 (patch) | |
tree | b9932a0a0ee5800beb7dc27e223fcbf13de37164 | |
parent | 69e62de87d80cf5b9d8c7f4eefcea0638fb2759d (diff) | |
download | scala-3009a525b58a4c7865ff524899b85518884ee5f7.tar.gz scala-3009a525b58a4c7865ff524899b85518884ee5f7.tar.bz2 scala-3009a525b58a4c7865ff524899b85518884ee5f7.zip |
SI-7915 Corrected range positions created during default args expansion
The tree created during expansion of default arguments contained trees with the
wrong type of positions. Let's discuss this with an example. Below is the tree
generated for the `foo` method in the test class included in this commit.
Before this commit:
```
[54:94]def foo(): [58]Unit = <70:90>{
[70:79]<artifact> val qual$1: [70]Bar = [70:79][70:79][70:79]new [74:77]Bar();
[80]<artifact> val x$1: [80]Int = [80]qual$1.bar$default$1;
<70:90><70:83>qual$1.bar([80]x$1)
}
```
Now:
```
[54:99]def foo(): [58]Unit = <70:95>{
<70:84><artifact> val qual$1: [70]Bar = [70:84][70:84][70:84]new [74:77]Bar();
[85]<artifact> val x$1: [85]Int = [85]qual$1.bar$default$1;
<70:95>[84:88]qual$1.bar([85]x$1)
}
```
Here are the list of changes:
* The synthetic `qual$1` has a transparent position, instead of a range position.
* The new Select tree (i.e., `qual$1.bar`) should always have a range position,
because `selected` (i.e., the called method) is always visible in the source
(in fact, this is the whole point of the fix, we need a range position or
hyperlinking request from the Scala IDE won't work).
* The Block that contains the expanded default arguments is forced to have a
transparent position, as it never exist in the original source.
The tricky part of the fix is the position assigned to the new Select tree,
which needs to respect the range position's invariants. In the specific case,
we ought to make sure that range positions don't overlap. Therefore, the position
assigned to the new Select tree is computed by intersecting the original Select
position (i.e., `baseFun`'s position) and the original qualifier's position
(i.e., `qual`'s position).
If you take a closer look at the range positions assigned in the tree after this
commit, you'll notice that the range position of the `qual$1`'s rhs (i.e.,
[70:84]), and `qual$1.bar` (i.e., [84:88]) might seem to overlap, because the
former ends where the latter begins. However, this not the case because of the
range position's invariant 2, which states:
> Invariant 2: in a range position, start <= point < end
Hence, the above two positions aren't overlapping as far as the compiler is
concerned.
One additional aspect (that may look like a detail) is that we make sure to
never generate a position such that its start is after its end. This is why we
take the position with the smallest end point.
Furthermore, calling `withStart` would turn any position in a range position,
which isn't desiderable in general (and, even worse, this can lead to
generation of invalid positions - bad offsets - if the computation is performed
on offset positions). Hence, the position's computation is only performed when
both `baseFun` and `qual` positions are range positions. Indeed, I expect this to
be always the case if the compiler is started with -Yrangepos.
-rw-r--r-- | src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala | 9 | ||||
-rw-r--r-- | test/files/presentation/t7915.check | 11 | ||||
-rw-r--r-- | test/files/presentation/t7915/Test.scala | 8 | ||||
-rw-r--r-- | test/files/presentation/t7915/src/Foo.scala | 9 |
4 files changed, 34 insertions, 3 deletions
diff --git a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala index 03aad71165..46ff98875f 100644 --- a/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala +++ b/src/compiler/scala/tools/nsc/typechecker/NamesDefaults.scala @@ -162,7 +162,7 @@ trait NamesDefaults { self: Analyzer => // never used for constructor calls, they always have a stable qualifier def blockWithQualifier(qual: Tree, selected: Name) = { - val sym = blockTyper.context.owner.newValue(unit.freshTermName("qual$"), qual.pos, newFlags = ARTIFACT) setInfo uncheckedBounds(qual.tpe) + val sym = blockTyper.context.owner.newValue(unit.freshTermName("qual$"), newFlags = ARTIFACT) setInfo uncheckedBounds(qual.tpe) setPos (qual.pos.makeTransparent) blockTyper.context.scope enter sym val vd = atPos(sym.pos)(ValDef(sym, qual) setType NoType) // it stays in Vegas: SI-5720, SI-5727 @@ -173,13 +173,16 @@ trait NamesDefaults { self: Analyzer => // setSymbol below is important because the 'selected' function might be overloaded. by // assigning the correct method symbol, typedSelect will just assign the type. the reason // to still call 'typed' is to correctly infer singleton types, SI-5259. - val f = blockTyper.typedOperator(Select(newQual, selected).setSymbol(baseFun1.symbol)) + val selectPos = + if(qual.pos.isRange && baseFun.pos.isRange) qual.pos.union(baseFun.pos).withStart(Math.min(qual.pos.end, baseFun.pos.end)) + else baseFun.pos + val f = blockTyper.typedOperator(Select(newQual, selected).setSymbol(baseFun1.symbol).setPos(selectPos)) if (funTargs.isEmpty) f else TypeApply(f, funTargs).setType(baseFun.tpe) } val b = Block(List(vd), baseFunTransformed) - .setType(baseFunTransformed.tpe).setPos(baseFun.pos) + .setType(baseFunTransformed.tpe).setPos(baseFun.pos.makeTransparent) context.namedApplyBlockInfo = Some((b, NamedApplyInfo(Some(newQual), defaultTargs, Nil, blockTyper))) b diff --git a/test/files/presentation/t7915.check b/test/files/presentation/t7915.check new file mode 100644 index 0000000000..b18b4ddb55 --- /dev/null +++ b/test/files/presentation/t7915.check @@ -0,0 +1,11 @@ +reload: Foo.scala + +askHyperlinkPos for `Bar` at (7,11) Foo.scala +================================================================================ +[response] found askHyperlinkPos for `Bar` at (1,7) Foo.scala +================================================================================ + +askHyperlinkPos for `bar` at (7,22) Foo.scala +================================================================================ +[response] found askHyperlinkPos for `bar` at (2,7) Foo.scala +================================================================================ diff --git a/test/files/presentation/t7915/Test.scala b/test/files/presentation/t7915/Test.scala new file mode 100644 index 0000000000..c2f89bdb17 --- /dev/null +++ b/test/files/presentation/t7915/Test.scala @@ -0,0 +1,8 @@ +import scala.tools.nsc.interactive.tests.InteractiveTest + +object Test extends InteractiveTest { + override def runDefaultTests() { + sourceFiles foreach (src => askLoadedTyped(src).get) + super.runDefaultTests() + } +} diff --git a/test/files/presentation/t7915/src/Foo.scala b/test/files/presentation/t7915/src/Foo.scala new file mode 100644 index 0000000000..a4166ae5b4 --- /dev/null +++ b/test/files/presentation/t7915/src/Foo.scala @@ -0,0 +1,9 @@ +class Bar { + def bar(b: Int = 2) {} +} + +class Foo { + def foo() { + new Bar/*#*/().bar/*#*/() + } +} |