From 9f142b114be1261f55ea82b9cd1f9d9f91b3cad6 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Wed, 29 Jan 2014 15:59:06 +0100 Subject: SI-6260 Avoid double-def error with lambdas over value classes Post-erasure of value classs in method signatures to the underlying type wreaks havoc when the erased signature overlaps with the generic signature from an overriden method. There just isn't room for both. But we *really* need both; callers to the interface method will be passing boxed values that the bridge needs to unbox and pass to the specific method that accepts unboxed values. This most commonly turns up with value classes that erase to Object that are used as the parameter or the return type of an anonymous function. This was thought to have been intractable, unless we chose a different name for the unboxed, specific method in the subclass. But that sounds like a big task that would require call-site rewriting, ala specialization. But there is an important special case in which we don't need to rewrite call sites. If the class defining the method is anonymous, there is actually no need for the unboxed method; it will *only* ever be called via the generic method. I came to this realisation when looking at how Java 8 lambdas are handled. I was expecting bridge methods, but found none. The lambda body is placed directly in a method exactly matching the generic signature. This commit detects the clash between bridge and target, and recovers for anonymous classes by mangling the name of the target method's symbol. This is used as the bytecode name. The generic bridge forward to that, as before, with the requisite box/unbox operations. --- test/files/neg/delambdafy_t6260_method.check | 13 ------------- test/files/neg/delambdafy_t6260_method.flags | 1 - test/files/neg/delambdafy_t6260_method.scala | 17 ----------------- test/files/neg/t6260-named.check | 13 +++++++++++++ test/files/neg/t6260-named.scala | 15 +++++++++++++++ test/files/neg/t6260.check | 13 ------------- test/files/neg/t6260.flags | 1 - test/files/neg/t6260.scala | 17 ----------------- test/files/neg/t6260b.check | 7 ------- test/files/neg/t6260b.scala | 3 --- test/files/pos/delambdafy_t6260_method.check | 13 +++++++++++++ test/files/pos/delambdafy_t6260_method.flags | 1 + test/files/pos/delambdafy_t6260_method.scala | 17 +++++++++++++++++ test/files/pos/t6260.flags | 1 + test/files/pos/t6260.scala | 17 +++++++++++++++++ test/files/pos/t6260b.scala | 3 +++ test/files/run/t6260-delambdafy.check | 4 ++++ test/files/run/t6260-delambdafy.flags | 1 + test/files/run/t6260-delambdafy.scala | 12 ++++++++++++ test/files/run/t6260c.check | 5 +++++ test/files/run/t6260c.scala | 17 +++++++++++++++++ 21 files changed, 119 insertions(+), 72 deletions(-) delete mode 100644 test/files/neg/delambdafy_t6260_method.check delete mode 100644 test/files/neg/delambdafy_t6260_method.flags delete mode 100644 test/files/neg/delambdafy_t6260_method.scala create mode 100644 test/files/neg/t6260-named.check create mode 100644 test/files/neg/t6260-named.scala delete mode 100644 test/files/neg/t6260.check delete mode 100644 test/files/neg/t6260.flags delete mode 100644 test/files/neg/t6260.scala delete mode 100644 test/files/neg/t6260b.check delete mode 100644 test/files/neg/t6260b.scala create mode 100644 test/files/pos/delambdafy_t6260_method.check create mode 100644 test/files/pos/delambdafy_t6260_method.flags create mode 100644 test/files/pos/delambdafy_t6260_method.scala create mode 100644 test/files/pos/t6260.flags create mode 100644 test/files/pos/t6260.scala create mode 100644 test/files/pos/t6260b.scala create mode 100644 test/files/run/t6260-delambdafy.check create mode 100644 test/files/run/t6260-delambdafy.flags create mode 100644 test/files/run/t6260-delambdafy.scala create mode 100644 test/files/run/t6260c.check create mode 100644 test/files/run/t6260c.scala (limited to 'test/files') diff --git a/test/files/neg/delambdafy_t6260_method.check b/test/files/neg/delambdafy_t6260_method.check deleted file mode 100644 index f5cd6947d1..0000000000 --- a/test/files/neg/delambdafy_t6260_method.check +++ /dev/null @@ -1,13 +0,0 @@ -delambdafy_t6260_method.scala:3: error: bridge generated for member method apply: (bx: Object)Object in class map$extension1 -which overrides method apply: (v1: Object)Object in trait Function1 -clashes with definition of the member itself; -both have erased type (bx: Object)Object - ((bx: Box[X]) => new Box(f(bx.x)))(this) - ^ -delambdafy_t6260_method.scala:8: error: bridge generated for member method apply: (bx: Object)Object in class map21 -which overrides method apply: (v1: Object)Object in trait Function1 -clashes with definition of the member itself; -both have erased type (bx: Object)Object - ((bx: Box[X]) => new Box(f(bx.x)))(self) - ^ -two errors found diff --git a/test/files/neg/delambdafy_t6260_method.flags b/test/files/neg/delambdafy_t6260_method.flags deleted file mode 100644 index 48b438ddf8..0000000000 --- a/test/files/neg/delambdafy_t6260_method.flags +++ /dev/null @@ -1 +0,0 @@ --Ydelambdafy:method diff --git a/test/files/neg/delambdafy_t6260_method.scala b/test/files/neg/delambdafy_t6260_method.scala deleted file mode 100644 index 93b5448227..0000000000 --- a/test/files/neg/delambdafy_t6260_method.scala +++ /dev/null @@ -1,17 +0,0 @@ -class Box[X](val x: X) extends AnyVal { - def map[Y](f: X => Y): Box[Y] = - ((bx: Box[X]) => new Box(f(bx.x)))(this) -} - -object Test { - def map2[X, Y](self: Box[X], f: X => Y): Box[Y] = - ((bx: Box[X]) => new Box(f(bx.x)))(self) - - def main(args: Array[String]) { - val f = (x: Int) => x + 1 - val g = (x: String) => x + x - - map2(new Box(42), f) - new Box("abc") map g - } -} diff --git a/test/files/neg/t6260-named.check b/test/files/neg/t6260-named.check new file mode 100644 index 0000000000..ed6ab5e76f --- /dev/null +++ b/test/files/neg/t6260-named.check @@ -0,0 +1,13 @@ +t6260-named.scala:12: error: bridge generated for member method apply: (a: C[Any])C[Any] in object O +which overrides method apply: (v1: T1)R in trait Function1 +clashes with definition of the member itself; +both have erased type (v1: Object)Object + def apply(a: C[Any]) = a + ^ +t6260-named.scala:14: error: bridge generated for member method apply: (a: C[Any])C[Any] in class X +which overrides method apply: (a: A)A in trait T +clashes with definition of the member itself; +both have erased type (a: Object)Object + class X extends T[C[Any]] { def apply(a: C[Any]) = a } + ^ +two errors found diff --git a/test/files/neg/t6260-named.scala b/test/files/neg/t6260-named.scala new file mode 100644 index 0000000000..7ce13476eb --- /dev/null +++ b/test/files/neg/t6260-named.scala @@ -0,0 +1,15 @@ +class C[A](private val a: Any) extends AnyVal +trait T[A] { + def apply(a: A): A +} + +object Test { + (x: C[Any]) => {println(s"f($x)"); x} // okay + new T[C[Any]] { def apply(a: C[Any]) = a } // okay + + // we can't rename the specific apply methid to avoid the clash + object O extends Function1[C[Any], C[Any]] { + def apply(a: C[Any]) = a + } + class X extends T[C[Any]] { def apply(a: C[Any]) = a } +} diff --git a/test/files/neg/t6260.check b/test/files/neg/t6260.check deleted file mode 100644 index 60c4add143..0000000000 --- a/test/files/neg/t6260.check +++ /dev/null @@ -1,13 +0,0 @@ -t6260.scala:3: error: bridge generated for member method apply: (bx: Box[X])Box[Y] in <$anon: Box[X] => Box[Y]> -which overrides method apply: (v1: T1)R in trait Function1 -clashes with definition of the member itself; -both have erased type (v1: Object)Object - ((bx: Box[X]) => new Box(f(bx.x)))(this) - ^ -t6260.scala:8: error: bridge generated for member method apply: (bx: Box[X])Box[Y] in <$anon: Box[X] => Box[Y]> -which overrides method apply: (v1: T1)R in trait Function1 -clashes with definition of the member itself; -both have erased type (v1: Object)Object - ((bx: Box[X]) => new Box(f(bx.x)))(self) - ^ -two errors found diff --git a/test/files/neg/t6260.flags b/test/files/neg/t6260.flags deleted file mode 100644 index 2349d8294d..0000000000 --- a/test/files/neg/t6260.flags +++ /dev/null @@ -1 +0,0 @@ --Ydelambdafy:inline diff --git a/test/files/neg/t6260.scala b/test/files/neg/t6260.scala deleted file mode 100644 index 93b5448227..0000000000 --- a/test/files/neg/t6260.scala +++ /dev/null @@ -1,17 +0,0 @@ -class Box[X](val x: X) extends AnyVal { - def map[Y](f: X => Y): Box[Y] = - ((bx: Box[X]) => new Box(f(bx.x)))(this) -} - -object Test { - def map2[X, Y](self: Box[X], f: X => Y): Box[Y] = - ((bx: Box[X]) => new Box(f(bx.x)))(self) - - def main(args: Array[String]) { - val f = (x: Int) => x + 1 - val g = (x: String) => x + x - - map2(new Box(42), f) - new Box("abc") map g - } -} diff --git a/test/files/neg/t6260b.check b/test/files/neg/t6260b.check deleted file mode 100644 index 3a7e8947aa..0000000000 --- a/test/files/neg/t6260b.check +++ /dev/null @@ -1,7 +0,0 @@ -t6260b.scala:3: error: bridge generated for member method apply: ()X in <$anon: () => X> -which overrides method apply: ()R in trait Function0 -clashes with definition of the member itself; -both have erased type ()Object -class Y { def f = new X("") or new X("") } - ^ -one error found diff --git a/test/files/neg/t6260b.scala b/test/files/neg/t6260b.scala deleted file mode 100644 index 73e2e58f73..0000000000 --- a/test/files/neg/t6260b.scala +++ /dev/null @@ -1,3 +0,0 @@ - -class X(val value: Object) extends AnyVal { def or(alt: => X): X = this } -class Y { def f = new X("") or new X("") } diff --git a/test/files/pos/delambdafy_t6260_method.check b/test/files/pos/delambdafy_t6260_method.check new file mode 100644 index 0000000000..f5cd6947d1 --- /dev/null +++ b/test/files/pos/delambdafy_t6260_method.check @@ -0,0 +1,13 @@ +delambdafy_t6260_method.scala:3: error: bridge generated for member method apply: (bx: Object)Object in class map$extension1 +which overrides method apply: (v1: Object)Object in trait Function1 +clashes with definition of the member itself; +both have erased type (bx: Object)Object + ((bx: Box[X]) => new Box(f(bx.x)))(this) + ^ +delambdafy_t6260_method.scala:8: error: bridge generated for member method apply: (bx: Object)Object in class map21 +which overrides method apply: (v1: Object)Object in trait Function1 +clashes with definition of the member itself; +both have erased type (bx: Object)Object + ((bx: Box[X]) => new Box(f(bx.x)))(self) + ^ +two errors found diff --git a/test/files/pos/delambdafy_t6260_method.flags b/test/files/pos/delambdafy_t6260_method.flags new file mode 100644 index 0000000000..48b438ddf8 --- /dev/null +++ b/test/files/pos/delambdafy_t6260_method.flags @@ -0,0 +1 @@ +-Ydelambdafy:method diff --git a/test/files/pos/delambdafy_t6260_method.scala b/test/files/pos/delambdafy_t6260_method.scala new file mode 100644 index 0000000000..93b5448227 --- /dev/null +++ b/test/files/pos/delambdafy_t6260_method.scala @@ -0,0 +1,17 @@ +class Box[X](val x: X) extends AnyVal { + def map[Y](f: X => Y): Box[Y] = + ((bx: Box[X]) => new Box(f(bx.x)))(this) +} + +object Test { + def map2[X, Y](self: Box[X], f: X => Y): Box[Y] = + ((bx: Box[X]) => new Box(f(bx.x)))(self) + + def main(args: Array[String]) { + val f = (x: Int) => x + 1 + val g = (x: String) => x + x + + map2(new Box(42), f) + new Box("abc") map g + } +} diff --git a/test/files/pos/t6260.flags b/test/files/pos/t6260.flags new file mode 100644 index 0000000000..2349d8294d --- /dev/null +++ b/test/files/pos/t6260.flags @@ -0,0 +1 @@ +-Ydelambdafy:inline diff --git a/test/files/pos/t6260.scala b/test/files/pos/t6260.scala new file mode 100644 index 0000000000..93b5448227 --- /dev/null +++ b/test/files/pos/t6260.scala @@ -0,0 +1,17 @@ +class Box[X](val x: X) extends AnyVal { + def map[Y](f: X => Y): Box[Y] = + ((bx: Box[X]) => new Box(f(bx.x)))(this) +} + +object Test { + def map2[X, Y](self: Box[X], f: X => Y): Box[Y] = + ((bx: Box[X]) => new Box(f(bx.x)))(self) + + def main(args: Array[String]) { + val f = (x: Int) => x + 1 + val g = (x: String) => x + x + + map2(new Box(42), f) + new Box("abc") map g + } +} diff --git a/test/files/pos/t6260b.scala b/test/files/pos/t6260b.scala new file mode 100644 index 0000000000..73e2e58f73 --- /dev/null +++ b/test/files/pos/t6260b.scala @@ -0,0 +1,3 @@ + +class X(val value: Object) extends AnyVal { def or(alt: => X): X = this } +class Y { def f = new X("") or new X("") } diff --git a/test/files/run/t6260-delambdafy.check b/test/files/run/t6260-delambdafy.check new file mode 100644 index 0000000000..b2a7bed988 --- /dev/null +++ b/test/files/run/t6260-delambdafy.check @@ -0,0 +1,4 @@ +f(C@2e) + +Test$lambda$1$$apply +apply diff --git a/test/files/run/t6260-delambdafy.flags b/test/files/run/t6260-delambdafy.flags new file mode 100644 index 0000000000..48b438ddf8 --- /dev/null +++ b/test/files/run/t6260-delambdafy.flags @@ -0,0 +1 @@ +-Ydelambdafy:method diff --git a/test/files/run/t6260-delambdafy.scala b/test/files/run/t6260-delambdafy.scala new file mode 100644 index 0000000000..056b1edd4e --- /dev/null +++ b/test/files/run/t6260-delambdafy.scala @@ -0,0 +1,12 @@ +class C[A](private val a: Any) extends AnyVal + +object Test { + val f = (x: C[Any]) => {println(s"f($x)"); x} + def main(args: Array[String]) { + f(new C(".")) + val methods = f.getClass.getDeclaredMethods.map(_.getName).sorted + println("") + println(methods.mkString("\n")) + } +} + diff --git a/test/files/run/t6260c.check b/test/files/run/t6260c.check new file mode 100644 index 0000000000..1a57f2d741 --- /dev/null +++ b/test/files/run/t6260c.check @@ -0,0 +1,5 @@ +f(C@2e) + +Test$$anonfun$$apply +apply +g(C@2e) diff --git a/test/files/run/t6260c.scala b/test/files/run/t6260c.scala new file mode 100644 index 0000000000..845dc157b7 --- /dev/null +++ b/test/files/run/t6260c.scala @@ -0,0 +1,17 @@ +class C[A](private val a: Any) extends AnyVal + +object Test { + val f = (x: C[Any]) => {println(s"f($x)"); x} + trait T[A] { + def apply(a: A): A + } + val g = new T[C[Any]] { def apply(a: C[Any]) = { println(s"g($a)"); a } } + def main(args: Array[String]) { + f(new C(".")) + val methods = f.getClass.getDeclaredMethods.map(_.getName).sorted + println("") + println(methods.mkString("\n")) + g.apply(new C(".")) + } +} + -- cgit v1.2.3 From d6b1e6e4ff1cb477e9b26ba7e1a02d1ea98fa132 Mon Sep 17 00:00:00 2001 From: Jason Zaugg Date: Mon, 10 Feb 2014 08:59:22 +0100 Subject: SI-6260 Adddress pull request review - fix typo - remove BRIDGE flag from the method that we promote from a bridge to a bona-fide method - note possibility for delambdafy to avoid the bridge method creation in *all* cases. - note inconsistency with anonymous class naming between `-Ydelamdafy:{inline,method}` --- src/compiler/scala/tools/nsc/transform/Delambdafy.scala | 8 +++++++- src/compiler/scala/tools/nsc/transform/Erasure.scala | 7 +++++-- test/files/neg/t6260-named.scala | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) (limited to 'test/files') diff --git a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala index b28a4a507d..d81a5d5755 100644 --- a/src/compiler/scala/tools/nsc/transform/Delambdafy.scala +++ b/src/compiler/scala/tools/nsc/transform/Delambdafy.scala @@ -232,6 +232,10 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre val parents = addSerializable(abstractFunctionErasedType) val funOwner = originalFunction.symbol.owner + // TODO harmonize the naming of delamdafy anon-fun classes with those spun up by Uncurry + // - make `anonClass.isAnonymousClass` true. + // - use `newAnonymousClassSymbol` or push the required variations into a similar factory method + // - reinstate the assertion in `Erasure.resolveAnonymousBridgeClash` val suffix = "$lambda$" + ( if (funOwner.isPrimaryConstructor) "" else "$" + funOwner.name @@ -283,8 +287,10 @@ abstract class Delambdafy extends Transform with TypingTransformers with ast.Tre else s"$sym: ${sym.tpe} in ${sym.owner}" bridgeMethod foreach (bm => + // TODO SI-6260 maybe just create the apply method with the signature (Object => Object) in all cases + // rather than the method+bridge pair. if (bm.symbol.tpe =:= applyMethodDef.symbol.tpe) - erasure.expandNameOfMethodThatClashesBridge(applyMethodDef.symbol) + erasure.resolveAnonymousBridgeClash(applyMethodDef.symbol, bm.symbol) ) val body = members ++ List(constr, applyMethodDef) ++ bridgeMethod diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index e2ffafb850..60c1553ef3 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -484,7 +484,7 @@ abstract class Erasure extends AddInterfaces !sigContainsValueClass || (checkBridgeOverrides(member, other, bridge) match { case Nil => true - case es if member.owner.isAnonymousClass => expandNameOfMethodThatClashesBridge(member); true + case es if member.owner.isAnonymousClass => resolveAnonymousBridgeClash(member, bridge); true case es => for ((pos, msg) <- es) unit.error(pos, msg); false }) ) @@ -1137,9 +1137,12 @@ abstract class Erasure extends AddInterfaces } } - final def expandNameOfMethodThatClashesBridge(sym: Symbol) { + final def resolveAnonymousBridgeClash(sym: Symbol, bridge: Symbol) { + // TODO reinstate this after Delambdafy generates anonymous classes that meet this requirement. + // require(sym.owner.isAnonymousClass, sym.owner) log(s"Expanding name of ${sym.debugLocationString} as it clashes with bridge. Renaming deemed safe because the owner is anonymous.") sym.expandName(sym.owner) + bridge.resetFlag(BRIDGE) } private class TypeRefAttachment(val tpe: TypeRef) diff --git a/test/files/neg/t6260-named.scala b/test/files/neg/t6260-named.scala index 7ce13476eb..7cd9ce8473 100644 --- a/test/files/neg/t6260-named.scala +++ b/test/files/neg/t6260-named.scala @@ -7,7 +7,7 @@ object Test { (x: C[Any]) => {println(s"f($x)"); x} // okay new T[C[Any]] { def apply(a: C[Any]) = a } // okay - // we can't rename the specific apply methid to avoid the clash + // we can't rename the specific apply method to avoid the clash object O extends Function1[C[Any], C[Any]] { def apply(a: C[Any]) = a } -- cgit v1.2.3