summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul Phillips <paulp@improving.org>2013-09-04 08:22:27 -0700
committerPaul Phillips <paulp@improving.org>2013-09-04 09:34:04 -0700
commitc58b7b10249adefa1045942a1dc7a55dc5932db8 (patch)
tree0808b4a7225eb7bd9d30fd4d937344b2ebe8c40d
parenta8c05274f738943ae58ecefda4b012b9daf5d8dc (diff)
downloadscala-c58b7b10249adefa1045942a1dc7a55dc5932db8.tar.gz
scala-c58b7b10249adefa1045942a1dc7a55dc5932db8.tar.bz2
scala-c58b7b10249adefa1045942a1dc7a55dc5932db8.zip
Eliminate TypeTrees with null original.
This is a retry of #2801 after figuring out the range position error. Should there be anyone out there who compiles with -Xdev, know that this commit eliminates the 1406 errors one presently incurs compiling src/library. A val declared in source code receives only one tree from the parser, but two are needed - one for the field and one for the getter. I discovered long ago that if the val had an existential type, this was creating issues with incompatible existentials between the field and the getter. However the remedy for that did not take into account the whole of the wide range of super subtle issues which accompany tree duplication. In particular, the duplicated tree must be given not only a fresh TypeTree(), but that TypeTree cannot share the same original without running afoul of range position invariants. That's because typedTypeTree resurrects the original tree with whatever position it has - so the "original" needs to be a duplicate of the original with a focused position. Should the call to TypeTree.duplicate also duplicate the original? I think so, but I bequeath this question to others. This commit also eliminated some duplicate error messages, because duplicate suppression depends on the errors having the same position. See c478eb770d, 7a6fa80937 for previous related work.
-rw-r--r--src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala22
-rw-r--r--src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala17
-rw-r--r--test/files/neg/classmanifests_new_deprecations.check8
-rw-r--r--test/files/neg/t935.check5
-rw-r--r--test/files/run/existential-rangepos.check13
-rw-r--r--test/files/run/existential-rangepos.scala13
6 files changed, 43 insertions, 35 deletions
diff --git a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
index 48263a496e..d6a6e027cb 100644
--- a/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
+++ b/src/compiler/scala/tools/nsc/transform/ExplicitOuter.scala
@@ -373,16 +373,10 @@ abstract class ExplicitOuter extends InfoTransform
/** The definition tree of the outer accessor of current class
*/
- def outerAccessorDef: Tree = {
- val outerAcc = outerAccessor(currentClass)
- val rhs: Tree =
- if (outerAcc.isDeferred) EmptyTree
- else This(currentClass) DOT outerField(currentClass)
-
- /* If we don't re-type the tree, we see self-type related crashes like #266.
- */
- localTyper typed {
- (DEF(outerAcc) withPos currentClass.pos withType null) === rhs
+ def outerAccessorDef: Tree = localTyper typed {
+ outerAccessor(currentClass) match {
+ case acc if acc.isDeferred => DefDef(acc, EmptyTree)
+ case acc => DefDef(acc, Select(This(currentClass), outerField(currentClass)))
}
}
@@ -404,12 +398,8 @@ abstract class ExplicitOuter extends InfoTransform
else if (mixinPrefix.typeArgs.nonEmpty) gen.mkAttributedThis(mixinPrefix.typeSymbol)
else gen.mkAttributedQualifier(mixinPrefix)
)
- localTyper typed {
- (DEF(outerAcc) withPos currentClass.pos) === {
- // Need to cast for nested outer refs in presence of self-types. See ticket #3274.
- gen.mkCast(transformer.transform(path), outerAcc.info.resultType)
- }
- }
+ // Need to cast for nested outer refs in presence of self-types. See ticket #3274.
+ localTyper typed DefDef(outerAcc, gen.mkCast(transformer.transform(path), outerAcc.info.resultType))
}
/** The main transformation method */
diff --git a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
index 3a5845c8ca..263b5ad784 100644
--- a/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
+++ b/src/compiler/scala/tools/nsc/typechecker/MethodSynthesis.scala
@@ -397,6 +397,12 @@ trait MethodSynthesis {
if (mods.isDeferred) basisSym
else basisSym.getter(enclClass)
)
+ // Range position errors ensue if we don't duplicate this in some
+ // circumstances (at least: concrete vals with existential types.)
+ private def tptOriginal = (
+ if (mods.isDeferred) tree.tpt // keep type tree of original abstract field
+ else tree.tpt.duplicate setPos tree.tpt.pos.focus // focused position of original tpt
+ )
override def derivedTree: DefDef = {
// For existentials, don't specify a type for the getter, even one derived
@@ -407,16 +413,11 @@ trait MethodSynthesis {
// starts compiling (instead of failing like it's supposed to) because the typer
// expects to be able to identify escaping locals in typedDefDef, and fails to
// spot that brand of them. In other words it's an artifact of the implementation.
- val tpt = derivedSym.tpe.finalResultType match {
+ val tpt = atPos(derivedSym.pos.focus)(derivedSym.tpe.finalResultType match {
case ExistentialType(_, _) => TypeTree()
case _ if mods.isDeferred => TypeTree()
case tp => TypeTree(tp)
- }
- tpt setPos derivedSym.pos.focus
- // keep type tree of original abstract field
- if (mods.isDeferred)
- tpt setOriginal tree.tpt
-
+ })
// TODO - reconcile this with the DefDef creator in Trees (which
// at this writing presented no way to pass a tree in for tpt.)
atPos(derivedSym.pos) {
@@ -425,7 +426,7 @@ trait MethodSynthesis {
derivedSym.name.toTermName,
Nil,
Nil,
- tpt,
+ tpt setOriginal tptOriginal,
if (mods.isDeferred) EmptyTree else fieldSelection
) setSymbol derivedSym
}
diff --git a/test/files/neg/classmanifests_new_deprecations.check b/test/files/neg/classmanifests_new_deprecations.check
index 5f9d0a1ccc..fd1e2728c3 100644
--- a/test/files/neg/classmanifests_new_deprecations.check
+++ b/test/files/neg/classmanifests_new_deprecations.check
@@ -7,9 +7,6 @@ classmanifests_new_deprecations.scala:3: warning: type ClassManifest in object P
classmanifests_new_deprecations.scala:4: warning: type ClassManifest in object Predef is deprecated: Use `scala.reflect.ClassTag` instead
val cm3: ClassManifest[Int] = null
^
-classmanifests_new_deprecations.scala:4: warning: type ClassManifest in object Predef is deprecated: Use `scala.reflect.ClassTag` instead
- val cm3: ClassManifest[Int] = null
- ^
classmanifests_new_deprecations.scala:6: warning: type ClassManifest in package reflect is deprecated: Use scala.reflect.ClassTag instead
def rcm1[T: scala.reflect.ClassManifest] = ???
^
@@ -19,9 +16,6 @@ classmanifests_new_deprecations.scala:7: warning: type ClassManifest in package
classmanifests_new_deprecations.scala:8: warning: type ClassManifest in package reflect is deprecated: Use scala.reflect.ClassTag instead
val rcm3: scala.reflect.ClassManifest[Int] = null
^
-classmanifests_new_deprecations.scala:8: warning: type ClassManifest in package reflect is deprecated: Use scala.reflect.ClassTag instead
- val rcm3: scala.reflect.ClassManifest[Int] = null
- ^
classmanifests_new_deprecations.scala:10: warning: type ClassManifest in object Predef is deprecated: Use `scala.reflect.ClassTag` instead
type CM[T] = ClassManifest[T]
^
@@ -29,5 +23,5 @@ classmanifests_new_deprecations.scala:15: warning: type ClassManifest in package
type RCM[T] = scala.reflect.ClassManifest[T]
^
error: No warnings can be incurred under -Xfatal-warnings.
-10 warnings found
+8 warnings found
one error found
diff --git a/test/files/neg/t935.check b/test/files/neg/t935.check
index 8b73700187..af634a2630 100644
--- a/test/files/neg/t935.check
+++ b/test/files/neg/t935.check
@@ -4,7 +4,4 @@ t935.scala:7: error: type arguments [Test3.B] do not conform to class E's type p
t935.scala:13: error: type arguments [Test4.B] do not conform to class E's type parameter bounds [T <: String]
val b: String @E[B](new B) = "hi"
^
-t935.scala:13: error: type arguments [Test4.B] do not conform to class E's type parameter bounds [T <: String]
- val b: String @E[B](new B) = "hi"
- ^
-three errors found
+two errors found
diff --git a/test/files/run/existential-rangepos.check b/test/files/run/existential-rangepos.check
new file mode 100644
index 0000000000..1212b60bae
--- /dev/null
+++ b/test/files/run/existential-rangepos.check
@@ -0,0 +1,13 @@
+[[syntax trees at end of patmat]] // newSource1.scala
+[0:76]package [0:0]<empty> {
+ [0:76]abstract class A[[17:18]T[17:18]] extends [20:76][76]scala.AnyRef {
+ [76]def <init>(): [20]A[T] = [76]{
+ [76][76][76]A.super.<init>();
+ [20]()
+ };
+ [24:51]private[this] val foo: [28]Set[_ <: T] = [47:51]null;
+ [28]<stable> <accessor> def foo: [28]Set[_ <: T] = [28][28]A.this.foo;
+ [54:74]<stable> <accessor> def bar: [58]Set[_ <: T]
+ }
+}
+
diff --git a/test/files/run/existential-rangepos.scala b/test/files/run/existential-rangepos.scala
new file mode 100644
index 0000000000..7d2b0810d3
--- /dev/null
+++ b/test/files/run/existential-rangepos.scala
@@ -0,0 +1,13 @@
+import scala.tools.partest._
+
+object Test extends DirectTest {
+ override def extraSettings: String = "-usejavacp -Yrangepos -Xprint:patmat -Xprint-pos -d " + testOutput.path
+
+ override def code = """
+abstract class A[T] {
+ val foo: Set[_ <: T] = null
+ val bar: Set[_ <: T]
+}""".trim
+
+ override def show(): Unit = Console.withErr(System.out)(compile())
+}