Skip to content

Commit 5a25c2a

Browse files
committed
Don't minimize reduntant parents used in super calls
Also removes the special-case in minimizeParents for Java interfaces, they can now be removed since we know which ones are used for super calls
1 parent f18b249 commit 5a25c2a

File tree

6 files changed

+49
-31
lines changed

6 files changed

+49
-31
lines changed

src/compiler/scala/tools/nsc/backend/jvm/BTypesFromSymbols.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ class BTypesFromSymbols[G <: Global](val global: G) extends BTypes {
252252

253253
val allParents = classParents ++ classSym.annotations.flatMap(newParentForAnnotation)
254254

255-
val minimizedParents = if (classSym.isJavaDefined) allParents else erasure.minimizeParents(allParents)
255+
val minimizedParents = if (classSym.isJavaDefined) allParents else erasure.minimizeParents(classSym, allParents)
256256
// We keep the superClass when computing minimizeParents to eliminate more interfaces.
257257
// Example: T can be eliminated from D
258258
// trait T

src/compiler/scala/tools/nsc/transform/Erasure.scala

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,8 @@ abstract class Erasure extends InfoTransform
2828

2929
val phaseName: String = "erasure"
3030

31+
val requiredDirectInterfaces = perRunCaches.newAnyRefMap[Symbol, mutable.Set[Symbol]]()
32+
3133
def newTransformer(unit: CompilationUnit): Transformer =
3234
new ErasureTransformer(unit)
3335

@@ -191,26 +193,21 @@ abstract class Erasure extends InfoTransform
191193

192194
/* Drop redundant types (ones which are implemented by some other parent) from the immediate parents.
193195
* This is important on Android because there is otherwise an interface explosion.
194-
* This is now restricted to Scala defined ancestors: a Java defined ancestor may need to be listed
195-
* as an immediate parent to support an `invokespecial`.
196196
*/
197-
def minimizeParents(parents: List[Type]): List[Type] = if (parents.isEmpty) parents else {
198-
def isRedundantParent(parent: Symbol, candidate: Symbol) =
199-
!parent.isJavaDefined &&
200-
parent.isTraitOrInterface &&
201-
candidate.isSubClass(parent)
202-
197+
def minimizeParents(cls: Symbol, parents: List[Type]): List[Type] = if (parents.isEmpty) parents else {
198+
val requiredDirect: Symbol => Boolean = requiredDirectInterfaces.getOrElse(cls, Set.empty)
203199
var rest = parents.tail
204200
var leaves = collection.mutable.ListBuffer.empty[Type] += parents.head
205201
while (rest.nonEmpty) {
206202
val candidate = rest.head
207-
if (candidate.typeSymbol.isJavaDefined && candidate.typeSymbol.isInterface) leaves += candidate
208-
else {
209-
val nonLeaf = leaves exists { t => t.typeSymbol isSubClass candidate.typeSymbol }
210-
if (!nonLeaf) {
211-
leaves = leaves filterNot { t => isRedundantParent(t.typeSymbol, candidate.typeSymbol) }
212-
leaves += candidate
203+
val candidateSym = candidate.typeSymbol
204+
val required = requiredDirect(candidateSym) || !leaves.exists(t => t.typeSymbol isSubClass candidateSym)
205+
if (required) {
206+
leaves = leaves filter { t =>
207+
val ts = t.typeSymbol
208+
requiredDirect(ts) || !ts.isTraitOrInterface || !candidateSym.isSubClass(ts)
213209
}
210+
leaves += candidate
214211
}
215212
rest = rest.tail
216213
}
@@ -224,7 +221,7 @@ abstract class Erasure extends InfoTransform
224221
def javaSig(sym0: Symbol, info: Type, markClassUsed: Symbol => Unit): Option[String] = enteringErasure {
225222
val isTraitSignature = sym0.enclClass.isTrait
226223

227-
def superSig(parents: List[Type]) = {
224+
def superSig(cls: Symbol, parents: List[Type]) = {
228225
def isInterfaceOrTrait(sym: Symbol) = sym.isInterface || sym.isTrait
229226

230227
// a signature should always start with a class
@@ -234,7 +231,7 @@ abstract class Erasure extends InfoTransform
234231
case _ => tps
235232
}
236233

237-
val minParents = minimizeParents(parents)
234+
val minParents = minimizeParents(cls, parents)
238235
val validParents =
239236
if (isTraitSignature)
240237
// java is unthrilled about seeing interfaces inherit from classes
@@ -373,7 +370,7 @@ abstract class Erasure extends InfoTransform
373370
case RefinedType(parents, decls) =>
374371
jsig(intersectionDominator(parents), primitiveOK = primitiveOK)
375372
case ClassInfoType(parents, _, _) =>
376-
superSig(parents)
373+
superSig(tp.typeSymbol, parents)
377374
case AnnotatedType(_, atp) =>
378375
jsig(atp, existentiallyBound, toplevel, primitiveOK)
379376
case BoundedWildcardType(bounds) =>
@@ -1185,6 +1182,10 @@ abstract class Erasure extends InfoTransform
11851182
reporter.error(tree.pos, s"Unable to emit super reference to ${sym.fullLocationString}, $owner is not accessible in ${localTyper.context.enclClass.owner}")
11861183
owner
11871184
}
1185+
1186+
if (sym.isJavaDefined && qualSym.isTraitOrInterface)
1187+
requiredDirectInterfaces.getOrElseUpdate(localTyper.context.enclClass.owner, mutable.Set.empty) += qualSym
1188+
11881189
if (qualSym != owner)
11891190
tree.updateAttachment(new QualTypeSymAttachment(qualSym))
11901191
} else if (!isJvmAccessible(owner, localTyper.context)) {

src/compiler/scala/tools/nsc/transform/Mixin.scala

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -218,13 +218,17 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
218218
if (isMemberOfClazz) {
219219
def genForwarder(required: Boolean): Unit = {
220220
val owner = member.owner
221-
if (owner.isJavaDefined && owner.isInterface && !clazz.parentSymbols.contains(owner)) {
221+
val isJavaInterface = owner.isJavaDefined && owner.isInterface
222+
if (isJavaInterface && !clazz.parentSymbols.contains(owner)) {
222223
if (required) {
223224
val text = s"Unable to implement a mixin forwarder for $member in $clazz unless interface ${owner.name} is directly extended by $clazz."
224225
reporter.error(clazz.pos, text)
225226
}
226-
} else
227+
} else {
228+
if (isJavaInterface)
229+
erasure.requiredDirectInterfaces.getOrElseUpdate(clazz, mutable.Set.empty) += owner
227230
cloneAndAddMixinMember(mixinClass, member).asInstanceOf[TermSymbol] setAlias member
231+
}
228232
}
229233

230234
// `member` is a concrete method defined in `mixinClass`, which is a base class of
@@ -291,9 +295,12 @@ abstract class Mixin extends InfoTransform with ast.TreeDSL with AccessorSynthes
291295
reporter.error(clazz.pos, "Member %s of mixin %s is missing a concrete super implementation.".format(
292296
mixinMember.alias, mixinClass))
293297
case alias1 =>
294-
if (alias1.owner.isJavaDefined && alias1.owner.isInterface && !clazz.parentSymbols.contains(alias1.owner)) {
295-
val suggestedParent = exitingTyper(clazz.info.baseType(alias1.owner))
296-
reporter.error(clazz.pos, s"Unable to implement a super accessor required by trait ${mixinClass.name} unless $suggestedParent is directly extended by $clazz.")
298+
if (alias1.owner.isJavaDefined && alias1.owner.isInterface) {
299+
if (!clazz.parentSymbols.contains(alias1.owner)) {
300+
val suggestedParent = exitingTyper(clazz.info.baseType(alias1.owner))
301+
reporter.error(clazz.pos, s"Unable to implement a super accessor required by trait ${mixinClass.name} unless $suggestedParent is directly extended by $clazz.")
302+
} else
303+
erasure.requiredDirectInterfaces.getOrElseUpdate(clazz, mutable.Set.empty) += alias1.owner
297304
}
298305
superAccessor.asInstanceOf[TermSymbol] setAlias alias1
299306
}

test/files/neg/trait-defaults-super.scala

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,3 @@ trait T extends java.lang.Iterable[String] {
1212
def iterator(): java.util.Iterator[String] = java.util.Collections.emptyList().iterator()
1313
}
1414
class C extends T
15-
object Test {
16-
def main(args: Array[String]): Unit = {
17-
val t: T = new C
18-
t.spliterator
19-
t.foo
20-
}
21-
}
Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,10 @@ trait T extends java.lang.Iterable[String] {
1414
class C extends T with java.lang.Iterable[String] // super accessor is okay with Iterable as a direct parent
1515
object Test {
1616
def main(args: Array[String]): Unit = {
17-
val t: T = new C
17+
val c = new C
18+
c.spliterator
19+
c.foo
20+
val t: T = c
1821
t.spliterator
1922
t.foo
2023
}

test/junit/scala/lang/traits/BytecodeTest.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,20 @@ class BytecodeTest extends BytecodeTesting {
407407
assert(ins contains Invoke(INVOKESPECIAL, "T", "f", "()I", true), ins.stringLines)
408408
}
409409

410+
@Test
411+
def noMinimizeScalaTraitAccessingJavaMember(): Unit = {
412+
val jCode = List("interface A { default int f() { return 1; } }" -> "A.java")
413+
val code =
414+
"""trait U extends A
415+
|trait V extends U
416+
|class C extends U with V { def t = super.f() }
417+
""".stripMargin
418+
val List(c, u, v) = compileClasses(code, jCode)
419+
assertEquals(c.interfaces.asScala.toList.sorted, List("U", "V"))
420+
val ins = getMethod(c, "t").instructions
421+
assert(ins contains Invoke(INVOKESPECIAL, "U", "f", "()I", true), ins.stringLines)
422+
}
423+
410424
def ifs(c: ClassNode, expected: List[String]) = assertEquals(expected, c.interfaces.asScala.toList.sorted)
411425
def invSt(m: Method, receiver: String, method: String = "f$", itf: Boolean = true): Unit =
412426
assert(m.instructions contains Invoke(INVOKESTATIC, receiver, method, s"(L$receiver;)I", itf), m.instructions.stringLines)

0 commit comments

Comments
 (0)