8000 Refactoring: Generic notion of `CheckingPhase` for the IR checkers. · scala-js/scala-js@a738094 · GitHub
[go: up one dir, main page]
Skip to content

Commit a738094

Browse files
sjrdsjrd
committed
Refactoring: Generic notion of CheckingPhase for the IR checkers.
It was bad enough that we had two boolean options for deciding what IR is allowed or not between phases. There was already one of the four combinations that was invalid. With the introduction of the desugarer, we had *three* such boolean options. Only 4 out of their 8 combinations were valid. We now introduce a generic concept of `CheckingPhase`, which knows what conditional IR features it *accepts*. This makes a lot of things clearer.
1 parent 009b2ef commit a738094

File tree

14 files changed

+182
-88
lines changed

14 files changed

+182
-88
lines changed

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

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import org.scalajs.ir.Trees.{MemberNamespace, JSNativeLoadSpec}
2929
import org.scalajs.ir.Types.ClassRef
3030

3131
import org.scalajs.linker._
32+
import org.scalajs.linker.checker.CheckingPhase
3233
import org.scalajs.linker.frontend.IRLoader
3334
import org.scalajs.linker.interface._
3435
import org.scalajs.linker.interface.unstable.ModuleInitializerImpl
@@ -43,15 +44,10 @@ import Analysis._
4344
import Infos.{NamespacedMethodName, ReachabilityInfo, ReachabilityInfoInClass}
4445

4546
final class Analyzer(config: CommonPhaseConfig, initial: Boolean,
46-
checkIR: Boolean, failOnError: Boolean, irLoader: IRLoader) {
47-
48-
private val infoLoader: InfoLoader = {
49-
new InfoLoader(irLoader,
50-
if (!checkIR) InfoLoader.NoIRCheck
51-
else if (initial) InfoLoader.InitialIRCheck
52-
else InfoLoader.InternalIRCheck
53-
)
54-
}
47+
checkIRFor: Option[CheckingPhase], failOnError: Boolean, irLoader: IRLoader) {
48+
49+
private val infoLoader: InfoLoader =
50+
new InfoLoader(irLoader, checkIRFor)
5551

5652
def computeReachability(moduleInitializers: Seq[ModuleInitializer],
5753
symbolRequirements: SymbolRequirement, logger: Logger)(implicit ec: ExecutionContext): Future[Analysis] = {

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

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,14 @@ import org.scalajs.ir.Trees._
2222

2323
import org.scalajs.logging._
2424

25-
import org.scalajs.linker.checker.ClassDefChecker
25+
import org.scalajs.linker.checker._
2626
import org.scalajs.linker.frontend.IRLoader
2727
import org.scalajs.linker.interface.LinkingException
2828
import org.scalajs.linker.CollectionsCompat.MutableMapCompatOps
2929

3030
import Platform.emptyThreadSafeMap
3131

32-
private[analyzer] final class InfoLoader(irLoader: IRLoader, irCheckMode: InfoLoader.IRCheckMode) {
32+
private[analyzer] final class InfoLoader(irLoader: IRLoader, checkIRFor: Option[CheckingPhase]) {
3333
private var logger: Logger = _
3434
private val cache = emptyThreadSafeMap[ClassName, InfoLoader.ClassInfoCache]
3535

@@ -44,7 +44,7 @@ private[analyzer] final class InfoLoader(irLoader: IRLoader, irCheckMode: InfoLo
4444
implicit ec: ExecutionContext): Option[Future[Infos.ClassInfo]] = {
4545
if (irLoader.classExists(className)) {
4646
val infoCache = cache.getOrElseUpdate(className,
47-
new InfoLoader.ClassInfoCache(className, irLoader, irCheckMode))
47+
new InfoLoader.ClassInfoCache(className, irLoader, checkIRFor))
4848
Some(infoCache.loadInfo(logger))
4949
} else {
5050
None
@@ -58,15 +58,9 @@ private[analyzer] final class InfoLoader(irLoader: IRLoader, irCheckMode: InfoLo
5858
}
5959

6060
private[analyzer] object InfoLoader {
61-
sealed trait IRCheckMode
62-
63-
case object NoIRCheck extends IRCheckMode
64-
case object InitialIRCheck extends IRCheckMode
65-
case object InternalIRCheck extends IRCheckMode
66-
6761
private type MethodInfos = Array[Map[MethodName, Infos.MethodInfo]]
6862

69-
private class ClassInfoCache(className: ClassName, irLoader: IRLoader, irCheckMode: InfoLoader.IRCheckMode) {
63+
private class ClassInfoCache(className: ClassName, irLoader: IRLoader, checkIRFor: Option[CheckingPhase]) {
7064
private var cacheUsed: Boolean = false
7165
private var version: Version = Version.Unversioned
7266
private var info: Future[Infos.ClassInfo] = _
@@ -86,26 +80,18 @@ private[analyzer] object InfoLoader {
8680
if (!version.sameVersion(newVersion)) {
8781
version = newVersion
8882
info = irLoader.loadClassDef(className).map { tree =>
89-
irCheckMode match {
90-
case InfoLoader.NoIRCheck =>
91-
// no check
92-
93-
case InfoLoader.InitialIRCheck =>
94-
val errorCount = ClassDefChecker.check(tree,
95-
postBaseLinker = false, postOptimizer = false, logger)
96-
if (errorCount != 0) {
83+
for (nextPhase <- checkIRFor) {
84+
val errorCount = ClassDefChecker.check(tree, nextPhase, logger)
85+
if (errorCount != 0) {
86+
if (nextPhase == CheckingPhase.BaseLinker) {
9787
throw new LinkingException(
9888
s"There were $errorCount ClassDef checking errors.")
99-
}
100-
101-
case InfoLoader.InternalIRCheck =>
102-
val errorCount = ClassDefChecker.check(tree,
103-
postBaseLinker = true, postOptimizer = true, logger)
104-
if (errorCount != 0) {
89+
} else {
10590
throw new LinkingException(
106-
s"There were $errorCount ClassDef checking errors after optimizing. " +
91+
s"There were $errorCount ClassDef checking errors after transformations. " +
10792
"Please report this as a bug.")
10893
}
94+
}
10995
}
11096

11197
generateInfos(tree)

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

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,68 @@
1+
/*
2+
* Scala.js (https://www.scala-js.org/)
3+
*
4+
* Copyright EPFL.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (https://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package org.scalajs.linker.checker
14+
15+
/** A phase *before which* we are checking IR for.
16+
*
17+
* When checking IR (with `ClassDefChecker` or `IRChecker`), different nodes
18+
* and transients are allowed between different phases. The `CheckingPhase`
19+
* records the *next* phase to run after the check. We are therefore checking
20+
* that the IR is a valid *input* to the target phase.
21+
*/
22+
abstract class CheckingPhase private[checker] () {
23+
def accept(feature: IRFeature): Boolean
24+
}
25+
26+
object CheckingPhase {
27+
object BaseLinker extends CheckingPhase {
28+
def accept(feature: IRFeature): Boolean = feature match {
29+
case IRFeature.NeedsDesugaring =>
30+
true
31+
case _ =>
32+
false
33+
}
34+
}
35+
36+
object Desugarer extends CheckingPhase {
37+
def accept(feature: IRFeature): Boolean = feature match {
38+
case IRFeature.Linked | IRFeature.NeedsDesugaring =>
39+
true
40+
case _ =>
41+
false
42+
}
43+
}
44+
45+
final class Emitter(afterOptimizer: Boolean) extends CheckingPhase {
46+
def accept(feature: IRFeature): Boolean = feature match {
47+
case IRFeature.Linked | IRFeature.Desugared =>
48+
true
49+
case IRFeature.Optimized =>
50+
afterOptimizer
51+
case _ =>
52+
false
53+
}
54+
}
55+
56+
object Emitter {
57+
def apply(afterOptimizer: Boolean): Emitter =
58+
new Emitter(afterOptimizer)
59+
}
60+
61+
object Optimizer extends CheckingPhase {
62+
// The optimizer must accept everythin an emitter without optimizer accepts
63+
private val delegate = new Emitter(afterOptimizer = false)
64+
65+
def accept(feature: IRFeature): Boolean =
66+
delegate.accept(feature)
67+
}
68+
}

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

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import org.scalajs.linker.standard.LinkedClass
2828

2929
/** Checker for the validity of the IR. */
3030
private final class ClassDefChecker(classDef: ClassDef,
31-
postBaseLinker: Boolean, postOptimizer: Boolean, reporter: ErrorReporter) {
31+
nextPhase: CheckingPhase, reporter: ErrorReporter) {
3232
import ClassDefChecker._
3333

3434
import reporter.reportError
@@ -107,7 +107,7 @@ private final class ClassDefChecker(classDef: ClassDef,
107107
* module classes and JS classes that are never instantiated. The classes
108108
* may still exist because their class data are accessed.
109109
*/
110-
if (!postBaseLinker) {
110+
if (!nextPhase.accept(IRFeature.Linked)) {
111111
/* Check that we have exactly 1 constructor in a module class. This goes
112112
* together with `checkMethodDef`, which checks that a constructor in a
113113
* module class must be 0-arg.
@@ -489,7 +489,7 @@ private final class ClassDefChecker(classDef: ClassDef,
489489
private def checkMethodNameNamespace(name: MethodName, namespace: MemberNamespace)(
490490
implicit ctx: ErrorContext): Unit = {
491491
if (name.isReflectiveProxy) {
492-
if (postBaseLinker) {
492+
if (nextPhase.accept(IRFeature.Linked)) {
493493
if (namespace != MemberNamespace.Public)
494494
reportError("reflective profixes are only allowed in the public namespace")
495495
} else {
@@ -557,7 +557,7 @@ private final class ClassDefChecker(classDef: ClassDef,
557557
}
558558

559559
if (rest.isEmpty) {
560-
if (!postOptimizer)
560+
if (!nextPhase.accept(IRFeature.Optimized))
561561
reportError(i"Constructor must contain a delegate constructor call")
562562

563563
val bodyStatsStoreModulesHandled =
@@ -576,7 +576,7 @@ private final class ClassDefChecker(classDef: ClassDef,
576576

577577
checkTree(receiver, unrestrictedEnv) // check that the This itself is valid
578578

579-
if (!postOptimizer) {
579+
if (!nextPhase.accept(IRFeature.Optimized)) {
580580
if (!(cls == classDef.className || classDef.superClass.exists(_.name == cls))) {
581581
implicit val ctx = ErrorContext(delegateCtorCall)
582582
reportError(
@@ -597,7 +597,7 @@ private final class ClassDefChecker(classDef: ClassDef,
597597
implicit ctx: ErrorContext): List[Tree] = {
598598

599599
if (classDef.kind.hasModuleAccessor) {
600-
if (postOptimizer) {
600+
if (nextPhase.accept(IRFeature.Optimized)) {
601601
/* If the super constructor call was inlined, the StoreModule can be anywhere.
602602
* Moreover, the optimizer can remove StoreModules altogether in many cases.
603603
*/
@@ -673,7 +673,7 @@ private final class ClassDefChecker(classDef: ClassDef,
673673
case Assign(lhs, rhs) =>
674674
lhs match {
675675
case Select(This(), field) if env.isThisRestricted =>
676-
if (postOptimizer || field.name.className == classDef.className)
676+
if (nextPhase.accept(IRFeature.Optimized) || field.name.className == classDef.className)
677677
checkTree(lhs, env.withIsThisRestricted(false))
678678
else
679679
checkTree(lhs, env)
@@ -851,8 +851,8 @@ private final class ClassDefChecker(classDef: ClassDef,
851851
}
852852

853853
case LinkTimeProperty(name) =>
854-
if (postBaseLinker)
855-
reportError(i"Illegal link-time property '$name' post base linker")
854+
if (!nextPhase.accept(IRFeature.NeedsDesugaring))
855+
reportError(i"Illegal link-time property '$name' after desugaring")
856856

857857
// JavaScript expressions
858858

@@ -999,7 +999,7 @@ private final class ClassDefChecker(classDef: ClassDef,
999999
}
10001000

10011001
private def checkAllowTransients()(implicit ctx: ErrorContext): Unit = {
1002-
if (!postOptimizer)
1002+
if (!nextPhase.accept(IRFeature.Optimized))
10031003
reportError("invalid transient tree")
10041004
}
10051005

@@ -1038,13 +1038,13 @@ object ClassDefChecker {
10381038
*
10391039
* @return Count of IR checking errors (0 in case of success)
10401040
*/
1041-
def check(classDef: ClassDef, postBaseLinker: Boolean, postOptimizer: Boolean, logger: Logger): Int = {
1041+
def check(classDef: ClassDef, nextPhase: CheckingPhase, logger: Logger): Int = {
10421042
val reporter = new LoggerErrorReporter(logger)
1043-
new ClassDefChecker(classDef, postBaseLinker, postOptimizer, reporter).checkClassDef()
1043+
new ClassDefChecker(classDef, nextPhase, reporter).checkClassDef()
10441044
reporter.errorCount
10451045
}
10461046

1047-
def check(linkedClass: LinkedClass, postOptimizer: Boolean, logger: Logger): Int = {
1047+
def check(linkedClass: LinkedClass, nextPhase: CheckingPhase, logger: Logger): Int = {
10481048
// Rebuild a ClassDef out of the LinkedClass
10491049
import linkedClass._
10501050
implicit val pos = linkedClass.pos
@@ -1065,7 +1065,7 @@ object ClassDefChecker {
10651065
topLevelExportDefs = Nil
10661066
)(optimizerHints)
10671067

1068-
check(classDef, postBaseLinker = true, postOptimizer, logger)
1068+
check(classDef, nextPhase, logger)
10691069
}
10701070

10711071
private class Env(

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import org.scalajs.linker.checker.ErrorReporter._
2929

3030
/** Checker for the validity of the IR. */
3131
private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
32-
postDesugarer: Boolean, postOptimizer: Boolean) {
32+
nextPhase: CheckingPhase) {
3333

3434
import IRChecker._
3535
import reporter.reportError
@@ -239,7 +239,7 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
239239
case Assign(lhs, rhs) =>
240240
def checkNonStaticField(receiver: Tree, name: FieldName): Unit = {
241241
receiver match {
242-
case This() if (postOptimizer && env.inConstructorOf.isDefined) ||
242+
case This() if (nextPhase.accept(IRFeature.Optimized) && env.inConstructorOf.isDefined) ||
243243
env.inConstructorOf == Some(name.className) =>
244244
/* ctors can write immutable fields of the class they are constructing.
245245
* postOptimizer, due to ctor inlining, we may write immutable parent class fields as well.
@@ -575,7 +575,7 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
575575
typecheckAny(expr, env)
576576
checkIsAsInstanceTargetType(tpe)
577577

578-
case LinkTimeProperty(name) if !postDesugarer =>
578+
case LinkTimeProperty(name) if nextPhase.accept(IRFeature.NeedsDesugaring) =>
579579

580580
// JavaScript expressions
581581

@@ -717,15 +717,17 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
717717
typecheckExpect(value, env, ctpe)
718718
}
719719

720-
case Transient(transient) if postOptimizer =>
720+
case Transient(transient) if nextPhase.accept(IRFeature.Optimized) =>
721721
transient.traverse(new Traversers.Traverser {
722722
override def traverse(tree: Tree): Unit = typecheck(tree, env)
723723
})
724724

725-
case Transient(Desugarer.Transients.Desugar(body)) if !postDesugarer =>
725+
case Transient(Desugarer.Transients.Desugar(body))
726+
if nextPhase.accept(IRFeature.Linked) && nextPhase.accept(IRFeature.NeedsDesugaring) =>
726727
typecheckExpect(body, env, tree.tpe)
727728

728-
case RecordSelect(record, SimpleFieldIdent(fieldName)) if postOptimizer =>
729+
case RecordSelect(record, SimpleFieldIdent(fieldName))
730+
if nextPhase.accept(IRFeature.Optimized) =>
729731
record.tpe match {
730732
case NothingType => // ok
731733

@@ -745,7 +747,8 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter,
745747

746748
typecheck(record, env)
747749

748-
case RecordValue(RecordType(fields), elems) if postOptimizer =>
750+
case RecordValue(RecordType(fields), elems)
751+
if nextPhase.accept(IRFeature.Optimized) =>
749752
if (fields.size == elems.size) {
750753
for ((field, elem) <- fields.zip(elems))
751754
typecheckExpect(elem, env, field.tpe)
@@ -927,10 +930,9 @@ object IRChecker {
927930
*
928931
* @return Count of IR checking errors (0 in case of success)
929932
*/
930-
def check(unit: LinkingUnit, logger: Logger, postDesugarer: Boolean,
931-
postOptimizer: Boolean): Int = {
933+
def check(unit: LinkingUnit, logger: Logger, nextPhase: CheckingPhase): Int = {
932934
val reporter = new LoggerErrorReporter(logger)
933-
new IRChecker(unit, reporter, postDesugarer, postOptimizer).check()
935+
new IRChecker(unit, reporter, nextPhase).check()
934936
reporter.errorCount
935937
}
936938
}

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Scala.js (https://www.scala-js.org/)
3+
*
4+
* Copyright EPFL.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (https://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package org.scalajs.linker.checker
14+
15+
final class IRFeature(val displayName: String) {
16+
override def toString(): String = displayName
17+
}
18+
19+
object IRFeature {
20+
/** IR coming after an initial linking phase. */
21+
val Linked = new IRFeature("linked")
22+
23+
/** IR that must be desugared away. */
24+
val NeedsDesugaring = new IRFeature("not-desugared")
25+
26+
/** IR that is only the result of desugaring. */
27+
val Desugared = new IRFeature("desugared")
28+
29+
/** IR that is only the result of optimizations. */
30+
val Optimized = new IRFeature("optimized")
31+
}

0 commit comments

Comments
 (0)
0