Skip to content

Fix #2563: Unconditionally emit mixin forwarders #6081

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

Merged
merged 3 commits into from
Mar 18, 2019
Merged
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
13 changes: 13 additions & 0 deletions compiler/src/dotty/tools/dotc/config/ScalaSettings.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package dotty.tools.dotc
package config

import dotty.tools.dotc.core.Contexts.Context
import dotty.tools.io.{ Directory, PlainDirectory, AbstractFile }
import PathResolver.Defaults
import rewrites.Rewrites
Expand Down Expand Up @@ -76,6 +77,18 @@ class ScalaSettings extends Settings.SettingGroup {
val XfatalWarnings: Setting[Boolean] = BooleanSetting("-Xfatal-warnings", "Fail the compilation if there are any warnings.")
val XverifySignatures: Setting[Boolean] = BooleanSetting("-Xverify-signatures", "Verify generic signatures in generated bytecode.")

val XmixinForceForwarders = ChoiceSetting(
name = "-Xmixin-force-forwarders",
helpArg = "mode",
descr = "Generate forwarder methods in classes inhering concrete methods from traits.",
choices = List("true", "junit", "false"),
default = "true")

object mixinForwarderChoices {
def isTruthy(implicit ctx: Context) = XmixinForceForwarders.value == "true"
def isAtLeastJunit(implicit ctx: Context) = isTruthy || XmixinForceForwarders.value == "junit"
}

/** -Y "Private" settings */
val YoverrideVars: Setting[Boolean] = BooleanSetting("-Yoverride-vars", "Allow vars to be overridden.")
val Yhelp: Setting[Boolean] = BooleanSetting("-Y", "Print a synopsis of private options.")
Expand Down
5 changes: 3 additions & 2 deletions compiler/src/dotty/tools/dotc/transform/MixinOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -59,12 +59,13 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(implicit ctx: Cont
def hasNonInterfaceDefinition = competingMethods.exists(!_.owner.is(Trait)) // there is a definition originating from class
!meth.isConstructor &&
meth.is(Method, butNot = PrivateOrAccessorOrDeferred) &&
(meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition || needsJUnit4Fix(meth) ) &&
(ctx.settings.mixinForwarderChoices.isTruthy || meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition || needsJUnit4Fix(meth)) &&
isCurrent(meth)
}

private def needsJUnit4Fix(meth: Symbol): Boolean = {
meth.annotations.nonEmpty && JUnit4Annotations.exists(annot => meth.hasAnnotation(annot))
meth.annotations.nonEmpty && JUnit4Annotations.exists(annot => meth.hasAnnotation(annot)) &&
ctx.settings.mixinForwarderChoices.isAtLeastJunit
}

final val PrivateOrAccessor: FlagSet = Private | Accessor
Expand Down
1 change: 1 addition & 0 deletions compiler/test/dotty/tools/dotc/CompilationTests.scala
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ class CompilationTests extends ParallelTesting {
compileFilesInDir("tests/run-custom-args/Yretain-trees", defaultOptions and "-Yretain-trees") +
compileFile("tests/run-custom-args/tuple-cons.scala", allowDeepSubtypes) +
compileFile("tests/run-custom-args/i5256.scala", allowDeepSubtypes) +
compileFile("tests/run-custom-args/no-useless-forwarders.scala", defaultOptions and "-Xmixin-force-forwarders:false") +
compileFilesInDir("tests/run", defaultOptions)
}.checkRuns()

Expand Down
6 changes: 3 additions & 3 deletions tests/neg/mixin-forwarder-clash1.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<527..527> in mixin-forwarder-clash1.scala
<284..284> in mixin-forwarder-clash1.scala
Name clash between inherited members:
override def concat(suffix: Int): X in trait OneB at line 10 and
override def concat: [Dummy](suffix: Int): Y in trait TwoB at line 18
def concat(suffix: Int): X in trait One at line 4 and
def concat: [Dummy](suffix: Int): Y in trait Two at line 8
have the same type after erasure.
30 changes: 10 additions & 20 deletions tests/neg/mixin-forwarder-clash1.scala
Original file line number Diff line number Diff line change
@@ -1,38 +1,28 @@
class Foo

// Using self-types to force mixin forwarders

trait OneA[X] {
trait One[X] {
def concat(suffix: Int): X = ???
}

trait OneB[X] { self: OneA[X] =>
override def concat(suffix: Int): X = ???
}

trait TwoA[Y/* <: Foo*/] {
trait Two[Y/* <: Foo*/] {
def concat[Dummy](suffix: Int): Y = ???
}

trait TwoB[Y/* <: Foo*/] { self: TwoA[Y] =>
override def concat[Dummy](suffix: Int): Y = ???
}

class Bar1 extends OneA[Foo] with OneB[Foo]
class Bar1 extends One[Foo]
// Because mixin forwarders are generated after erasure, we get:
// override def concat(suffix: Int): Object

class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo] // error
// We get a mixin forwarder for TwoB:
class Bar2 extends Bar1 with Two[Foo] // error
// We get a mixin forwarder for Two:
// override def concat(suffix: Int): Object
// This clashes with the forwarder generated in Bar1, and the compiler detects that:
//
// |class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo]
// |class Bar2 extends Bar1 with Two[Foo]
// | ^
// | Name clash between inherited members:
// | override def concat(suffix: Int): Object in trait OneB at line 10 and
// | override def concat: [Dummy](suffix: Int): Y in trait TwoB at line 18
// | have the same type after erasure.
// | Name clash between inherited members:
// | def concat(suffix: Int): X in trait One at line 4 and
// | def concat: [Dummy](suffix: Int): Y in trait Two at line 8
// | have the same type after erasure.
//
// This also works with separate compilation as demonstrated by
// mixin-forwarder-clash2.
4 changes: 2 additions & 2 deletions tests/neg/mixin-forwarder-clash2.check
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<6..6> in B_2.scala
Name clash between inherited members:
override def concat(suffix: Int): X in trait OneB and
override def concat: [Dummy](suffix: Int): Y in trait TwoB
def concat(suffix: Int): X in trait One and
def concat: [Dummy](suffix: Int): Y in trait Two
have the same type after erasure.
16 changes: 3 additions & 13 deletions tests/neg/mixin-forwarder-clash2/A_1.scala
Original file line number Diff line number Diff line change
@@ -1,23 +1,13 @@
class Foo

// Using self-types to force mixin forwarders

trait OneA[X] {
trait One[X] {
def concat(suffix: Int): X = ???
}

trait OneB[X] { self: OneA[X] =>
override def concat(suffix: Int): X = ???
}

trait TwoA[Y/* <: Foo*/] {
trait Two[Y/* <: Foo*/] {
def concat[Dummy](suffix: Int): Y = ???
}

trait TwoB[Y/* <: Foo*/] { self: TwoA[Y] =>
override def concat[Dummy](suffix: Int): Y = ???
}

class Bar1 extends OneA[Foo] with OneB[Foo]
class Bar1 extends One[Foo]
// Because mixin forwarders are generated after erasure, we get:
// override def concat(suffix: Int): Object
8 changes: 4 additions & 4 deletions tests/neg/mixin-forwarder-clash2/B_2.scala
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo] // error
// We get a mixin forwarder for TwoB:
class Bar2 extends Bar1 with Two[Foo] // error
// We get a mixin forwarder for Two:
// override def concat(suffix: Int): Object
// This clashes with the forwarder generated in Bar1, and
// we can detect that even with separate compilation,
Expand All @@ -11,6 +11,6 @@ class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo] // error
// |class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo]
// | ^
// | Name clash between inherited members:
// | override def concat(suffix: Int): X in trait OneB and
// | override def concat: [Dummy](suffix: Int): Y in trait TwoB
// | def concat(suffix: Int): X in trait One and
// | def concat: [Dummy](suffix: Int): Y in trait Two
// | have the same type after erasure.
22 changes: 6 additions & 16 deletions tests/pos/mixin-forwarder-clash1.scala
Original file line number Diff line number Diff line change
Expand Up @@ -4,36 +4,26 @@

class Foo

// Using self-types to force mixin forwarders

trait OneA[X] {
trait One[X] {
def concat(suffix: Int): X = ???
}

trait OneB[X] { self: OneA[X] =>
override def concat(suffix: Int): X = ???
}

trait TwoA[Y <: Foo] {
trait Two[Y <: Foo] {
def concat[Dummy](suffix: Int): Y = ???
}

trait TwoB[Y <: Foo] { self: TwoA[Y] =>
override def concat[Dummy](suffix: Int): Y = ???
}

class Bar1 extends OneA[Foo] with OneB[Foo]
class Bar1 extends One[Foo]
// Because mixin forwarders are generated before erasure, we get:
// override def concat(suffix: Int): Foo

class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo] // error
// We get a mixin forwarder for TwoB:
class Bar2 extends Bar1 with Two[Foo] // error
// We get a mixin forwarder for Two:
// override def concat[Dummy](suffix: Int): Foo
// which gets erased to:
// override def concat(suffix: Int): Foo
// This clashes with the forwarder generated in Bar1, and the compiler detects that:
//
// |class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo]
// |class Bar2 extends Bar1 with Two[Foo]
// | ^
// | Name clash between defined and inherited member:
// | override def concat(suffix: Int): Foo in class Bar1 and
Expand Down
21 changes: 8 additions & 13 deletions tests/pos/mixin-forwarder-clash2/A_1.scala
Original file line number Diff line number Diff line change
@@ -1,23 +1,18 @@
class Foo
// This test case was supposed to fail when mixin forwarders were generated before erasure,
// but didn't due to separate compilation unlike mixin-forwarder-clash1,
// it's not supposed to fail anymore since the forwarders generated after erasure do not clash,
// the comments are preserved for posterity.

// Using self-types to force mixin forwarders
class Foo

trait OneA[X] {
trait One[X] {
def concat(suffix: Int): X = ???
}

trait OneB[X] { self: OneA[X] =>
override def concat(suffix: Int): X = ???
}

trait TwoA[Y <: Foo] {
trait Two[Y <: Foo] {
def concat[Dummy](suffix: Int): Y = ???
}

trait TwoB[Y <: Foo] { self: TwoA[Y] =>
override def concat[Dummy](suffix: Int): Y = ???
}

class Bar1 extends OneA[Foo] with OneB[Foo]
class Bar1 extends One[Foo]
// Because mixin forwarders are generated before erasure, we get:
// override def concat(suffix: Int): Foo
4 changes: 2 additions & 2 deletions tests/pos/mixin-forwarder-clash2/B_2.scala
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
// it's not supposed to fail anymore since the forwarders generated after erasure do not clash,
// the comments are preserved for posterity.

class Bar2 extends Bar1 with TwoA[Foo] with TwoB[Foo] // error
// We get a mixin forwarder for TwoB:
class Bar2 extends Bar1 with Two[Foo] // error
// We get a mixin forwarder for Two:
// override def concat[Dummy](suffix: Int): Foo
// which gets erased to:
// override def concat(suffix: Int): Foo
Expand Down