Skip to content

Commit

Permalink
Fix #2563: Unconditionally emit mixin forwarders
Browse files Browse the repository at this point in the history
This is what Scala 2.12+ does for cold performance reasons (see #5928
for more details) and we should align ourselves with them when possible.

About two years ago in #2563, Dmitry objected that we may not need to do
that if we found another way to get performance back, or if newer JDKs
improved the performance of default method resolution. It doesn't look
like these things have happened so far (but there's some recent glimmer
of hope here: https://bugs.openjdk.java.net/browse/JDK-8036580).

Dmitry also said "I don't recall triggering bugs by emitting more
forwarders rather then less.", but in fact since then I've found one
case where the standard library failed to compile with extra forwarders
causing name clashes, requiring a non-binary-compatible change:
scala/scala@e3ef657.
As of #6079 this is no longer a problem since we now emit mixin
forwarders after erasure like scalac, but it still seems prudent to emit
as many forwarders as scalac to catch potential name clash issues.
  • Loading branch information
smarter committed Mar 18, 2019
1 parent cdbee63 commit 6c64430
Show file tree
Hide file tree
Showing 3 changed files with 2 additions and 1 deletion.
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 @@ -82,7 +82,7 @@ class ScalaSettings extends Settings.SettingGroup {
helpArg = "mode",
descr = "Generate forwarder methods in classes inhering concrete methods from traits.",
choices = List("true", "junit", "false"),
default = "junit")
default = "true")

object mixinForwarderChoices {
def isTruthy(implicit ctx: Context) = XmixinForceForwarders.value == "true"
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
File renamed without changes.

0 comments on commit 6c64430

Please sign in to comment.