Skip to content

Commit e914efe

Browse files
authored
No mixin forwarder when ancestor is sealed (#23482)
Fixes #23479 Adds check for Java sealed ancestor to `needsMixinForwarder`. If it's ok to invokespecial an indirect superinterface, then the fix should be in the backend, to not add the interface parent.
2 parents 28a4921 + 545fdd9 commit e914efe

File tree

7 files changed

+47
-14
lines changed

7 files changed

+47
-14
lines changed

compiler/src/dotty/tools/dotc/config/ScalaSettings.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ private sealed trait XSettings:
358358
AdvancedSetting,
359359
name = "Xmixin-force-forwarders",
360360
helpArg = "mode",
361-
descr = "Generate forwarder methods in classes inhering concrete methods from traits.",
361+
descr = "Generate forwarder methods in classes inheriting concrete methods from traits.",
362362
choices = List("true", "junit", "false"),
363363
default = "true")
364364

compiler/src/dotty/tools/dotc/transform/Mixin.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ object Mixin {
9696
* <mods> def x_=(y: T) = ()
9797
*
9898
* 4.5 (done in `mixinForwarders`) For every method
99-
* `<mods> def f[Ts](ps1)...(psN): U` imn M` that needs to be disambiguated:
99+
* `<mods> def f[Ts](ps1)...(psN): U` in M` that needs to be disambiguated:
100100
*
101101
* <mods> def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN)
102102
*

compiler/src/dotty/tools/dotc/transform/MixinOps.scala

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -48,33 +48,40 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
4848
cls.info.nonPrivateMember(sym.name).hasAltWith(_.symbol == sym)
4949
}
5050

51-
/** Does `method` need a forwarder to in class `cls`
52-
* Method needs a forwarder in those cases:
51+
/** Does `method` need a forwarder in class `cls`?
52+
* Method needs a forwarder in these cases:
5353
* - there's a class defining a method with same signature
5454
* - there are multiple traits defining method with same signature
5555
*/
56-
def needsMixinForwarder(meth: Symbol): Boolean = {
56+
def needsMixinForwarder(meth: Symbol): Boolean =
5757
lazy val competingMethods = competingMethodsIterator(meth).toList
5858

59-
def needsDisambiguation = competingMethods.exists(x=> !x.is(Deferred)) // multiple implementations are available
59+
def needsDisambiguation = competingMethods.exists(!_.is(Deferred)) // multiple implementations are available
6060
def hasNonInterfaceDefinition = competingMethods.exists(!_.owner.is(Trait)) // there is a definition originating from class
6161

6262
// JUnit 4 won't recognize annotated default methods, so always generate a forwarder for them.
6363
def generateJUnitForwarder: Boolean =
64-
meth.annotations.nonEmpty && JUnit4Annotations.exists(annot => meth.hasAnnotation(annot)) &&
64+
meth.annotations.nonEmpty && JUnit4Annotations.exists(meth.hasAnnotation) &&
6565
ctx.settings.mixinForwarderChoices.isAtLeastJunit
6666

6767
// Similarly, Java serialization won't take into account a readResolve/writeReplace default method.
6868
def generateSerializationForwarder: Boolean =
6969
(meth.name == nme.readResolve || meth.name == nme.writeReplace) && meth.info.paramNamess.flatten.isEmpty
7070

71-
!meth.isConstructor &&
72-
meth.is(Method, butNot = PrivateOrAccessorOrDeferred) &&
73-
(ctx.settings.mixinForwarderChoices.isTruthy || meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition ||
74-
generateJUnitForwarder || generateSerializationForwarder) &&
75-
isInImplementingClass(meth) &&
76-
!meth.name.is(InlineAccessorName)
77-
}
71+
!meth.isConstructor
72+
&& meth.is(Method, butNot = PrivateOrAccessorOrDeferred)
73+
&& (!meth.is(JavaDefined) || !meth.owner.is(Sealed) || meth.owner.children.contains(cls))
74+
&& (
75+
ctx.settings.mixinForwarderChoices.isTruthy
76+
|| meth.owner.is(Scala2x)
77+
|| needsDisambiguation
78+
|| hasNonInterfaceDefinition
79+
|| generateJUnitForwarder
80+
|| generateSerializationForwarder
81+
)
82+
&& isInImplementingClass(meth)
83+
&& !meth.name.is(InlineAccessorName)
84+
end needsMixinForwarder
7885

7986
final val PrivateOrAccessor: FlagSet = Private | Accessor
8087
final val PrivateOrAccessorOrDeferred: FlagSet = Private | Accessor | Deferred

tests/run/i23479/NonSeal.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// test: -jvm 17+
2+
public non-sealed interface NonSeal extends Seal {
3+
}

tests/run/i23479/Seal.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
2+
public sealed interface Seal permits NonSeal {
3+
default int g() {
4+
return 42;
5+
}
6+
}

tests/run/i23479/test.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
// scalajs: --skip
2+
class C() extends NonSeal
3+
4+
@main def Test = C()

tests/run/i23479b.scala

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
// scalajs: --skip
2+
trait Ord[A]
3+
4+
object Ord:
5+
sealed trait Rev[A] extends Ord[A]:
6+
def reverse: Rev[A] = this
7+
8+
trait IntOrd extends Ord[Int]
9+
10+
object Int extends IntOrd with Rev[Int]
11+
12+
@main def Test =
13+
assert(Ord.Int.getClass.getDeclaredMethods.map(_.getName).toList.contains("reverse"))

0 commit comments

Comments
 (0)