Specialize erasure of `synchronized` primitive method

The goal is to avoid emitting unneeded `BoxedUnit` values,
which are the result of adapting a `Unit`-typed expression
inside a `synchronized(...)` to the erased type of
`synchronized`'s argument -- `Object`.

The proposed solution gives `synchronized` a polymorphic
type (the info of the type param is still erased so that
bounds checking works in the erased type system), so that
an application `synchronized(println("boo"))` erases to
`synchronized[Unit])(println("boo"))`, and no boxing is
performed on the `println("boo")` argument, whose expected
type is now `Unit` instead of `Object`.

5 files changed, 33 insertions, 30 deletions

diff --git a/src/compiler/scala/tools/nsc/transform/CleanUp.scala b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
index 0fb6213d36..81df28bc87 100644
--- a/src/compiler/scala/tools/nsc/transform/CleanUp.scala
+++ b/src/compiler/scala/tools/nsc/transform/CleanUp.scala
@@ -456,6 +456,11 @@ abstract class CleanUp extends Statics with Transform with ast.TreeDSL {
 
         super.transform(treeCopy.ApplyDynamic(tree, atPos(fn.pos)(Ident(SymbolLiteral_dummy).setType(SymbolLiteral_dummy.info)), LIT(SymbolLiteral_bootstrap) :: arg :: Nil))
 
+      // Drop the TypeApply, which was used in Erasure to make `synchronized { ... } ` erase like `...`
+      // (and to avoid boxing the argument to the polymorphic `synchronized` method).
+      case app@Apply(TypeApply(fun, _), args) if fun.symbol == Object_synchronized =>
+        super.transform(treeCopy.Apply(app, fun, args))
+
       // Replaces `Array(Predef.wrapArray(ArrayValue(...).$asInstanceOf[...]), <tag>)`
       // with just `ArrayValue(...).$asInstanceOf[...]`
       //
diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala
index eecd52546c..69240b07a1 100644
--- a/src/compiler/scala/tools/nsc/transform/Erasure.scala
+++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala
@@ -1117,7 +1117,8 @@ abstract class Erasure extends InfoTransform
 
         case TypeApply(fun, args) if (fun.symbol.owner != AnyClass &&
                                       fun.symbol != Object_asInstanceOf &&
-                                      fun.symbol != Object_isInstanceOf) =>
+                                      fun.symbol != Object_isInstanceOf &&
+                                      fun.symbol != Object_synchronized) =>
           // leave all other type tests/type casts, remove all other type applications
           preErase(fun)
 
@@ -1194,7 +1195,7 @@ abstract class Erasure extends InfoTransform
         else {
           val tree1 = preErase(tree)
           tree1 match {
-            case TypeApply(fun, targs @ List(targ)) if fun.symbol == Any_asInstanceOf && targ.tpe == UnitTpe =>
+            case TypeApply(fun, targs @ List(targ)) if (fun.symbol == Any_asInstanceOf  || fun.symbol == Object_synchronized) && targ.tpe == UnitTpe =>
               // SI-9066 prevent transforming `o.asInstanceOf[Unit]` to `o.asInstanceOf[BoxedUnit]`.
               // adaptMember will then replace the call by a reference to BoxedUnit.UNIT.
               treeCopy.TypeApply(tree1, transform(fun), targs).clearType()
diff --git a/src/reflect/scala/reflect/internal/transform/Erasure.scala b/src/reflect/scala/reflect/internal/transform/Erasure.scala
index a9e26c0f7d..e2f1e74740 100644
--- a/src/reflect/scala/reflect/internal/transform/Erasure.scala
+++ b/src/reflect/scala/reflect/internal/transform/Erasure.scala
@@ -342,23 +342,30 @@ trait Erasure {
     }
   }
 
-  /**  The symbol's erased info. This is the type's erasure, except for the following symbols:
-   *
-   *   - For $asInstanceOf      : [T]T
-   *   - For $isInstanceOf      : [T]scala#Boolean
-   *   - For class Array        : [T]C where C is the erased classinfo of the Array class.
-   *   - For Array[T].<init>    : {scala#Int)Array[T]
-   *   - For a type parameter   : A type bounds type consisting of the erasures of its bounds.
-   */
+  /** The symbol's erased info. This is the type's erasure, except for the following primitive symbols:
+    *
+    *   - $asInstanceOf    --> [T]T
+    *   - $isInstanceOf    --> [T]scala#Boolean
+    *   - synchronized     --> [T](x: T)T
+    *   - class Array      --> [T]C where C is the erased classinfo of the Array class.
+    *   - Array[T].<init>  --> {scala#Int)Array[T]
+    *
+    * An abstract type's info erases to a TypeBounds type consisting of the erasures of the abstract type's bounds.
+    */
   def transformInfo(sym: Symbol, tp: Type): Type = {
-    if (sym == Object_asInstanceOf)
+    // Do not erase the primitive `synchronized` method's info or the info of its parameter.
+    // We do erase the info of its type param so that subtyping can relate its bounds after erasure.
+    def synchronizedPrimitive(sym: Symbol) =
+      sym == Object_synchronized || (sym.owner == Object_synchronized && sym.isTerm)
+
+    if (sym == Object_asInstanceOf || synchronizedPrimitive(sym))
       sym.info
     else if (sym == Object_isInstanceOf || sym == ArrayClass)
       PolyType(sym.info.typeParams, specialErasure(sym)(sym.info.resultType))
     else if (sym.isAbstractType)
-      TypeBounds(WildcardType, WildcardType)
+      TypeBounds(WildcardType, WildcardType) // TODO why not use the erasure of the type's bounds, as stated in the doc?
     else if (sym.isTerm && sym.owner == ArrayClass) {
-      if (sym.isClassConstructor)
+      if (sym.isClassConstructor) // TODO: switch on name for all branches -- this one is sym.name == nme.CONSTRUCTOR
         tp match {
           case MethodType(params, TypeRef(pre, sym1, args)) =>
             MethodType(cloneSymbolsAndModify(params, specialErasure(sym)),
@@ -375,12 +382,14 @@ trait Erasure {
     } else if (
       sym.owner != NoSymbol &&
       sym.owner.owner == ArrayClass &&
-      sym == Array_update.paramss.head(1)) {
+      sym == Array_update.paramss.head(1)) { // TODO: can we simplify the guard, perhaps cache the symbol to compare to?
       // special case for Array.update: the non-erased type remains, i.e. (Int,A)Unit
       // since the erasure type map gets applied to every symbol, we have to catch the
       // symbol here
       tp
     } else {
+      // TODO OPT: altogether, there are 9 symbols that we special-case.
+      // Could we get to the common case more quickly by looking them up in a set?
       specialErasure(sym)(tp)
     }
   }
diff --git a/test/files/run/delambdafy_t6028.check b/test/files/run/delambdafy_t6028.check
index 7bd68c78e9..eaba70ee1a 100644
--- a/test/files/run/delambdafy_t6028.check
+++ b/test/files/run/delambdafy_t6028.check
@@ -44,14 +44,8 @@ package <empty> {
     };
     final <stable> private[this] def MethodLocalObject$1(barParam$1: String, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
       if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
-        {
-          T.this.synchronized({
-            if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
-              MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
-            scala.runtime.BoxedUnit.UNIT
-          });
-          ()
-        };
+        T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+          MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1));
       MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
     };
     final <artifact> private[this] def $anonfun$tryy$1(tryyParam$1: String, tryyLocal$1: runtime.ObjectRef): Unit = try {
diff --git a/test/files/run/t6028.check b/test/files/run/t6028.check
index f757bc93ff..d6cc452bbf 100644
--- a/test/files/run/t6028.check
+++ b/test/files/run/t6028.check
@@ -56,14 +56,8 @@ package <empty> {
     };
     final <stable> private[this] def MethodLocalObject$1(barParam$1: Int, MethodLocalObject$module$1: runtime.VolatileObjectRef): T#MethodLocalObject$2.type = {
       if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
-        {
-          T.this.synchronized({
-            if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
-              MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1);
-            scala.runtime.BoxedUnit.UNIT
-          });
-          ()
-        };
+        T.this.synchronized[Unit](if (MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]().eq(null))
+          MethodLocalObject$module$1.elem = new T#MethodLocalObject$2.type(T.this, barParam$1));
       MethodLocalObject$module$1.elem.$asInstanceOf[T#MethodLocalObject$2.type]()
     };
     @SerialVersionUID(value = 0) final <synthetic> class $anonfun$tryy$1 extends scala.runtime.AbstractFunction0$mcV$sp with Serializable {