Skip to content

Commit 9bd7774

Browse files
authored
Emit mixin forwarders as ordinary, non-bridge methods again (#23942)
Lets try to merge again #21890
2 parents 834e8e9 + aa267b2 commit 9bd7774

File tree

35 files changed

+220
-135
lines changed

35 files changed

+220
-135
lines changed

.github/workflows/stdlib.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,7 @@ jobs:
304304
mima-scala-library-nonbootstrapped:
305305
runs-on: ubuntu-latest
306306
needs: scala-library-nonbootstrapped
307+
if: false
307308
steps:
308309
- name: Git Checkout
309310
uses: actions/checkout@v5

.gitmodules

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@
100100
[submodule "community-build/community-projects/scala-parallel-collections"]
101101
path = community-build/community-projects/scala-parallel-collections
102102
url = https://github.com/dotty-staging/scala-parallel-collections.git
103+
branch = serialisation-stability-fix
103104
[submodule "community-build/community-projects/scala-collection-compat"]
104105
path = community-build/community-projects/scala-collection-compat
105106
url = https://github.com/dotty-staging/scala-collection-compat.git

compiler/src/dotty/tools/backend/jvm/BCodeHelpers.scala

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import dotty.tools.dotc.core.Types.*
3131
import dotty.tools.dotc.core.TypeErasure
3232
import dotty.tools.dotc.transform.GenericSignatures
3333
import dotty.tools.dotc.transform.ElimErasedValueType
34+
import dotty.tools.dotc.transform.Mixin
3435
import dotty.tools.io.AbstractFile
3536
import dotty.tools.dotc.report
3637

@@ -395,12 +396,20 @@ trait BCodeHelpers extends BCodeIdiomatic {
395396
*/
396397
def getGenericSignature(sym: Symbol, owner: Symbol): String = {
397398
atPhase(erasurePhase) {
398-
val memberTpe =
399+
def computeMemberTpe(): Type =
399400
if (sym.is(Method)) sym.denot.info
400401
else if sym.denot.validFor.phaseId > erasurePhase.id && sym.isField && sym.getter.exists then
401402
// Memoization field of getter entered after erasure, see run/i17069 for an example
402403
sym.getter.denot.info.resultType
403404
else owner.denot.thisType.memberInfo(sym)
405+
406+
val memberTpe = if sym.is(MixedIn) then
407+
mixinPhase.asInstanceOf[Mixin].mixinForwarderGenericInfos.get(sym) match
408+
case Some(genericInfo) => genericInfo
409+
case none => computeMemberTpe()
410+
else
411+
computeMemberTpe()
412+
404413
getGenericSignatureHelper(sym, owner, memberTpe).orNull
405414
}
406415
}
@@ -491,7 +500,7 @@ trait BCodeHelpers extends BCodeIdiomatic {
491500
report.debuglog(s"Potentially conflicting names for forwarders: $conflictingNames")
492501

493502
for (m0 <- sortedMembersBasedOnFlags(moduleClass.info, required = Method, excluded = ExcludedForwarder)) {
494-
val m = if (m0.is(Bridge)) m0.nextOverriddenSymbol else m0
503+
val m = if (m0.isOneOf(Bridge | MixedIn)) m0.nextOverriddenSymbol else m0
495504
if (m == NoSymbol)
496505
report.log(s"$m0 is a bridge method that overrides nothing, something went wrong in a previous phase.")
497506
else if (m.isType || m.is(Deferred) || (m.owner eq defn.ObjectClass) || m.isConstructor || m.name.is(ExpandedName))
@@ -507,10 +516,7 @@ trait BCodeHelpers extends BCodeIdiomatic {
507516
// we generate ACC_SYNTHETIC forwarders so Java compilers ignore them.
508517
val isSynthetic =
509518
m0.name.is(NameKinds.SyntheticSetterName) ||
510-
// Only hide bridges generated at Erasure, mixin forwarders are also
511-
// marked as bridge but shouldn't be hidden since they don't have a
512-
// non-bridge overload.
513-
m0.is(Bridge) && m0.initial.validFor.firstPhaseId == erasurePhase.next.id
519+
m0.is(Bridge)
514520
addForwarder(jclass, moduleClass, m, isSynthetic)
515521
}
516522
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ class BTypesFromSymbols[I <: DottyBackendInterface](val int: I, val frontendAcce
299299
// illegal combination of modifiers at the bytecode level so
300300
// suppress final if abstract if present.
301301
&& !sym.isOneOf(AbstractOrTrait)
302-
// Mixin forwarders are bridges and can be final, but final bridges confuse some frameworks
302+
// Bridges can be final, but final bridges confuse some frameworks
303303
&& !sym.is(Bridge), ACC_FINAL)
304304
.addFlagIf(sym.isStaticMember, ACC_STATIC)
305305
.addFlagIf(sym.is(Bridge), ACC_BRIDGE | ACC_SYNTHETIC)

compiler/src/dotty/tools/dotc/core/Phases.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,7 @@ object Phases {
239239
private var myErasurePhase: Phase = uninitialized
240240
private var myElimErasedValueTypePhase: Phase = uninitialized
241241
private var myLambdaLiftPhase: Phase = uninitialized
242+
private var myMixinPhase: Phase = uninitialized
242243
private var myCountOuterAccessesPhase: Phase = uninitialized
243244
private var myFlattenPhase: Phase = uninitialized
244245
private var myGenBCodePhase: Phase = uninitialized
@@ -266,6 +267,7 @@ object Phases {
266267
final def gettersPhase: Phase = myGettersPhase
267268
final def erasurePhase: Phase = myErasurePhase
268269
final def elimErasedValueTypePhase: Phase = myElimErasedValueTypePhase
270+
final def mixinPhase: Phase = myMixinPhase
269271
final def lambdaLiftPhase: Phase = myLambdaLiftPhase
270272
final def countOuterAccessesPhase = myCountOuterAccessesPhase
271273
final def flattenPhase: Phase = myFlattenPhase
@@ -295,6 +297,7 @@ object Phases {
295297
myErasurePhase = phaseOfClass(classOf[Erasure])
296298
myElimErasedValueTypePhase = phaseOfClass(classOf[ElimErasedValueType])
297299
myPatmatPhase = phaseOfClass(classOf[PatternMatcher])
300+
myMixinPhase = phaseOfClass(classOf[Mixin])
298301
myLambdaLiftPhase = phaseOfClass(classOf[LambdaLift])
299302
myCountOuterAccessesPhase = phaseOfClass(classOf[CountOuterAccesses])
300303
myFlattenPhase = phaseOfClass(classOf[Flatten])
@@ -550,6 +553,7 @@ object Phases {
550553
def gettersPhase(using Context): Phase = ctx.base.gettersPhase
551554
def erasurePhase(using Context): Phase = ctx.base.erasurePhase
552555
def elimErasedValueTypePhase(using Context): Phase = ctx.base.elimErasedValueTypePhase
556+
def mixinPhase(using Context): Phase = ctx.base.mixinPhase
553557
def lambdaLiftPhase(using Context): Phase = ctx.base.lambdaLiftPhase
554558
def flattenPhase(using Context): Phase = ctx.base.flattenPhase
555559
def genBCodePhase(using Context): Phase = ctx.base.genBCodePhase

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

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import StdNames.*
1616
import Names.*
1717
import NameKinds.*
1818
import NameOps.*
19+
import Phases.erasurePhase
1920
import ast.Trees.*
2021

2122
import dotty.tools.dotc.transform.sjs.JSSymUtils.isJSType
@@ -115,6 +116,15 @@ object Mixin {
115116
class Mixin extends MiniPhase with SymTransformer { thisPhase =>
116117
import ast.tpd.*
117118

119+
/** Infos before erasure of the generated mixin forwarders.
120+
*
121+
* These will be used to generate Java generic signatures of the mixin
122+
* forwarders. Normally we use the types before erasure; we cannot do that
123+
* for mixin forwarders since they are created after erasure, and therefore
124+
* their type history does not have anything recorded for before erasure.
125+
*/
126+
val mixinForwarderGenericInfos = MutableSymbolMap[Type]()
127+
118128
override def phaseName: String = Mixin.name
119129

120130
override def description: String = Mixin.description
@@ -305,8 +315,25 @@ class Mixin extends MiniPhase with SymTransformer { thisPhase =>
305315
for (meth <- mixin.info.decls.toList if needsMixinForwarder(meth))
306316
yield {
307317
util.Stats.record("mixin forwarders")
308-
transformFollowing(DefDef(mkForwarderSym(meth.asTerm, Bridge), forwarderRhsFn(meth)))
318+
transformFollowing(DefDef(mkMixinForwarderSym(meth.asTerm), forwarderRhsFn(meth)))
319+
}
320+
321+
def mkMixinForwarderSym(target: TermSymbol): TermSymbol =
322+
val sym = mkForwarderSym(target, extraFlags = MixedIn)
323+
val (infoBeforeErasure, isDifferentThanInfoNow) = atPhase(erasurePhase) {
324+
val beforeErasure = cls.thisType.memberInfo(target)
325+
(beforeErasure, !(beforeErasure =:= sym.info))
309326
}
327+
if isDifferentThanInfoNow then
328+
// The info before erasure would not have been the same as the info now.
329+
// We want to store it for the backend to compute the generic Java signature.
330+
// However, we must still avoid doing that if erasing that signature would
331+
// not give the same erased type. If it doesn't, we'll just give a completely
332+
// incorrect Java signature. (This could be improved by generating dedicated
333+
// bridges, but we don't go that far; scalac doesn't either.)
334+
if TypeErasure.transformInfo(target, infoBeforeErasure) =:= sym.info then
335+
mixinForwarderGenericInfos(sym) = infoBeforeErasure
336+
sym
310337

311338
cpy.Template(impl)(
312339
constr =

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package transform
33

44
import core.*
55
import Symbols.*, Types.*, Contexts.*, DenotTransformers.*, Flags.*
6+
import NameKinds.*
67
import util.Spans.*
78

89
import StdNames.*, NameOps.*
@@ -71,7 +72,8 @@ class MixinOps(cls: ClassSymbol, thisPhase: DenotTransformer)(using Context) {
7172
meth.is(Method, butNot = PrivateOrAccessorOrDeferred) &&
7273
(ctx.settings.mixinForwarderChoices.isTruthy || meth.owner.is(Scala2x) || needsDisambiguation || hasNonInterfaceDefinition ||
7374
generateJUnitForwarder || generateSerializationForwarder) &&
74-
isInImplementingClass(meth)
75+
isInImplementingClass(meth) &&
76+
!meth.name.is(InlineAccessorName)
7577
}
7678

7779
final val PrivateOrAccessor: FlagSet = Private | Accessor

compiler/test/dotc/run-test-pickling.excludelist

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,11 @@ i9473.scala
1212
i13433.scala
1313
i13433b.scala
1414
macros-in-same-project1
15-
mixin-forwarder-overload
1615
t10889
1716
t3452d
1817
t3452e
1918
t3452g
2019
t7374
21-
t8905
2220
tuple-drop.scala
2321
tuple-ops.scala
2422
tuple-ops.scala

project/MiMaFilters.scala

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -559,28 +559,11 @@ object MiMaFilters {
559559
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.StringView.compose"),
560560
ProblemFilters.exclude[DirectMissingMethodProblem]("scala.collection.StringView.andThen"),
561561

562-
// This issue only arise in the non-bootstrapped stdlib
563-
// It has to do with the fact that the special erasure of Pure was handled such as
564-
// `scala.Pure`, not `scala.caps.Pure`. This filter should be removed once we move to 3.8.1
565-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.collection.Map.from"),
566-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.collection.SeqMap.from"),
567-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.collection.mutable.Map.from"),
568-
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.collection.mutable.SeqMap.from"),
569-
570562
// TO INVESTIGATE: This constructor changed, but it is private... why complaining?
571563
ProblemFilters.exclude[IncompatibleMethTypeProblem]("scala.collection.immutable.LazyList.this"),
572564
// This one should be fine, public class inside private object
573565
ProblemFilters.exclude[IncompatibleResultTypeProblem]("scala.collection.immutable.LazyList#LazyBuilder#DeferredState.eval"),
574566

575-
// MIX IN FORWARDERS ISSUE (SHOULD BE FIXED WHEN WE REMERGE THE PR)
576-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.StrictOptimizedSeqOps.prepended"),
577-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.StrictOptimizedSeqOps.appended"),
578-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.StrictOptimizedSeqOps.appendedAll"),
579-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.StrictOptimizedSeqOps.prependedAll"),
580-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.StrictOptimizedSeqOps.padTo"),
581-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.immutable.StrictOptimizedSeqOps.updated"),
582-
ProblemFilters.exclude[NewMixinForwarderProblem]("scala.collection.immutable.StrictOptimizedSeqOps.patch"),
583-
584567
// NO IDEA FOR NOW :)
585568
ProblemFilters.exclude[ReversedMissingMethodProblem]("scala.collection.mutable.ArrayDequeOps.scala$collection$mutable$ArrayDequeOps$$super$sliding"),
586569
),

0 commit comments

Comments
 (0)