10000 Run IRChecker after optimizer if RuntimeLongs aren't in use · scala-js/scala-js@d15567d · GitHub
[go: up one dir, main page]
Skip to content

Commit d15567d

Browse files
gzm0gzm0
committed
Run IRChecker after optimizer if RuntimeLongs aren't in use
1 parent fb3f7c0 commit d15567d

File tree

3 files changed

+125
-17
lines changed

3 files changed

+125
-17
lines changed

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

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,8 @@ import org.scalajs.linker.standard.LinkedClass
2828
import org.scalajs.linker.checker.ErrorReporter._
2929

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

3334
import IRChecker._
3435
import reporter.reportError
@@ -238,8 +239,14 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter) {
238239
case Assign(lhs, rhs) =>
239240
def checkNonStaticField(receiver: Tree, name: FieldName): Unit = {
240241
receiver match {
241-
case This() if env.inConstructorOf == Some(name.className) =>
242-
// ok
242+
case This() if (postOptimizer && env.inConstructorOf.isDefined) ||
243+
env.inConstructorOf == Some(name.className) =>
244+
/* ctors can write immutable fields of the class they are constructing.
245+
* postOptimizer, due to ctor inlining, we may write immutable parent class fields as well.
246+
* IR checking of the lhs makes sure this field is actually in the parent class chain
247+
* (otherwise `This` would be ill-typed).
248+
*/
249+
243250
case _ =>
244251
if (lookupClass(name.className).lookupField(name).exists(!_.flags.isMutable))
245252
reportError(i"Assignment to immutable field $name.")
@@ -725,6 +732,17 @@ private final class IRChecker(unit: LinkingUnit, reporter: ErrorReporter) {
725732
typecheckExpect(value, env, ctpe)
726733
}
727734

735+
case Transient(transient) if postOptimizer =>
736+
transient.traverse(new Traversers.Traverser {
737+
override def traverse(tree: Tree): Unit = typecheck(tree, env)
738+
})
739+
740+
case _: RecordSelect if postOptimizer =>
741+
// TODO
742+
743+
case _: RecordValue if postOptimizer =>
744+
// TODO
745+
728746
case _:RecordSelect | _:RecordValue | _:Transient | _:JSSuperConstructorCall =>
729747
reportError("invalid tree")
730748
}
@@ -893,9 +911,9 @@ object IRChecker {
893911
*
894912
* @return Count of IR checking errors (0 in case of success)
895913
*/
896-
def check(unit: LinkingUnit, logger: Logger): Int = {
914+
def check(unit: LinkingUnit, logger: Logger, postOptimizer: Boolean = false): Int = {
897915
val reporter = new LoggerErrorReporter(logger)
898-
new IRChecker(unit, reporter).check()
916+
new IRChecker(unit, reporter, postOptimizer).check()
899917
reporter.errorCount
900918
}
901919
}

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

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import org.scalajs.ir.Trees.ClassDef
2020

2121
import org.scalajs.logging._
2222

23+
import org.scalajs.linker.checker.IRChecker
2324
import org.scalajs.linker.interface.ModuleInitializer
2425
import org.scalajs.linker.standard._
2526
import org.scalajs.linker.standard.ModuleSet.ModuleID
@@ -33,6 +34,16 @@ final class Refiner(config: CommonPhaseConfig, checkIR: Boolean) {
3334
private val analyzer =
3435
new Analyzer(config, initial = false, checkIR = checkIR, failOnError = true, irLoader)
3536

37+
/* TODO: Remove this and replace with `checkIR` once the optimizer generates
38+
* well-typed IR with runtime longs.
39+
*/
40+
private val shouldRunIRChecker = {
41+
val optimizerUsesRuntimeLong =
42+
!config.coreSpec.esFeatures.allowBigIntsForLongs &&
43+
!config.coreSpec.targetIsWebAssembly
44+
checkIR && !optimizerUsesRuntimeLong
45+
}
46+
3647
def refine(classDefs: Seq[(ClassDef, Version)],
3748
moduleInitializers: List[ModuleInitializer],
3849
symbolRequirements: SymbolRequirement, logger: Logger)(
@@ -47,7 +58,7 @@ final class Refiner(config: CommonPhaseConfig, checkIR: Boolean) {
4758
val result = for {
4859
analysis <- analysis
4960
} yield {
50-
logger.time("Refiner: Assemble LinkedClasses") {
61+
val result = logger.time("Refiner: Assemble LinkedClasses") {
5162
val assembled = for {
5263
(classDef, version) <- classDefs
5364
if analysis.classInfos.contains(classDef.className)
@@ -65,6 +76,18 @@ final class Refiner(config: CommonPhaseConfig, checkIR: Boolean) {
6576
new LinkingUnit(config.coreSpec, linkedClassDefs.toList,
6677
linkedTopLevelExports.flatten.toList, moduleInitializers, globalInfo)
6778
}
79+
80+
if (shouldRunIRChecker) {
81+
logger.time("Refiner: Check IR") {
82+
val errorCount = IRChecker.check(result, logger, postOptimizer = true)
83+
if (errorCount != 0) {
84+
throw new AssertionError(
85+
s"There were $errorCount IR checking errors after optimization (this is a Scala.js bug)")
86+
}
87+
}
88+
}
89+
90+
result
6891
}
6992

7093
result.andThen { case _ =>

‎linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala

Lines changed: 78 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,9 @@ import org.scalajs.logging._
2828
import org.scalajs.junit.async._
2929

3030
import org.scalajs.linker.interface._
31+
import org.scalajs.linker.interface.unstable.IRFileImpl
3132
import org.scalajs.linker.standard._
33+
import org.scalajs.linker.frontend.Refiner
3234

3335
import org.scalajs.linker.testutils._
3436
import org.scalajs.linker.testutils.TestIRBuilder._
@@ -306,46 +308,111 @@ class IRCheckerTest {
306308
}
307309
}
308310

311+
def immutableFieldAssignTestClassDefs(parent: Boolean): Seq[ClassDef] = {
312+
val ctorBodyUnderTest =
313+
Assign(Select(thisFor("Bar"), FieldName("Foo", "fooFld"))(IntType), int(1))
314+
315+
Seq(
316+
classDef(
317+
"Foo",
318+
superClass = Some(ObjectClass),
319+
fields = List(FieldDef(EMF, FieldName("Foo", "fooFld"), NON, IntType))
320+
),
321+
classDef(
322+
"Bar",
323+
superClass = Some(if (parent) "Foo" else ObjectClass),
324+
methods = List(
325+
MethodDef(
326+
EMF.withNamespace(MemberNamespace.Constructor),
327+
NoArgConstructorName, NON, Nil, VoidType,
328+
Some(ctorBodyUnderTest))(EOH, UNV)
329+
)
330+
),
331+
mainTestClassDef(New("Bar", NoArgConstructorName, Nil))
332+
)
333+
}
334+
335+
@Test
336+
def noImmutableAssignNonParent(): AsyncResult = await {
337+
val classDefs = immutableFieldAssignTestClassDefs(parent = false)
338+
339+
for {
340+
log <- testLinkIRErrors(classDefs, MainTestModuleInitializers, postOptimizer = true)
341+
} yield {
342+
log.assertContainsError("Foo expected but Bar! found for tree of type org.scalajs.ir.Trees$This")
343+
}
344+
}
345+
346+
@Test
347+
def allowImmutableAssignParent(): AsyncResult = await {
348+
val classDefs = immutableFieldAssignTestClassDefs(parent = true)
349+
testLinkNoIRError(classDefs, MainTestModuleInitializers, postOptimizer = true)
350+
}
351+
309352
}
310353

311354
object IRCheckerTest {
312355
def testLinkNoIRError(classDefs: Seq[ClassDef],
313-
moduleInitializers: List[ModuleInitializer])(
356+
moduleInitializers: List[ModuleInitializer],
357+
postOptimizer: Boolean = false)(
314358
implicit ec: ExecutionContext): Future[Unit] = {
315-
link(classDefs, moduleInitializers, new ScalaConsoleLogger(Level.Error))
359+
link(classDefs, moduleInitializers, new ScalaConsoleLogger(Level.Error), postOptimizer)
316360
}
317361

318362
def testLinkIRErrors(classDefs: Seq[ClassDef],
319-
moduleInitializers: List[ModuleInitializer])(
363+
moduleInitializers: List[ModuleInitializer],
364+
postOptimizer: Boolean = false)(
320365
implicit ec: ExecutionContext): Future[LogLines] = {
321366

322367
val logger = new CapturingLogger
323368

324-
link(classDefs, moduleInitializers, logger).transform {
369+
link(classDefs, moduleInitializers, logger, postOptimizer).transform {
325370
case Success(_) => Failure(new AssertionError("IR checking did not fail"))
326371
case Failure(_) => Success(logger.allLogLines)
327372
}
328373
}
329374

330375
private def link(classDefs: Seq[ClassDef],
331376
moduleInitializers: List[ModuleInitializer],
332-
logger: Logger)(implicit ec: ExecutionContext): Future[Unit] = {
333-
val config = StandardConfig()
377+
logger: Logger, postOptimizer: Boolean)(
378+
implicit ec: ExecutionContext): Future[Unit] = {
379+
val baseConfig = StandardConfig()
334380
.withCheckIR(true)
335381
.withOptimizer(false)
336-
val linkerFrontend = StandardLinkerFrontend(config)
382+
383+
val config = {
384+
/* Disable RuntimeLongs to workaround the Refiner disabling IRChecks in this case.
385+
* TODO: Remove once we run IRChecks post optimizer all the time.
386+
*/
387+
if (postOptimizer) baseConfig.withESFeatures(_.withAllowBigIntsForLongs(true))
388+
else baseConfig
389+
}
337390

338391
val noSymbolRequirements = SymbolRequirement
339392
.factory("IRCheckerTest")
340393
.none()
341394

342395
TestIRRepo.minilib.flatMap { stdLibFiles =>
343-
val irFiles = (
396+
if (postOptimizer) {
397+
val refiner = new Refiner(CommonPhaseConfig.fromStandardConfig(config), checkIR = true)
398+
399+
Future.traverse(stdLibFiles)(f => IRFileImpl.fromIRFile(f).tree).flatMap { stdLibClassDefs =>
400+
val allClassDefs = (
401+
stdLibClassDefs ++
402+
classDefs
403+
)
404+
405+
refiner.refine(allClassDefs.map(c => (c, UNV)), moduleInitializers,
406+
noSymbolRequirements, logger)
407+
}
408+
} else {
409+
val linkerFrontend = StandardLinkerFrontend(config)
410+
val irFiles = (
344411
stdLibFiles ++
345412
classDefs.map(MemClassDefIRFile(_))
346-
)
347-
348-
linkerFrontend.link(irFiles, moduleInitializers, noSymbolRequirements, logger)
413+
)
414+
linkerFrontend.link(irFiles, moduleInitializers, noSymbolRequirements, logger)
415+
}
349416
}.map(_ => ())
350417
}
351418
}

0 commit comments

Comments
 (0)
0