8000 Merge pull request #5120 from sjrd/refactor-checking-phase · scala-js/scala-js@f014e0e · GitHub
[go: up one dir, main page]
Skip to content

Commit f014e0e

Browse files
sjrdsjrd
authored
Merge pull request #5120 from sjrd/refactor-checking-phase
Refactoring: Generic notion of CheckingPhase for the IR checkers.
2 parents fb051e5 + 2a77c9c commit f014e0e

File tree

11 files changed

+173
-74
lines changed

11 files changed

+173
-74
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 (previousPhase <- checkIRFor) {
84+
val errorCount = ClassDefChecker.check(tree, previousPhase, logger)
85+
if (errorCount != 0) {
86+
if (previousPhase == CheckingPhase.Compiler) {
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: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
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 *after which* we are checking IR.
16+
*
17+
* When checking IR (with `ClassDefChecker` or `IRChecker`), different nodes
18+
* and transients are allowed between different phases. The `CheckingPhase`
19+
* records the *previous* phase to run before the check. We are therefore
20+
* checking that the IR is a valid *output* of the target phase.
21+
*/
22+
sealed abstract class CheckingPhase
23+
24+
object CheckingPhase {
25+
case object Compiler extends CheckingPhase
26+
case object BaseLinker extends CheckingPhase
27+
case object Optimizer extends CheckingPhase
28+
}

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

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,11 +28,13 @@ 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+
previousPhase: CheckingPhase, reporter: ErrorReporter) {
3232
import ClassDefChecker._
3333

3434
import reporter.reportError
3535

36+
private val featureSet = FeatureSet.allowedAfter(previousPhase)
37+
3638
private[this] val isJLObject = classDef.name.name == ObjectClass
3739

3840
private[this] val instanceThisType: Type = {
@@ -107,7 +109,7 @@ private final class ClassDefChecker(classDef: ClassDef,
107109
* module classes and JS classes that are never instantiated. The classes
108110
* may still exist because their class data are accessed.
109111
*/
110-
if (!postBaseLinker) {
112+
if (!featureSet.supports(FeatureSet.OptionalConstructors)) {
111113
/* Check that we have exactly 1 constructor in a module class. This goes
112114
* together with `checkMethodDef`, which checks that a constructor in a
113115
* module class must be 0-arg.
@@ -489,7 +491,7 @@ private final class ClassDefChecker(classDef: ClassDef,
489491
private def checkMethodNameNamespace(name: MethodName, namespace: MemberNamespace)(
490492
implicit ctx: ErrorContext): Unit = {
491493
if (name.isReflectiveProxy) {
492-
if (postBaseLinker) {
494+
if (featureSet.supports(FeatureSet.ReflectiveProxies)) {
493495
if (namespace != MemberNamespace.Public)
494496
reportError("reflective profixes are only allowed in the public namespace")
495497
} else {
@@ -557,7 +559,7 @@ private final class ClassDefChecker(classDef: ClassDef,
557559
}
558560

559561
if (rest.isEmpty) {
560-
if (!postOptimizer)
562+
if (!featureSet.supports(FeatureSet.RelaxedCtorBodies))
561563
reportError(i"Constructor must contain a delegate constructor call")
562564

563565
val bodyStatsStoreModulesHandled =
@@ -576,7 +578,7 @@ private final class ClassDefChecker(classDef: ClassDef,
576578

577579
checkTree(receiver, unrestrictedEnv) // check that the This itself is valid
578580

579-
if (!postOptimizer) {
581+
if (!featureSet.supports(FeatureSet.RelaxedCtorBodies)) {
580582
if (!(cls == classDef.className || classDef.superClass.exists(_.name == cls))) {
581583
implicit val ctx = ErrorContext(delegateCtorCall)
582584
reportError(
@@ -597,7 +599,7 @@ private final class ClassDefChecker(classDef: ClassDef,
597599
implicit ctx: ErrorContext): List[Tree] = {
598600

599601
if (classDef.kind.hasModuleAccessor) {
600-
if (postOptimizer) {
602+
if (featureSet.supports(FeatureSet.RelaxedCtorBodies)) {
601603
/* If the super constructor call was inlined, the StoreModule can be anywhere.
602604
* Moreover, the optimizer can remove StoreModules altogether in many cases.
603605
*/
@@ -673,7 +675,7 @@ private final class ClassDefChecker(classDef: ClassDef,
673675
case Assign(lhs, rhs) =>
674676
lhs match {
675677
case Select(This(), field) if env.isThisRestricted =>
676-
if (postOptimizer || field.name.className == classDef.className)
678+
if (featureSet.supports(FeatureSet.RelaxedCtorBodies) || field.name.className == classDef.className)
677679
checkTree(lhs, env.withIsThisRestricted(false))
678680
else
679681
checkTree(lhs, env)
@@ -819,11 +821,13 @@ private final class ClassDefChecker(classDef: ClassDef,
819821
checkTree(index, env)
820822

821823
case RecordSelect(record, _) =>
822-
checkAllowTransients()
824+
if (!featureSet.supports(FeatureSet.Records))
825+
reportError("invalid use of records")
823826
checkTree(record, env)
824827

825828
case RecordValue(_, elems) =>
826-
checkAllowTransients()
829+
if (!featureSet.supports(FeatureSet.Records))
830+
reportError("invalid use of records")
827831
checkTrees(elems, env)
828832

829833
case IsInstanceOf(expr, testType) =>
@@ -987,7 +991,9 @@ private final class ClassDefChecker(classDef: ClassDef,
987991
checkTrees(captureValues, env)
988992

989993
case Transient(transient) =>
990-
checkAllowTransients()
994+
if (!featureSet.supports(FeatureSet.OptimizedTransients))
995+
reportError(i"invalid transient tree of class ${transient.getClass().getName()}")
996+
991997
transient.traverse(new Traversers.Traverser {
992998
override def traverse(tree: Tree): Unit = checkTree(tree, env)
993999
})
@@ -996,11 +1002,6 @@ private final class ClassDefChecker(classDef: ClassDef,
9961002
newEnv
9971003
}
9981004

999-
private def checkAllowTransients()(implicit ctx: ErrorContext): Unit = {
1000-
if (!postOptimizer)
1001-
reportError("invalid transient tree")
1002-
}
1003-
10041005
private def checkArrayType(tpe: ArrayType)(
10051006
implicit ctx: ErrorContext): Unit = {
10061007
checkArrayTypeRef(tpe.arrayTypeRef)
@@ -1036,13 +1037,13 @@ object ClassDefChecker {
10361037
*
10371038
* @return Count of IR checking errors (0 in case of success)
10381039
*/
1039-
def check(classDef: ClassDef, postBaseLinker: Boolean, postOptimizer: Boolean, logger: Logger): Int = {
1040+
def check(classDef: ClassDef, previousPhase: CheckingPhase, logger: Logger): Int = {
10401041
val reporter = new LoggerErrorReporter(logger)
1041-
new ClassDefChecker(classDef, postBaseLinker, postOptimizer, reporter).checkClassDef()
1042+
new ClassDefChecker(classDef, previousPhase, reporter).checkClassDef()
10421043
reporter.errorCount
10431044
}
10441045

1045-
def check(linkedClass: LinkedClass, postOptimizer: Boolean, logger: Logger): Int = {
1046+
def check(linkedClass: LinkedClass, previousPhase: CheckingPhase, logger: Logger): Int = {
10461047
// Rebuild a ClassDef out of the LinkedClass
10471048
import linkedClass._
10481049
implicit val pos = linkedClass.pos
@@ -1063,7 +1064,7 @@ object ClassDefChecker {
10631064
topLevelExportDefs = Nil
10641065
)(optimizerHints)
10651066

1066-
check(classDef, postBaseLinker = true, postOptimizer, logger)
1067+
check(classDef, previousPhase, logger)
10671068
}
10681069

10691070
private class Env(

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
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+
import org.scalajs.linker.checker.CheckingPhase._
16+
17+
/** A set of conditional IR features that the checkers can accept.
18+
*
19+
* At any given phase, the `ClassDefChecker` and the `IRChecker` must agree on
20+
* the set of IR features that are valid. A `FeatureSet` factors out the
21+
* knowledge of what feature is acceptable when.
22+
*/
23+
private[checker] final class FeatureSet private (private val flags: Int) extends AnyVal {
24+
/** Does this feature set support (all of) the given feature set. */
25+
def supports(features: FeatureSet): Boolean =
26+
(features.flags & flags) == features.flags
27+
28+
/** The union of this feature set and `that` feature set. */
29+
def |(that: FeatureSet): FeatureSet =
30+
new FeatureSet(this.flags | that.flags)
31+
}
32+
33+
private[checker] object FeatureSet {
34+
/** Empty feature set. */
35+
val Empty = new FeatureSet(0)
36+
37+
// Individual features
38+
39+
/** Optional constructors in module classes and JS classes. */
40+
val OptionalConstructors = new FeatureSet(1 << 0)
41+
42+
/** Explicit reflective proxy definitions. */
43+
val ReflectiveProxies = new FeatureSet(1 << 1)
44+
45+
/** Transients that are the result of optimizations. */
46+
val OptimizedTransients = new FeatureSet(1 << 2)
47+
48+
/** Records and record types. */
49+
val Records = new FeatureSet(1 << 3)
50+
51+
/** Relaxed constructor discipline.
52+
*
53+
* - Optional super/delegate constructor call.
54+
* - Delegate constructor calls can target any super class.
55+
* - `this.x = ...` assignments before the delegate call can assign super class fields.
56+
* - `StoreModule` can be anywhere, or not be there at all.
57+
*/
58+
val RelaxedCtorBodies = new FeatureSet(1 << 4)
59+
60+
// Common sets
61+
62+
/** Features introduced by the base linker. */
63+
private val Linked =
64+
OptionalConstructors | ReflectiveProxies
65+
66+
/** IR that is only the result of optimizations. */
67+
private val Optimized =
68+
OptimizedTransients | Records | RelaxedCtorBodies
69+
70+
/** The set of features allowed as output of the given phase. */
71+
def allowedAfter(phase: CheckingPhase): FeatureSet = phase match {
72+
case Compiler => Empty
73+
case BaseLinker => Linked
74+
case Optimizer => Linked | Optimized
75+
}
76+
}

0 commit comments

Comments
 (0)
0