diff options
author | Martin Odersky <odersky@gmail.com> | 2012-09-10 18:07:20 +0200 |
---|---|---|
committer | Paul Phillips <paulp@improving.org> | 2012-09-20 09:41:33 -0700 |
commit | 96408154f46dce623d3b3c3fdc67f5ccc3779f8f (patch) | |
tree | cc02888f504bf1ebbd131d0297145b31684222f5 /src | |
parent | d834d90d88e1dab6a8621b13c9d4b64d3417a94e (diff) | |
download | scala-96408154f46dce623d3b3c3fdc67f5ccc3779f8f.tar.gz scala-96408154f46dce623d3b3c3fdc67f5ccc3779f8f.tar.bz2 scala-96408154f46dce623d3b3c3fdc67f5ccc3779f8f.zip |
Fixes SI-6260
Guards against bridge methods that clash with other methods. Two
tests: The neg test is the original ticket. The run test tweaks
things slightly so that the generated bridge method does not clash,
and tests that the necessary unboxings are indeed performed at
runtime.
Diffstat (limited to 'src')
-rw-r--r-- | src/compiler/scala/tools/nsc/Global.scala | 2 | ||||
-rw-r--r-- | src/compiler/scala/tools/nsc/transform/Erasure.scala | 88 |
2 files changed, 69 insertions, 21 deletions
diff --git a/src/compiler/scala/tools/nsc/Global.scala b/src/compiler/scala/tools/nsc/Global.scala index 6fb6b1736b..58fcee4b30 100644 --- a/src/compiler/scala/tools/nsc/Global.scala +++ b/src/compiler/scala/tools/nsc/Global.scala @@ -1094,6 +1094,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) // TODO - trim these to the absolute minimum. @inline final def afterErasure[T](op: => T): T = afterPhase(currentRun.erasurePhase)(op) + @inline final def afterPostErasure[T](op: => T): T = afterPhase(currentRun.posterasurePhase)(op) @inline final def afterExplicitOuter[T](op: => T): T = afterPhase(currentRun.explicitouterPhase)(op) @inline final def afterFlatten[T](op: => T): T = afterPhase(currentRun.flattenPhase)(op) @inline final def afterIcode[T](op: => T): T = afterPhase(currentRun.icodePhase)(op) @@ -1403,6 +1404,7 @@ class Global(var currentSettings: Settings, var reporter: Reporter) val specializePhase = phaseNamed("specialize") val explicitouterPhase = phaseNamed("explicitouter") val erasurePhase = phaseNamed("erasure") + val posterasurePhase = phaseNamed("posterasure") // val lazyvalsPhase = phaseNamed("lazyvals") val lambdaliftPhase = phaseNamed("lambdalift") // val constructorsPhase = phaseNamed("constructors") diff --git a/src/compiler/scala/tools/nsc/transform/Erasure.scala b/src/compiler/scala/tools/nsc/transform/Erasure.scala index b3b0c82d38..9381d6432e 100644 --- a/src/compiler/scala/tools/nsc/transform/Erasure.scala +++ b/src/compiler/scala/tools/nsc/transform/Erasure.scala @@ -167,6 +167,8 @@ abstract class Erasure extends AddInterfaces case tp => tp :: Nil } + private def isErasedValueType(tpe: Type) = tpe.isInstanceOf[ErasedValueType] + /** The Java signature of type 'info', for symbol sym. The symbol is used to give the right return * type for constructors. */ @@ -373,18 +375,18 @@ abstract class Erasure extends AddInterfaces } } - class ComputeBridges(owner: Symbol) { + class ComputeBridges(unit: CompilationUnit, root: Symbol) { assert(phase == currentRun.erasurePhase, phase) var toBeRemoved = immutable.Set[Symbol]() - val site = owner.thisType + val site = root.thisType val bridgesScope = newScope val bridgeTarget = mutable.HashMap[Symbol, Symbol]() var bridges = List[Tree]() val opc = beforeExplicitOuter { - new overridingPairs.Cursor(owner) { - override def parents = List(owner.info.firstParent) + new overridingPairs.Cursor(root) { + override def parents = List(root.info.firstParent) override def exclude(sym: Symbol) = !sym.isMethod || sym.isPrivate || super.exclude(sym) } } @@ -402,8 +404,50 @@ abstract class Erasure extends AddInterfaces (bridges, toBeRemoved) } + /** Check that a bridge only overrides members that are also overridden by the original member. + * This test is necessary only for members that have a value class in their type. + * Such members are special because their types after erasure and after post-erasure differ/. + * This means we generate them after erasure, but the post-erasure transform might introduce + * a name clash. The present method guards against these name clashes. + * + * @param member The original member + * @param other The overidden symbol for which the bridge was generated + * @param bridge The bridge + */ + def checkBridgeOverrides(member: Symbol, other: Symbol, bridge: Symbol): Boolean = { + def fulldef(sym: Symbol) = + if (sym == NoSymbol) sym.toString + else s"$sym: ${sym.tpe} in ${sym.owner}" + var noclash = true + def clashError(what: String) = { + noclash = false + unit.error( + if (member.owner == root) member.pos else root.pos, + s"""bridge generated for member ${fulldef(member)} + |which overrides ${fulldef(other)} + |clashes with definition of $what; + |both have erased type ${afterPostErasure(bridge.tpe)}""".stripMargin) + } + for (bc <- root.baseClasses) { + def overriddenBy(sym: Symbol) = + sym.matchingSymbol(bc, site).alternatives filter (sym => !sym.isBridge) + for (overBridge <- afterPostErasure(overriddenBy(bridge))) { + if (overBridge == member) { + clashError("the member itself") + } else { + val overMembers = overriddenBy(member) + if (!overMembers.exists(overMember => + afterPostErasure(overMember.tpe =:= overBridge.tpe))) { + clashError(fulldef(overBridge)) + } + } + } + } + noclash + } + def checkPair(member: Symbol, other: Symbol) { - val otpe = erasure(owner)(other.tpe) + val otpe = erasure(root)(other.tpe) val bridgeNeeded = afterErasure ( !(other.tpe =:= member.tpe) && !(deconstMap(other.tpe) =:= deconstMap(member.tpe)) && @@ -417,24 +461,29 @@ abstract class Erasure extends AddInterfaces return val newFlags = (member.flags | BRIDGE) & ~(ACCESSOR | DEFERRED | LAZY | lateDEFERRED) - val bridge = other.cloneSymbolImpl(owner, newFlags) setPos owner.pos + val bridge = other.cloneSymbolImpl(root, newFlags) setPos root.pos debuglog("generating bridge from %s (%s): %s to %s: %s".format( other, flagsToString(newFlags), otpe + other.locationString, member, - erasure(owner)(member.tpe) + member.locationString) + erasure(root)(member.tpe) + member.locationString) ) // the parameter symbols need to have the new owner bridge setInfo (otpe cloneInfo bridge) bridgeTarget(bridge) = member - afterErasure(owner.info.decls enter bridge) - if (other.owner == owner) { - afterErasure(owner.info.decls.unlink(other)) - toBeRemoved += other + + if (!(member.tpe exists (_.typeSymbol.isDerivedValueClass)) || + checkBridgeOverrides(member, other, bridge)) { + afterErasure(root.info.decls enter bridge) + if (other.owner == root) { + afterErasure(root.info.decls.unlink(other)) + toBeRemoved += other + } + + bridgesScope enter bridge + bridges ::= makeBridgeDefDef(bridge, member, other) } - bridgesScope enter bridge - bridges ::= makeBridgeDefDef(bridge, member, other) } def makeBridgeDefDef(bridge: Symbol, member: Symbol, other: Symbol) = afterErasure { @@ -466,7 +515,7 @@ abstract class Erasure extends AddInterfaces val rhs = member.tpe match { case MethodType(Nil, ConstantType(c)) => Literal(c) case _ => - val sel: Tree = Select(This(owner), member) + val sel: Tree = Select(This(root), member) val bridgingCall = (sel /: bridge.paramss)((fun, vparams) => Apply(fun, vparams map Ident)) maybeWrap(bridgingCall) @@ -480,8 +529,6 @@ abstract class Erasure extends AddInterfaces private def isPrimitiveValueType(tpe: Type) = isPrimitiveValueClass(tpe.typeSymbol) - private def isErasedValueType(tpe: Type) = tpe.isInstanceOf[ErasedValueType] - private def isDifferentErasedValueType(tpe: Type, other: Type) = isErasedValueType(tpe) && (tpe ne other) @@ -814,7 +861,6 @@ abstract class Erasure extends AddInterfaces * but their erased types are the same. */ private def checkNoDoubleDefs(root: Symbol) { - def afterErasure[T](op: => T): T = atPhase(phase.next.next)(op) def doubleDefError(sym1: Symbol, sym2: Symbol) { // the .toString must also be computed at the earlier phase val tpe1 = afterRefchecks(root.thisType.memberType(sym1)) @@ -830,7 +876,7 @@ abstract class Erasure extends AddInterfaces sym2 + ":" + afterRefchecks(tpe2.toString) + (if (sym2.owner == root) " at line " + (sym2.pos).line else sym2.locationString) + "\nhave same type" + - (if (afterRefchecks(tpe1 =:= tpe2)) "" else " after erasure: " + afterErasure(sym1.tpe))) + (if (afterRefchecks(tpe1 =:= tpe2)) "" else " after erasure: " + afterPostErasure(sym1.tpe))) sym1.setInfo(ErrorType) } @@ -840,7 +886,7 @@ abstract class Erasure extends AddInterfaces if (e.sym.isTerm) { var e1 = decls.lookupNextEntry(e) while (e1 ne null) { - if (afterErasure(e1.sym.info =:= e.sym.info)) doubleDefError(e.sym, e1.sym) + if (afterPostErasure(e1.sym.info =:= e.sym.info)) doubleDefError(e.sym, e1.sym) e1 = decls.lookupNextEntry(e1) } } @@ -854,7 +900,7 @@ abstract class Erasure extends AddInterfaces || !sym.hasTypeAt(currentRun.refchecksPhase.id)) override def matches(sym1: Symbol, sym2: Symbol): Boolean = - afterErasure(sym1.tpe =:= sym2.tpe) + afterPostErasure(sym1.tpe =:= sym2.tpe) } while (opc.hasNext) { if (!afterRefchecks( @@ -902,7 +948,7 @@ abstract class Erasure extends AddInterfaces private def bridgeDefs(owner: Symbol): (List[Tree], immutable.Set[Symbol]) = { assert(phase == currentRun.erasurePhase, phase) debuglog("computing bridges for " + owner) - new ComputeBridges(owner) compute() + new ComputeBridges(unit, owner) compute() } def addBridges(stats: List[Tree], base: Symbol): List[Tree] = |