From aa7ee91ec92e672fa566e1f69b2c7f6f631f0fdc Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 29 Dec 2017 19:05:04 +0100 Subject: [PATCH 1/2] Fix #1905: Detect case where bridge would clash with the member With value classes it is possible that a bridge is needed because the semi erased types of a member and its overridden method differ, yet the fully erased types of the member and the bridge are the same. With this commit we flag this situation as an error. Before it generated a ClassCastException at runtime instead. --- .../dotty/tools/dotc/transform/Bridges.scala | 27 +++++++++++++++---- tests/neg/i1905.scala | 17 ++++++++++++ 2 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 tests/neg/i1905.scala diff --git a/compiler/src/dotty/tools/dotc/transform/Bridges.scala b/compiler/src/dotty/tools/dotc/transform/Bridges.scala index c2a758363240..6ed871e04d55 100644 --- a/compiler/src/dotty/tools/dotc/transform/Bridges.scala +++ b/compiler/src/dotty/tools/dotc/transform/Bridges.scala @@ -16,6 +16,7 @@ class Bridges(root: ClassSymbol)(implicit ctx: Context) { assert(ctx.phase == ctx.erasurePhase.next) private val preErasureCtx = ctx.withPhase(ctx.erasurePhase) + private val elimErasedCtx = ctx.withPhase(ctx.elimErasedValueTypePhase.next) private class BridgesCursor(implicit ctx: Context) extends OverridingPairs.Cursor(root) { @@ -35,31 +36,47 @@ class Bridges(root: ClassSymbol)(implicit ctx: Context) { private val bridgesScope = newScope private val bridgeTarget = newMutableSymbolMap[Symbol] + def bridgePosFor(member: Symbol) = + if (member.owner == root && member.pos.exists) member.pos else root.pos + /** Add a bridge between `member` and `other`, where `member` overrides `other` * before erasure, if the following conditions are satisfied. * - * - `member` and other have different signatures - * - there is not yet a bridge with the same name and signature in `root` + * - `member` and `other` have different signatures + * - there is not yet a bridge with the same name and signature in `root`. * * The bridge has the erased info of `other` and forwards to `member`. + * Additionally, if `member` and `other` do have the same signature, + * but not the same type after erasure and before elimErasedValueTypes + * issue an error: A bridge would be needed yet it would clash with the member itself. + * See neg/i1905.scala */ private def addBridgeIfNeeded(member: Symbol, other: Symbol) = { def bridgeExists = bridgesScope.lookupAll(member.name).exists(bridge => bridgeTarget(bridge) == member && bridge.signature == other.signature) - if (!(member.signature == other.signature || bridgeExists)) + def info(sym: Symbol)(implicit ctx: Context) = sym.info + def desc(sym: Symbol)= i"$sym${info(sym)(preErasureCtx)} in ${sym.owner}" + if (member.signature == other.signature) { + if (!member.info.matches(other.info)) + ctx.error(em"""bridge generated for member ${desc(member)} + |which overrides ${desc(other)} + |clashes with definition of the member itself; both have erased type ${info(member)(elimErasedCtx)}."""", + bridgePosFor(member)) + } + else if (!bridgeExists) addBridge(member, other) } /** Generate bridge between `member` and `other` */ private def addBridge(member: Symbol, other: Symbol) = { - val bridgePos = if (member.owner == root && member.pos.exists) member.pos else root.pos val bridge = other.copy( owner = root, flags = (member.flags | Method | Bridge | Artifact) &~ (Accessor | ParamAccessor | CaseAccessor | Deferred | Lazy | Module), - coord = bridgePos).enteredAfter(ctx.erasurePhase.asInstanceOf[DenotTransformer]).asTerm + coord = bridgePosFor(member)) + .enteredAfter(ctx.erasurePhase.asInstanceOf[DenotTransformer]).asTerm ctx.debuglog( i"""generating bridge from ${other.showLocated}: ${other.info} diff --git a/tests/neg/i1905.scala b/tests/neg/i1905.scala new file mode 100644 index 000000000000..c2572a1e7aa4 --- /dev/null +++ b/tests/neg/i1905.scala @@ -0,0 +1,17 @@ +class Arr[T](private val underlying: scala.Array[T]) extends AnyVal + +abstract class SeqMonoTransforms[+A, +Repr] { + protected[this] def fromIterableWithSameElemType(): Repr + def getFIWSET: Repr = fromIterableWithSameElemType() +} + +class ArrOps[A](val xs: Arr[A]) extends SeqMonoTransforms[A, Arr[A]] { + def fromIterableWithSameElemType(): Arr[A] = xs // error: bridge clashes with member +} + +object Test { + def main(args: Array[String]) = { + val t = new ArrOps(new Arr(Array(1, 2, 3))) + val t2 = t.getFIWSET + } +} From f65309595af3f75fea48a990265db317c4995a06 Mon Sep 17 00:00:00 2001 From: Martin Odersky Date: Fri, 29 Dec 2017 19:59:49 +0100 Subject: [PATCH 2/2] Omit this-prefix when printing parameter references Print a reference to a parameter with just the name (e.g. "A") instead of "C.this.A". --- .../src/dotty/tools/dotc/printing/RefinedPrinter.scala | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala index 57587541d036..0e5b33c52d18 100644 --- a/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala +++ b/compiler/src/dotty/tools/dotc/printing/RefinedPrinter.scala @@ -170,6 +170,12 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { case tp: TypeRef => if (tp.symbol.isAnonymousClass && !ctx.settings.uniqid.value) return toText(tp.info) + if (tp.symbol.is(Param)) + tp.prefix match { + case pre: ThisType if pre.cls == tp.symbol.owner => + return nameString(tp.symbol) + case _ => + } case ExprType(result) => return "=> " ~ toText(result) case ErasedValueType(tycon, underlying) => @@ -501,7 +507,7 @@ class RefinedPrinter(_ctx: Context) extends PlainPrinter(_ctx) { typeDefText(tparamsTxt, toText(rhs)) case LambdaTypeTree(tparams, body) => recur(body, tparamsText(tparams)) - case rhs: TypeTree if rhs.tpe.isInstanceOf[TypeBounds] => + case rhs: TypeTree if rhs.typeOpt.isInstanceOf[TypeBounds] => typeDefText(tparamsTxt, toText(rhs)) case rhs => typeDefText(tparamsTxt, optText(rhs)(" = " ~ _))