Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 9bd7774

Browse files
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-
valmemberTpe =
399+
defcomputeMemberTpe():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)

AltStyle によって変換されたページ (->オリジナル) /