Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

SI-7343 Fixed phase ordering in specialization

Specialization rewires class parents during info transformation, and
the new info then guides the tree changes. But if a symbol is created
during duplication, which runs after specialization, its info is not
visited and thus the corresponding tree is not specialized.

One manifestation is the following:
```
object Test {

  class Parent[@specialized(Int) T]

  def spec_method[@specialized(Int) T](t: T, expectedXSuper: String) = {
     class X extends Parent[T]()
     // even in the specialized variant, the local X class
     // doesn't extend Parent$mcI$sp, since its symbol has
     // been created after specialization and was not seen
     // by specialzation's info transformer.
     ...
  }
}
```
We can fix this by forcing duplication to take place before specialization.

Review by @dragos, @paulp or @axel22.
  • Loading branch information...
commit c43b504ac1d843a580683a78eca0cb55bb427c15 1 parent c0ba5eb
@VladUreche VladUreche authored
View
32 src/compiler/scala/tools/nsc/transform/SpecializeTypes.scala
@@ -1264,7 +1264,35 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
}
protected override def newBodyDuplicator(context: Context) = new BodyDuplicator(context)
+ }
+ /** Introduced to fix SI-7343: Phase ordering problem between Duplicators and Specialization.
+ * brief explanation: specialization rewires class parents during info transformation, and
+ * the new info then guides the tree changes. But if a symbol is created during duplication,
+ * which runs after specialization, its info is not visited and thus the corresponding tree
+ * is not specialized. One manifestation is the following:
+ * ```
+ * object Test {
+ * class Parent[@specialized(Int) T]
+ *
+ * def spec_method[@specialized(Int) T](t: T, expectedXSuper: String) = {
+ * class X extends Parent[T]()
+ * // even in the specialized variant, the local X class
+ * // doesn't extend Parent$mcI$sp, since its symbol has
+ * // been created after specialization and was not seen
+ * // by specialzation's info transformer.
+ * ...
+ * }
+ * }
+ * ```
+ * We fix this by forcing duplication to take place before specialization.
+ *
+ * Note: The constructors phase (which also uses duplication) comes after erasure and uses the
+ * post-erasure typer => we must protect it from the beforeSpecialization phase shifting.
+ */
+ class SpecializationDuplicator(casts: Map[Symbol, Type]) extends Duplicator(casts) {
+ override def retyped(context: Context, tree: Tree, oldThis: Symbol, newThis: Symbol, env: scala.collection.Map[Symbol, Type]): Tree =
+ beforeSpecialize(super.retyped(context, tree, oldThis, newThis, env))
}
/** A tree symbol substituter that substitutes on type skolems.
@@ -1637,7 +1665,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
val tree1 = deriveValDef(tree)(_ => body(symbol.alias).duplicate)
debuglog("now typing: " + tree1 + " in " + tree.symbol.owner.fullName)
- val d = new Duplicator(emptyEnv)
+ val d = new SpecializationDuplicator(emptyEnv)
val newValDef = d.retyped(
localTyper.context1.asInstanceOf[d.Context],
tree1,
@@ -1664,7 +1692,7 @@ abstract class SpecializeTypes extends InfoTransform with TypingTransformers {
val symbol = tree.symbol
val meth = addBody(tree, source)
- val d = new Duplicator(castmap)
+ val d = new SpecializationDuplicator(castmap)
debuglog("-->d DUPLICATING: " + meth)
d.retyped(
localTyper.context1.asInstanceOf[d.Context],
View
1  src/compiler/scala/tools/nsc/typechecker/Duplicators.scala
@@ -37,6 +37,7 @@ abstract class Duplicators extends Analyzer {
envSubstitution = new SubstSkolemsTypeMap(env.keysIterator.toList, env.valuesIterator.toList)
debuglog("retyped with env: " + env)
+
newBodyDuplicator(context).typed(tree)
}
View
55 test/files/specialized/SI-7343.scala
@@ -0,0 +1,55 @@
+class Parent[@specialized(Int) T]
+
+object Test extends App {
+
+ /**
+ * This method will check if specialization is correctly rewiring parents
+ * for classes defined inside methods. The pattern is important since this
+ * is how closures are currently represented: as locally-defined anonymous
+ * classes, which usually end up inside methods. For these closures we do
+ * want their parents rewired correctly:
+ *
+ * ```
+ * def checkSuperClass$mIc$sp[T](t: T, ...) = {
+ * class X extends Parent$mcI$sp // instead of just Parent
+ * ...
+ * }
+ */
+ def checkSuperClass[@specialized(Int) T](t: T, expectedXSuper: String) = {
+ // test target:
+ // - in checkSuperClass, X should extend Parent
+ // - in checkSuperClass$mIc$sp, X should extend Parent$mcI$sp
+ class X extends Parent[T]()
+
+ // get the superclass for X and make sure it's correct
+ val actualXSuper = (new X).getClass().getSuperclass().getSimpleName()
+ assert(actualXSuper == expectedXSuper, actualXSuper + " != " + expectedXSuper)
+ }
+
+ checkSuperClass("x", "Parent")
+ checkSuperClass(101, "Parent$mcI$sp")
+
+ /**
+ * This is the same check, but in value. It should work exactly the same
+ * as its method counterpart.
+ */
+ class Val[@specialized(Int) T](t: T, expectedXSuper: String) {
+ val check: T = {
+ class X extends Parent[T]()
+
+ // get the superclass for X and make sure it's correct
+ val actualXSuper = (new X).getClass().getSuperclass().getSimpleName()
+ assert(actualXSuper == expectedXSuper, actualXSuper + " != " + expectedXSuper)
+ t
+ }
+ }
+
+ new Val("x", "Parent")
+ new Val(101, "Parent$mcI$sp")
+
+ /**
+ * NOTE: The the same check, only modified to affect constructors, won't
+ * work since the class X definition will always be lifted to become a
+ * member of the class, making it impossible to force its duplication.
+ */
+}
View
2  test/files/specialized/spec-ame.check
@@ -1,3 +1,3 @@
abc
10
-3
+2
View
3  test/files/specialized/spec-ame.scala
@@ -13,6 +13,9 @@ object Test {
def main(args: Array[String]) {
println((new A("abc")).foo.value)
println((new A(10)).foo.value)
+ // before fixing SI-7343, this was printing 3. Now it's printing 2,
+ // since the anonymous class created by doing new B[T] { ... } when
+ // T = Int is now rewired to B$mcI$sp instead of just B[Int]
println(runtime.BoxesRunTime.integerBoxCount)
}
}
Please sign in to comment.
Something went wrong with that request. Please try again.