From 31024d2a1f589a9065b728bf0b4ce9c6ec9b0a6f Mon Sep 17 00:00:00 2001 From: Guillaume Martres Date: Tue, 30 Apr 2019 17:44:52 +0200 Subject: [PATCH] Fix #1392: Disallow super-calls that cannot be implemented correctly A call `super[T].m` that resolves to `A.m` cannot be translated to correct bytecode if `A` is a class (not a trait / interface), but not the direct superclass. Invokespecial would select an overriding method in the direct superclass, rather than `A.m`. We allow this if there are statically no intervening overrides. Based on https://github.com/scala/scala/commit/a980fded6806f83bebe2ced31ab1ed70926254b2 and https://github.com/scala/scala/commit/0a840380aa47d52d2addd2b96dbbb68b874c3a67 Note that unlike Scala 2 we do not need to check if the mixin part of a Super is to a direct parent here because this is already done in TypeAssigner. Co-Authored-By: Jason Zaugg Co-Authored-By: Lukas Rytz --- .../tools/dotc/transform/SuperAccessors.scala | 38 +++++++++++++++---- tests/neg/i1392.scala | 6 +++ tests/run/i1392a.scala | 16 ++++++++ tests/run/i1392b.scala | 16 ++++++++ tests/run/i1392c.scala | 16 ++++++++ 5 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 tests/neg/i1392.scala create mode 100644 tests/run/i1392a.scala create mode 100644 tests/run/i1392b.scala create mode 100644 tests/run/i1392c.scala diff --git a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala index 6f7681141974..117db5c342a0 100644 --- a/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala +++ b/compiler/src/dotty/tools/dotc/transform/SuperAccessors.scala @@ -115,15 +115,39 @@ class SuperAccessors(thisPhase: DenotTransformer) { sel.sourcePos) else ctx.log(i"ok super $sel ${sym.showLocated} $member $clazz ${member.isIncompleteIn(clazz)}") } - else if (mix.name.isEmpty && !(sym.owner is Trait)) - // SI-4989 Check if an intermediate class between `clazz` and `sym.owner` redeclares the method as abstract. - for (intermediateClass <- clazz.info.baseClasses.tail.takeWhile(_ != sym.owner)) { - val overriding = sym.overridingSymbol(intermediateClass) - if ((overriding is (Deferred, butNot = AbsOverride)) && !(overriding.owner is Trait)) - ctx.error( - s"${sym.showLocated} cannot be directly accessed from ${clazz} because ${overriding.owner} redeclares it as abstract", + else { + val owner = sym.owner + if (!owner.is(Trait)) { + if (mix.name.isEmpty) { + // scala/bug#4989 Check if an intermediate class between `clazz` and `sym.owner` redeclares the method as abstract. + for (intermediateClass <- clazz.info.baseClasses.tail.takeWhile(_ != sym.owner)) { + val overriding = sym.overridingSymbol(intermediateClass) + if ((overriding is (Deferred, butNot = AbsOverride)) && !(overriding.owner is Trait)) + ctx.error( + s"${sym.showLocated} cannot be directly accessed from ${clazz} because ${overriding.owner} redeclares it as abstract", + sel.sourcePos) + } + } else { + // scala/scala-dev#143: + // a call `super[T].m` that resolves to `A.m` cannot be translated to correct bytecode if + // `A` is a class (not a trait / interface), but not the direct superclass. Invokespecial + // would select an overriding method in the direct superclass, rather than `A.m`. + // We allow this if there are statically no intervening overrides. + def hasClassOverride(member: Symbol, subCls: ClassSymbol): Boolean = { + if (subCls == defn.ObjectClass || subCls == member.owner) false + else if (member.overridingSymbol(subCls).exists) true + else hasClassOverride(member, subCls.superClass.asClass) + } + val superCls = clazz.asClass.superClass.asClass + if (owner != superCls && hasClassOverride(sym, superCls)) { + ctx.error( + hl"""Super call cannot be emitted: the selected $sym is declared in $owner, which is not the direct superclass of $clazz. + |An unqualified super call (super.${sym.name}) would be allowed.""", sel.sourcePos) + } + } } + } if (name.isTermName && mix.name.isEmpty && ((clazz is Trait) || clazz != ctx.owner.enclosingClass || !validCurrentClass)) superAccessorCall(sel)(ctx.withPhase(thisPhase.next)) diff --git a/tests/neg/i1392.scala b/tests/neg/i1392.scala new file mode 100644 index 000000000000..afabf7a74af4 --- /dev/null +++ b/tests/neg/i1392.scala @@ -0,0 +1,6 @@ +class A { def m = 1 } +class B extends A { override def m = 2 } +trait T extends A +class C extends B with T { + override def m = super[T].m // error: Super call cannot be emitted: the selected method m is declared in class A, which is not the direct superclass of class C. +} diff --git a/tests/run/i1392a.scala b/tests/run/i1392a.scala new file mode 100644 index 000000000000..d7b014958aa5 --- /dev/null +++ b/tests/run/i1392a.scala @@ -0,0 +1,16 @@ +trait A { def m = 1 } +class B extends A { override def m = 2 } +trait T extends A +class C extends B with T { + def t1 = super[B].m + def t2 = super.m + def t3 = super[T].m +} +object Test { + def main(args: Array[String]): Unit = { + val c = new C + assert(c.t1 == 2) + assert(c.t2 == 2) + assert(c.t3 == 1) + } +} diff --git a/tests/run/i1392b.scala b/tests/run/i1392b.scala new file mode 100644 index 000000000000..60cd17b96104 --- /dev/null +++ b/tests/run/i1392b.scala @@ -0,0 +1,16 @@ +class A { def m = 1 } +class B extends A +trait T extends A { override def m = 2 } +class C extends B with T { + def t1 = super[B].m + def t2 = super.m + def t3 = super[T].m +} +object Test { + def main(args: Array[String]): Unit = { + val c = new C + assert(c.t1 == 1) + assert(c.t2 == 2) + assert(c.t3 == 2) + } +} diff --git a/tests/run/i1392c.scala b/tests/run/i1392c.scala new file mode 100644 index 000000000000..a5ae0419df22 --- /dev/null +++ b/tests/run/i1392c.scala @@ -0,0 +1,16 @@ +trait A { def m = 1 } +class B extends A +trait T extends A { override def m = 2 } +class C extends B with T { + def t1 = super[B].m + def t2 = super.m + def t3 = super[T].m +} +object Test { + def main(args: Array[String]): Unit = { + val c = new C + assert(c.t1 == 1) + assert(c.t2 == 2) + assert(c.t3 == 2) + } +}