Skip to content

No mixin forwarder when ancestor is sealed #23482

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,7 @@ private sealed trait XSettings:
AdvancedSetting,
name = "Xmixin-force-forwarders",
helpArg = "mode",
descr = "Generate forwarder methods in classes inhering concrete methods from traits.",
descr = "Generate forwarder methods in classes inheriting concrete methods from traits.",
choices = List("true", "junit", "false"),
default = "true")

Expand Down
2 changes: 1 addition & 1 deletion compiler/src/dotty/tools/dotc/transform/Mixin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ object Mixin {
* <mods> def x_=(y: T) = ()
*
* 4.5 (done in `mixinForwarders`) For every method
* `<mods> def f[Ts](ps1)...(psN): U` imn M` that needs to be disambiguated:
* `<mods> def f[Ts](ps1)...(psN): U` in M` that needs to be disambiguated:
*
* <mods> def f[Ts](ps1)...(psN): U = super[M].f[Ts](ps1)...(psN)
*
Expand Down
29 changes: 18 additions & 11 deletions compiler/src/dotty/tools/dotc/transform/MixinOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -47,32 +47,39 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
cls.info.nonPrivateMember(sym.name).hasAltWith(_.symbol == sym)
}

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

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

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

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

!meth.isConstructor &&
meth.is(Method, butNot = PrivateOrAccessorOrDeferred) &&
(ctx.settings.mixinForwarderChoices.isTruthy || meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition ||
generateJUnitForwarder || generateSerializationForwarder) &&
isInImplementingClass(meth)
}
!meth.isConstructor
&& meth.is(Method, butNot = PrivateOrAccessorOrDeferred)
&& (!meth.is(JavaDefined) || !meth.owner.is(Sealed) || meth.owner.children.contains(cls))
&& (
ctx.settings.mixinForwarderChoices.isTruthy
|| meth.owner.is(Scala2x)
|| needsDisambiguation
|| hasNonInterfaceDefinition
|| generateJUnitForwarder
|| generateSerializationForwarder
)
&& isInImplementingClass(meth)
end needsMixinForwarder

final val PrivateOrAccessor: FlagSet = Private | Accessor
final val PrivateOrAccessorOrDeferred: FlagSet = Private | Accessor | Deferred
Expand Down
3 changes: 3 additions & 0 deletions tests/run/i23479/NonSeal.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// test: -jvm 17+
public non-sealed interface NonSeal extends Seal {
}
6 changes: 6 additions & 0 deletions tests/run/i23479/Seal.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@

public sealed interface Seal permits NonSeal {
default int g() {
return 42;
}
}
4 changes: 4 additions & 0 deletions tests/run/i23479/test.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// scalajs: --skip
class C() extends NonSeal

@main def Test = C()
13 changes: 13 additions & 0 deletions tests/run/i23479b.scala
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// scalajs: --skip
trait Ord[A]

object Ord:
sealed trait Rev[A] extends Ord[A]:
def reverse: Rev[A] = this

trait IntOrd extends Ord[Int]

object Int extends IntOrd with Rev[Int]

@main def Test =
assert(Ord.Int.getClass.getDeclaredMethods.map(_.getName).toList.contains("reverse"))
Loading