Skip to content

Commit b483cb6

Browse files
sjrdsjrd
committed
Restrict usages of StoreModule even further.
* Must appear exactly once, right after the super constructor call, in module classes. * Cannot appear anywhere else. This was not actually the case for JS module classes. We never emitted `StoreModule`s in JS constructors. We use a deserialization hack to introduce them, and add tests for the corner case that `StoreModule` is meant to solve. In this commit, we always enable the deserialization hack, and do not change the compiler yet.
1 parent f3e2ae7 commit b483cb6

File tree

9 files changed

+229
-44
lines changed

9 files changed

+229
-44
lines changed

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1560,7 +1560,7 @@ object Serializers {
15601560
case TagFieldDef => fieldsBuilder += readFieldDef()
15611561
case TagJSFieldDef => fieldsBuilder += readJSFieldDef()
15621562
case TagMethodDef => methodsBuilder += readMethodDef(cls, kind)
1563-
case TagJSConstructorDef => jsConstructorBuilder += readJSConstructorDef()
1563+
case TagJSConstructorDef => jsConstructorBuilder += readJSConstructorDef(kind)
15641564
case TagJSMethodDef => jsMethodPropsBuilder += readJSMethodDef()
15651565
case TagJSPropertyDef => jsMethodPropsBuilder += readJSPropertyDef()
15661566
case TagJSNativeMemberDef => jsNativeMembersBuilder += readJSNativeMemberDef()
@@ -1986,7 +1986,9 @@ object Serializers {
19861986
}
19871987
}
19881988

1989-
private def readJSConstructorDef()(implicit pos: Position): JSConstructorDef = {
1989+
private def readJSConstructorDef(ownerKind: ClassKind)(
1990+
implicit pos: Position): JSConstructorDef = {
1991+
19901992
val optHash = readOptHash()
19911993
// read and discard the length
19921994
val len = readInt()
@@ -2001,7 +2003,17 @@ object Serializers {
20012003
val bodyPos = readPosition()
20022004
val beforeSuper = readTrees()
20032005
val superCall = readTree().asInstanceOf[JSSuperConstructorCall]
2004-
val afterSuper = readTrees()
2006+
val afterSuper0 = readTrees()
2007+
2008+
val afterSuper = if (true /*hacks.use17*/ && ownerKind == ClassKind.JSModuleClass) {
2009+
afterSuper0 match {
2010+
case StoreModule() :: _ => afterSuper0
2011+
case _ => StoreModule()(superCall.pos) :: afterSuper0
2012+
}
2013+
} else {
2014+
afterSuper0
2015+
}
2016+
20052017
val body = JSConstructorBody(beforeSuper, superCall, afterSuper)(bodyPos)
20062018
JSConstructorDef(flags, params, restParam, body)(
20072019
OptimizerHints.fromBits(readInt()), optHash)

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

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -592,14 +592,30 @@ private final class ClassDefChecker(classDef: ClassDef,
592592
}
593593
}
594594

595-
private def handleStoreModulesAfterSuperCtorCall(trees: List[Tree]): List[Tree] = {
596-
/* If StoreModule's are allowed, there is nothing left to check about them,
597-
* so we filter them out.
598-
*/
599-
if (classDef.kind.hasModuleAccessor)
600-
trees.filter(!_.isInstanceOf[StoreModule])
601-
else
595+
private def handleStoreModulesAfterSuperCtorCall(trees: List[Tree])(
596+
implicit ctx: ErrorContext): List[Tree] = {
597+
598+
if (classDef.kind.hasModuleAccessor) {
599+
if (postOptimizer) {
600+
/* If the super constructor call was inlined, the StoreModule can be anywhere.
601+
* Moreover, the optimizer can remove StoreModules altogether in many cases.
602+
*/
603+
trees.filter(!_.isInstanceOf[StoreModule])
604+
} else {
605+
/* Before the optimizer, there must be a StoreModule and it must come
606+
* right after the super constructor call.
607+
*/
608+
trees match {
609+
case StoreModule() :: rest =>
610+
rest
611+
case _ =>
612+
reportError(i"Missing StoreModule right after the super constructor call")
613+
trees
614+
}
615+
}
616+
} else {
602617
trees
618+
}
603619
}
604620

605621
private def checkBlockStats(stats: List[Tree], env: Env): Env = {

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ class AnalyzerTest {
467467
"A",
468468
kind = ClassKind.ModuleClass,
469469
superClass = Some(ObjectClass),
470-
methods = List(trivialCtor("A")),
470+
methods = List(trivialCtor("A", forModuleClass = true)),
471471
topLevelExportDefs = List(
472472
TopLevelMethodExportDef("main", JSMethodDef(
473473
EMF.withNamespace(MemberNamespace.PublicStatic),
@@ -492,7 +492,7 @@ class AnalyzerTest {
492492
def singleDef(name: String) = {
493493
classDef(name,
494494
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
495-
methods = List(trivialCtor(name)),
495+
methods = List(trivialCtor(name, forModuleClass = true)),
496496
topLevelExportDefs = List(TopLevelModuleExportDef(name, "foo")))
497497
}
498498

@@ -515,7 +515,7 @@ class AnalyzerTest {
515515
def singleDef(name: String) = {
516516
classDef(name,
517517
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
518-
methods = List(trivialCtor(name)),
518+
methods = List(trivialCtor(name, forModuleClass = true)),
519519
topLevelExportDefs = List(TopLevelModuleExportDef("main", "foo")))
520520
}
521521

@@ -536,7 +536,7 @@ class AnalyzerTest {
536536
def degenerateConflictingTopLevelExports(): AsyncResult = await {
537537
val classDefs = Seq(classDef("A",
538538
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
539-
methods = List(trivialCtor("A")),
539+
methods = List(trivialCtor("A", forModuleClass = true)),
540540
topLevelExportDefs = List(
541541
TopLevelModuleExportDef("main", "foo"),
542542
TopLevelModuleExportDef("main", "foo"))))
@@ -552,7 +552,7 @@ class AnalyzerTest {
552552
val classDefs = Seq(classDef("A",
553553
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
554554
methods = List(
555-
trivialCtor("A"),
555+
trivialCtor("A", forModuleClass = true),
556556
mainMethodDef(Skip())
557557
),
558558
topLevelExportDefs = List(TopLevelModuleExportDef("A", "foo"))))
@@ -630,9 +630,8 @@ class AnalyzerTest {
630630

631631
val classDefs = Seq(
632632
classDef("A",
633-
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
633+
kind = ClassKind.Class, superClass = Some(ObjectClass),
634634
methods = List(
635-
trivialCtor("A"),
636635
mainMethodDef(ApplyDynamicImport(EAF, "B", dynName, Nil)))
637636
),
638637
classDef("B",
@@ -688,9 +687,8 @@ class AnalyzerTest {
688687
def importMetaWithoutESModule(): AsyncResult = await {
689688
val classDefs = Seq(
690689
classDef("A",
691-
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
690+
kind = ClassKind.Class, superClass = Some(ObjectClass),
692691
methods = List(
693-
trivialCtor("A"),
694692
mainMethodDef(JSImportMeta())
695693
)
696694
)
@@ -812,9 +810,8 @@ class AnalyzerTest {
812810
def test(invalidLinkTimeProperty: LinkTimeProperty): Future[Unit] = {
813811
val classDefs = Seq(
814812
classDef("A",
815-
kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
813+
kind = ClassKind.Class, superClass = Some(ObjectClass),
816814
methods = List(
817-
trivialCtor("A"),
818815
mainMethodDef(invalidLinkTimeProperty)
819816
)
820817
)

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ class IRCheckerTest {
176176
classDef("B", kind = ClassKind.NativeJSClass, superClass = Some(ObjectClass)),
177177
classDef("C", kind = ClassKind.NativeJSModuleClass, superClass = Some(ObjectClass)),
178178

179-
classDef("D", kind = ClassKind.JSClass, superClass = Some("A"), jsConstructor = Some(trivialJSCtor)),
179+
classDef("D", kind = ClassKind.JSClass, superClass = Some("A"), jsConstructor = Some(trivialJSCtor())),
180180

181181
mainTestClassDef(Block(
182182
LoadJSConstructor("B"),

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

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ class IncrementalTest {
5757
kind = ClassKind.ModuleClass,
5858
superClass = Some(ObjectClass),
5959
methods = List(
60-
trivialCtor(FooClass),
60+
trivialCtor(FooClass, forModuleClass = true),
6161
MethodDef(EMF.withNamespace(MemberNamespace.PublicStatic),
6262
staticMethodName, NON, Nil, IntType, Some(int(6)))(EOH, UNV)
6363
),
@@ -238,7 +238,7 @@ class IncrementalTest {
238238

239239
List(
240240
v -> classDef(FooClass, kind = ClassKind.ModuleClass, superClass = Some(ObjectClass),
241-
methods = trivialCtor(FooClass) :: stepDependentMembers),
241+
methods = trivialCtor(FooClass, forModuleClass = true) :: stepDependentMembers),
242242

243243
v -> mainTestClassDef(Block(stepDependentMainStats))
244244
)
@@ -313,12 +313,13 @@ class IncrementalTest {
313313
val FooModule = ClassName("Foo")
314314

315315
def fooCtor(pre: Boolean) = {
316-
val superCtor = {
316+
val superCtor = Block(
317317
ApplyStatically(EAF.withConstructor(true),
318318
thisFor(FooModule),
319319
ObjectClass, MethodIdent(NoArgConstructorName),
320-
Nil)(NoType)
321-
}
320+
Nil)(NoType),
321+
StoreModule()
322+
)
322323

323324
val body =
324325
if (pre) superCtor
@@ -362,7 +363,7 @@ class IncrementalTest {
362363
kind = ClassKind.ModuleClass,
363364
superClass = Some(ObjectClass),
364365
methods = List(
365-
trivialCtor(AModule)
366+
trivialCtor(AModule, forModuleClass = true)
366367
),
367368
jsMethodProps = List(
368369
JSMethodDef(EMF, str("foo"), Nil, None,
@@ -374,7 +375,7 @@ class IncrementalTest {
374375
kind = ClassKind.ModuleClass,
375376
superClass = Some(ObjectClass),
376377
methods = List(
377-
trivialCtor(BModule),
378+
trivialCtor(BModule, forModuleClass = true),
378379
MethodDef(EMF, targetMethodName, NON, Nil, IntType,
379380
Some(int(if (pre) 1 else 2)))(EOH.withInline(true), UNV)
380381
)
@@ -419,7 +420,7 @@ class IncrementalTest {
419420
kind = ClassKind.ModuleClass,
420421
superClass = Some(ObjectClass),
421422
methods = List(
422-
trivialCtor(BModule),
423+
trivialCtor(BModule, forModuleClass = true),
423424
MethodDef(EMF, targetMethodName, NON, Nil, IntType,
424425
Some(int(if (pre) 1 else 2)))(EOH.withInline(true), UNV)
425426
)
@@ -451,7 +452,7 @@ class IncrementalTest {
451452
kind = ClassKind.ModuleClass,
452453
superClass = Some(ObjectClass),
453454
methods = List(
454-
trivialCtor(BModule),
455+
trivialCtor(BModule, forModuleClass = true),
455456
MethodDef(EMF, targetMethodName, NON, Nil, IntType,
456457
Some(int(if (pre) 1 else 2)))(EOH.withInline(true), UNV)
457458
)

‎linker/shared/src/test/scala/org/scalajs/linker/checker/ClassDefCheckerTest.scala

Lines changed: 74 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ class ClassDefCheckerTest {
145145
fields = List(
146146
FieldDef(EMF.withNamespace(MemberNamespace.PublicStatic), FieldName("A", "foo"), NON, IntType)
147147
),
148-
methods = List(trivialCtor("A")),
148+
methods = List(trivialCtor("A", forModuleClass = true)),
149149
topLevelExportDefs = List(
150150
TopLevelFieldExportDef("main", "foo", FieldName("B", "foo"))
151151
)
@@ -533,14 +533,48 @@ class ClassDefCheckerTest {
533533
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
534534
Block(
535535
StoreModule(),
536-
superCtorCall
536+
superCtorCall,
537+
StoreModule()
537538
)
538539
})(EOH, UNV)
539540
)
540541
),
541542
"Illegal StoreModule"
542543
)
543544

545+
assertError(
546+
classDef(
547+
"Foo",
548+
kind = ClassKind.ModuleClass,
549+
superClass = Some(ObjectClass),
550+
methods = List(
551+
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
552+
Block(
553+
superCtorCall
554+
)
555+
})(EOH, UNV)
556+
)
557+
),
558+
"Missing StoreModule right after the super constructor call"
559+
)
560+
561+
assertError(
562+
classDef(
563+
"Foo",
564+
kind = ClassKind.ModuleClass,
565+
superClass = Some(ObjectClass),
566+
methods = List(
567+
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
568+
Block(
569+
superCtorCall,
570+
IntLiteral(1)
571+
)
572+
})(EOH, UNV)
573+
)
574+
),
575+
"Missing StoreModule right after the super constructor call"
576+
)
577+
544578
assertError(
545579
classDef(
546580
"Foo",
@@ -550,6 +584,7 @@ class ClassDefCheckerTest {
550584
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
551585
Block(
552586
superCtorCall,
587+
StoreModule(),
553588
If(BooleanLiteral(true), StoreModule(), Skip())(NoType)
554589
)
555590
})(EOH, UNV)
@@ -564,7 +599,25 @@ class ClassDefCheckerTest {
564599
kind = ClassKind.ModuleClass,
565600
superClass = Some(ObjectClass),
566601
methods = List(
567-
trivialCtor("Foo"),
602+
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
603+
Block(
604+
superCtorCall,
605+
StoreModule(),
606+
StoreModule()
607+
)
608+
})(EOH, UNV)
609+
)
610+
),
611+
"Illegal StoreModule"
612+
)
613+
614+
assertError(
615+
classDef(
616+
"Foo",
617+
kind = ClassKind.ModuleClass,
618+
superClass = Some(ObjectClass),
619+
methods = List(
620+
trivialCtor("Foo", forModuleClass = true),
568621
MethodDef(EMF, MethodName("foo", Nil, VoidRef), NON, Nil, NoType, Some {
569622
Block(
570623
StoreModule()
@@ -582,7 +635,8 @@ class ClassDefCheckerTest {
582635
superClass = Some("scala.scalajs.js.Object"),
583636
jsConstructor = Some(
584637
JSConstructorDef(JSCtorFlags, Nil, None,
585-
JSConstructorBody(StoreModule() :: Nil, JSSuperConstructorCall(Nil), Undefined() :: Nil))(
638+
JSConstructorBody(StoreModule() :: Nil, JSSuperConstructorCall(Nil),
639+
StoreModule() :: Undefined() :: Nil))(
586640
EOH, UNV)
587641
)
588642
),
@@ -597,7 +651,22 @@ class ClassDefCheckerTest {
597651
jsConstructor = Some(
598652
JSConstructorDef(JSCtorFlags, Nil, None,
599653
JSConstructorBody(Nil, JSSuperConstructorCall(Nil),
600-
If(BooleanLiteral(true), StoreModule(), Skip())(NoType) :: Undefined() :: Nil))(
654+
Undefined() :: Nil))(
655+
EOH, UNV)
656+
)
657+
),
658+
"Missing StoreModule right after the super constructor call"
659+
)
660+
661+
assertError(
662+
classDef(
663+
"Foo",
664+
kind = ClassKind.JSModuleClass,
665+
superClass = Some("scala.scalajs.js.Object"),
666+
jsConstructor = Some(
667+
JSConstructorDef(JSCtorFlags, Nil, None,
668+
JSConstructorBody(Nil, JSSuperConstructorCall(Nil),
669+
StoreModule() :: StoreModule() :: Undefined() :: Nil))(
601670
EOH, UNV)
602671
)
603672
),

0 commit comments

Comments
 (0)