Skip to content

Commit b21e091

Browse files
sjrdsjrd
committed
Add a desugaring pass between the base linker and the optimizer.
Previously, the emitters and the optimizer all had to perform the same desugaring for `LinkTimeProperty` nodes. Instead, we now perform the desugaring in a dedicated phase, after the base linker. The reachability analysis records whether each method needs desugaring or not. We mark those that do so that the desugaring pass knows what to process. Methods that do not require desugaring are not processed, and so incur no additional cost. No caching is performed in `Desugarer`. It processes so few methods that caching makes it (slightly) *slower*. The machinery is heavy. It definitely outweighs the benefits in terms of duplication for `LinkTimeProperty` alone. However, the same machinery will be used to desugar `NewLambda` nodes. This commit serves as a stepping stone in that direction.
1 parent b341cc9 commit b21e091

File tree

18 files changed

+322
-60
lines changed

18 files changed

+322
-60
lines changed

‎ir/shared/src/main/scala/org/scalajs/ir/Transformers.scala

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -234,14 +234,8 @@ object Transformers {
234234
case jsMethodDef: JSMethodDef =>
235235
transformJSMethodDef(jsMethodDef)
236236

237-
case JSPropertyDef(flags, name, getterBody, setterArgAndBody) =>
238-
JSPropertyDef(
239-
flags,
240-
transform(name),
241-
transformTreeOpt(getterBody),
242-
setterArgAndBody.map { case (arg, body) =>
243-
(arg, transform(body))
244-
})(Unversioned)(jsMethodPropDef.pos)
237+
case jsPropertyDef: JSPropertyDef =>
238+
transformJSPropertyDef(jsPropertyDef)
245239
}
246240
}
247241

@@ -251,6 +245,18 @@ object Transformers {
251245
jsMethodDef.optimizerHints, Unversioned)(jsMethodDef.pos)
252246
}
253247

248+
def transformJSPropertyDef(jsPropertyDef: JSPropertyDef): JSPropertyDef = {
249+
val JSPropertyDef(flags, name, getterBody, setterArgAndBody) = jsPropertyDef
250+
JSPropertyDef(
251+
flags,
252+
transform(name),
253+
transformTreeOpt(getterBody),
254+
setterArgAndBody.map { case (arg, body) =>
255+
(arg, transform(body))
256+
}
257+
)(Unversioned)(jsPropertyDef.pos)
258+
}
259+
254260
def transformJSConstructorBody(body: JSConstructorBody): JSConstructorBody = {
255261
implicit val pos = body.pos
256262

‎linker/shared/src/main/scala/org/scalajs/linker/analyzer/Analysis.scala

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,8 @@ object Analysis {
8484
def methodInfos(
8585
namespace: MemberNamespace): scala.collection.Map[MethodName, MethodInfo]
8686

87+
def anyJSMemberNeedsDesugaring: Boolean
88+
8789
def displayName: String = className.nameString
8890
}
8991

@@ -103,6 +105,7 @@ object Analysis {
103105
def instantiatedSubclasses: scala.collection.Seq[ClassInfo]
104106
def nonExistent: Boolean
105107
def syntheticKind: MethodSyntheticKind
108+
def needsDesugaring: Boolean
106109

107110
def displayName: String = methodName.displayName
108111

@@ -161,6 +164,7 @@ object Analysis {
161164
def owningClass: ClassName
162165
def staticDependencies: scala.collection.Set[ClassName]
163166
def externalDependencies: scala.collection.Set[String]
167+
def needsDesugaring: Boolean
164168
}
165169

166170
sealed trait Error {

‎linker/shared/src/main/scala/org/scalajs/linker/analyzer/Analyzer.scala

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,9 @@ private class AnalyzerRun(config: CommonPhaseConfig, initial: Boolean,
686686
val publicMethodInfos: mutable.Map[MethodName, MethodInfo] =
687687
methodInfos(MemberNamespace.Public)
688688

689+
def anyJSMemberNeedsDesugaring: Boolean =
690+
data.jsMethodProps.exists(info => (info.globalFlags & ReachabilityInfo.FlagNeedsDesugaring) != 0)
691+
689692
def lookupAbstractMethod(methodName: MethodName): MethodInfo = {
690693
val candidatesIterator = for {
691694
ancestor <- ancestors.iterator
@@ -1285,6 +1288,9 @@ private class AnalyzerRun(config: CommonPhaseConfig, initial: Boolean,
12851288
def isDefaultBridge: Boolean =
12861289
syntheticKind.isInstanceOf[MethodSyntheticKind.DefaultBridge]
12871290

1291+
def needsDesugaring: Boolean =
1292+
(data.globalFlags & ReachabilityInfo.FlagNeedsDesugaring) != 0
1293+
12881294
/** Throws MatchError if `!isDefaultBridge`. */
12891295
def defaultBridgeTarget: ClassName = (syntheticKind: @unchecked) match {
12901296
case MethodSyntheticKind.DefaultBridge(target) => target
@@ -1367,6 +1373,9 @@ private class AnalyzerRun(config: CommonPhaseConfig, initial: Boolean,
13671373
def staticDependencies: scala.collection.Set[ClassName] = _staticDependencies.keySet
13681374
def externalDependencies: scala.collection.Set[String] = _externalDependencies.keySet
13691375

1376+
def needsDesugaring: Boolean =
1377+
(data.reachability.globalFlags & ReachabilityInfo.FlagNeedsDesugaring) != 0
1378+
13701379
def reach(): Unit = followReachabilityInfo(data.reachability, this)(FromExports)
13711380
}
13721381

@@ -1441,7 +1450,7 @@ private class AnalyzerRun(config: CommonPhaseConfig, initial: Boolean,
14411450
}
14421451
}
14431452

1444-
val globalFlags = data.globalFlags
1453+
val globalFlags = data.globalFlags & ~ReachabilityInfo.FlagNeedsDesugaring
14451454

14461455
if (globalFlags != 0) {
14471456
if ((globalFlags & ReachabilityInfo.FlagAccessedClassClass) != 0) {

‎linker/shared/src/main/scala/org/scalajs/linker/analyzer/Infos.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ object Infos {
115115
final val FlagAccessedImportMeta = 1 << 2
116116
final val FlagUsedExponentOperator = 1 << 3
117117
final val FlagUsedClassSuperClass = 1 << 4
118+
final val FlagNeedsDesugaring = 1 << 5
118119
}
119120

120121
/** Things from a given class that are reached by one method. */
@@ -395,6 +396,7 @@ object Infos {
395396
setFlag(ReachabilityInfo.FlagUsedClassSuperClass)
396397

397398
def addReferencedLinkTimeProperty(linkTimeProperty: LinkTimeProperty): this.type = {
399+
setFlag(ReachabilityInfo.FlagNeedsDesugaring)
398400
linkTimeProperties.append((linkTimeProperty.name, linkTimeProperty.tpe))
399401
this
400402
}

‎linker/shared/src/main/scala/org/scalajs/linker/backend/emitter/FunctionEmitter.scala

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,9 +1260,8 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
12601260

12611261
def test(tree: Tree): Boolean = tree match {
12621262
// Atomic expressions
1263-
case _: Literal => true
1264-
case _: JSNewTarget => true
1265-
case _: LinkTimeProperty => true
1263+
case _: Literal => true
1264+
case _: JSNewTarget => true
12661265

12671266
// Vars (side-effect free, pure if immutable)
12681267
case VarRef(name) =>
@@ -2811,11 +2810,6 @@ private[emitter] class FunctionEmitter(sjsGen: SJSGen) {
28112810
case AsInstanceOf(expr, tpe) =>
28122811
extractWithGlobals(genAsInstanceOf(transformExprNoChar(expr), tpe))
28132812

2814-
case prop: LinkTimeProperty =>
2815-
transformExpr(
2816-
config.coreSpec.linkTimeProperties.transformLinkTimeProperty(prop),
2817-
preserveChar)
2818-
28192813
// Transients
28202814

28212815
case Transient(Cast(expr, tpe)) =>

‎linker/shared/src/main/scala/org/scalajs/linker/backend/wasmemitter/DerivedClasses.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,7 @@ object DerivedClasses {
146146
staticDependencies = Set.empty,
147147
externalDependencies = Set.empty,
148148
dynamicDependencies = Set.empty,
149+
desugaringInfo = LinkedClass.DesugaringInfo.Empty,
149150
clazz.version
150151
)
151152
}

‎linker/shared/src/main/scala/org/scalajs/linker/backend/wasmemitter/FunctionEmitter.scala

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -550,7 +550,6 @@ private class FunctionEmitter private (
550550
case t: Match => genMatch(t, expectedType)
551551
case t: Debugger => VoidType // ignore
552552
case t: Skip => VoidType
553-
case t: LinkTimeProperty => genLinkTimeProperty(t)
554553

555554
// JavaScript expressions
556555
case t: JSNew => genJSNew(t)
@@ -590,7 +589,7 @@ private class FunctionEmitter private (
590589
// Transients (only generated by the optimizer)
591590
case t: Transient => genTransient(t)
592591

593-
case _: JSSuperConstructorCall =>
592+
case _:JSSuperConstructorCall | _:LinkTimeProperty =>
594593
throw new AssertionError(s"Invalid tree: $tree")
595594
}
596595

@@ -2649,12 +2648,6 @@ private class FunctionEmitter private (
26492648
ClassType(boxClassName, nullable = false)
26502649
}
26512650

2652-
private def genLinkTimeProperty(tree: LinkTimeProperty): Type = {
2653-
val lit = ctx.coreSpec.linkTimeProperties.transformLinkTimeProperty(tree)
2654-
genLiteral(lit, lit.tpe)
2655-
lit.tpe
2656-
}
2657-
26582651
private def genJSNew(tree: JSNew): Type = {
26592652
val JSNew(ctor, args) = tree
26602653

‎linker/shared/src/main/scala/org/scalajs/linker/checker/CheckingPhase.scala

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,6 @@ sealed abstract class CheckingPhase
2424
object CheckingPhase {
2525
case object Compiler extends CheckingPhase
2626
case object BaseLinker extends CheckingPhase
27+
case object Desugarer extends CheckingPhase
2728
case object Optimizer extends CheckingPhase
2829
}

‎linker/shared/src/main/scala/org/scalajs/linker/checker/ClassDefChecker.scala

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -855,6 +855,8 @@ private final class ClassDefChecker(classDef: ClassDef,
855855
}
856856

857857
case LinkTimeProperty(name) =>
858+
if (!featureSet.supports(FeatureSet.LinkTimeProperty))
859+
reportError(i"Illegal link-time property '$name' after desugaring")
858860

859861
// JavaScript expressions
860862

‎linker/shared/src/main/scala/org/scalajs/linker/checker/FeatureSet.scala

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,20 @@ private[checker] object FeatureSet {
3838

3939
// Individual features
4040

41+
/** The `LinkTimeProperty` IR node. */
42+
val LinkTimeProperty = new FeatureSet(1 << 0)
43+
4144
/** Optional constructors in module classes and JS classes. */
42-
val OptionalConstructors = new FeatureSet(1 << 0)
45+
val OptionalConstructors = new FeatureSet(1 << 1)
4346

4447
/** Explicit reflective proxy definitions. */
45-
val ReflectiveProxies = new FeatureSet(1 << 1)
48+
val ReflectiveProxies = new FeatureSet(1 << 2)
4649

4750
/** Transients that are the result of optimizations. */
48-
val OptimizedTransients = new FeatureSet(1 << 2)
51+
val OptimizedTransients = new FeatureSet(1 << 3)
4952

5053
/** Records and record types. */
51-
val Records = new FeatureSet(1 << 3)
54+
val Records = new FeatureSet(1 << 4)
5255

5356
/** Relaxed constructor discipline.
5457
*
@@ -57,22 +60,31 @@ private[checker] object FeatureSet {
5760
* - `this.x = ...` assignments before the delegate call can assign super class fields.
5861
* - `StoreModule` can be anywhere, or not be there at all.
5962
*/
60-
val RelaxedCtorBodies = new FeatureSet(1 << 4)
63+
val RelaxedCtorBodies = new FeatureSet(1 << 5)
6164

6265
// Common sets
6366

6467
/** Features introduced by the base linker. */
6568
private val Linked =
6669
OptionalConstructors | ReflectiveProxies
6770

71+
/** Features that must be desugared away. */
72+
private val NeedsDesugaring =
73+
LinkTimeProperty
74+
75+
/** IR that is only the result of desugaring (currently empty). */
76+
private val Desugared =
77+
Empty
78+
6879
/** IR that is only the result of optimizations. */
6980
private val Optimized =
7081
OptimizedTransients | Records | RelaxedCtorBodies
7182

7283
/** The set of features allowed as output of the given phase. */
7384
def allowedAfter(phase: CheckingPhase): FeatureSet = phase match {
74-
case Compiler => Empty
75-
case BaseLinker => Linked
76-
case Optimizer => Linked | Optimized
85+
case Compiler => NeedsDesugaring
86+
case BaseLinker => Linked | NeedsDesugaring
87+
case Desugarer => Linked | Desugared
88+
case Optimizer => Linked | Desugared | Optimized
7789
}
7890
}

‎linker/shared/src/main/scala/org/scalajs/linker/checker/IRChecker.scala

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -577,7 +577,7 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
577577
typecheckAny(expr, env)
578578
checkIsAsInstanceTargetType(tpe)
579579

580-
case LinkTimeProperty(name) =>
580+
case LinkTimeProperty(name) if featureSet.supports(FeatureSet.LinkTimeProperty) =>
581581

582582
// JavaScript expressions
583583

@@ -760,7 +760,8 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
760760
typecheck(elem, env)
761761
}
762762

763-
case _:RecordSelect | _:RecordValue | _:Transient | _:JSSuperConstructorCall =>
763+
case _:RecordSelect | _:RecordValue | _:Transient |
764+
_:JSSuperConstructorCall | _:LinkTimeProperty =>
764765
reportError("invalid tree")
765766
}
766767
}

‎linker/shared/src/main/scala/org/scalajs/linker/frontend/BaseLinker.scala

Lines changed: 25 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -129,17 +129,23 @@ private[frontend] object BaseLinker {
129129
classInfo.isAnySubclassInstantiated
130130
}
131131

132-
val methods = classDef.methods.filter { m =>
133-
val methodInfo =
134-
classInfo.methodInfos(m.flags.namespace)(m.methodName)
135-
136-
val reachable = methodInfo.isReachable
137-
assert(m.body.isDefined || !reachable,
138-
s"The abstract method ${classDef.name.name}.${m.methodName} " +
139-
"is reachable.")
140-
141-
reachable
142-
}
132+
// var of immutable data type because it will stay empty for most classes
133+
var methodsDesugaring = LinkedClass.DesugaringInfo.Empty.methods
134+
135+
val methods: List[MethodDef] = classDef.methods.iterator
136+
.map(m => m -> classInfo.methodInfos(m.flags.namespace)(m.methodName))
137+
.filter(_._2.isReachable)
138+
.map { case (m, info) =>
139+
assert(m.body.isDefined,
140+
s"The abstract method ${classDef.name.name}.${m.methodName} is reachable.")
141+
if (info.needsDesugaring) {
142+
val namespaceOrdinal = m.flags.namespace.ordinal
143+
methodsDesugaring = methodsDesugaring.updated(namespaceOrdinal,
144+
methodsDesugaring(namespaceOrdinal) + m.methodName)
145+
}
146+
m
147+
}
148+
.toList
143149

144150
val jsConstructor =
145151
if (classInfo.isAnySubclassInstantiated) classDef.jsConstructor
@@ -156,6 +162,11 @@ private[frontend] object BaseLinker {
156162

157163
val ancestors = classInfo.ancestors.map(_.className)
158164

165+
val desugaringInfo = new LinkedClass.DesugaringInfo(
166+
methods = methodsDesugaring,
167+
exportedMembers = classInfo.anyJSMemberNeedsDesugaring
168+
)
169+
159170
val linkedClass = new LinkedClass(
160171
classDef.name,
161172
classDef.kind,
@@ -181,6 +192,7 @@ private[frontend] object BaseLinker {
181192
staticDependencies = classInfo.staticDependencies.toSet,
182193
externalDependencies = classInfo.externalDependencies.toSet,
183194
dynamicDependencies = classInfo.dynamicDependencies.toSet,
195+
desugaringInfo,
184196
version)
185197

186198
val linkedTopLevelExports = for {
@@ -189,7 +201,8 @@ private[frontend] object BaseLinker {
189201
val infos = analysis.topLevelExportInfos(
190202
(ModuleID(topLevelExport.moduleID), topLevelExport.topLevelExportName))
191203
new LinkedTopLevelExport(classDef.className, topLevelExport,
192-
infos.staticDependencies.toSet, infos.externalDependencies.toSet)
204+
infos.staticDependencies.toSet, infos.externalDependencies.toSet,
205+
needsDesugaring = infos.needsDesugaring)
193206
}
194207

195208
(linkedClass, linkedTopLevelExports)

0 commit comments

Comments
 (0)