Skip to content

Commit 576bcf1

Browse files
sjrdsjrd
committed
Check restrictions on how this values can be used in constructors.
Essentially, `this` cannot be used before the super constructor call, except to assign fields (but not reading). These restrictions match those enforced by the JVM for bytecode. While we never intended `this` values to be restricted in this way, in practice we never produced IR that would violate those restrictions. There was one exception before the parent commit: we could have lifted methods coming from the arguments to a super constructor call. Since they were lifted as instance methods, they caused the constructor to refer to `this` before the super constructor call. This is in theory a backward breaking change in the IR! However, we audited the entire ecosystem of libraries published on Maven Central. None of them contained IR that violates the new restriction. In the future, we may leverage those restrictions to validate strict field initialization. If a field *needs* to be initialized before it is ever read (for example, because it has a non-nullable reference type), then we can do so: we only have to check that it is definitely initialized before the super constructor call. The restrictions implemented in this commit ensure that nobody can read the field before the super constructor call starts.
1 parent 204acae commit 576bcf1

File tree

2 files changed

+113
-12
lines changed

2 files changed

+113
-12
lines changed

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

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -524,6 +524,13 @@ private final class ClassDefChecker(classDef: ClassDef,
524524
* delegate constructor call.
525525
* - There is no such `ApplyStatically` anywhere else in the body.
526526
* - `cls` must be the enclosing class or its direct superclass.
527+
* - In the statements before the delegate constructor call, and within
528+
* `args`:
529+
* - `This()` cannot be used except in `Assign(Select(This(), _), _)`,
530+
* i.e., to assign to a field (but not read from one).
531+
* - `StoreModule()` cannot be used.
532+
* - After the delegate constructor call, `This` can be used without
533+
* restriction.
527534
*
528535
* After the optimizer, there may be no delegate constructor call at all.
529536
* This frequently happens as the optimizer inlines super constructor
@@ -559,11 +566,14 @@ private final class ClassDefChecker(classDef: ClassDef,
559566
val (delegateCtorCall: ApplyStatically) :: afterDelegateCtor = rest
560567
val ApplyStatically(_, receiver, cls, MethodIdent(ctor), args) = delegateCtorCall
561568

562-
val envJustBeforeDelegate = checkBlockStats(beforeDelegateCtor, bodyEnv)
569+
val initEnv = bodyEnv.withIsThisRestricted(true)
570+
val envJustBeforeDelegate = checkBlockStats(beforeDelegateCtor, initEnv)
563571

564572
checkApplyArgs(ctor, args, envJustBeforeDelegate)
565573

566-
checkTree(receiver, envJustBeforeDelegate) // check that the This itself is valid
574+
val unrestrictedEnv = envJustBeforeDelegate.withIsThisRestricted(false)
575+
576+
checkTree(receiver, unrestrictedEnv) // check that the This itself is valid
567577

568578
if (!postOptimizer) {
569579
if (!(cls == classDef.className || classDef.superClass.exists(_.name == cls))) {
@@ -575,7 +585,7 @@ private final class ClassDefChecker(classDef: ClassDef,
575585
}
576586
}
577587

578-
checkBlockStats(afterDelegateCtor, envJustBeforeDelegate)
588+
checkBlockStats(afterDelegateCtor, unrestrictedEnv)
579589
}
580590
}
581591
}
@@ -632,7 +642,12 @@ private final class ClassDefChecker(classDef: ClassDef,
632642
checkTree(body, env.withLabel(label.name))
633643

634644
case Assign(lhs, rhs) =>
635-
checkTree(lhs, env)
645+
lhs match {
646+
case Select(This(), _) if env.isThisRestricted =>
647+
checkTree(lhs, env.withIsThisRestricted(false))
648+
case _ =>
649+
checkTree(lhs, env)
650+
}
636651
checkTree(rhs, env)
637652

638653
lhs match {
@@ -704,10 +719,12 @@ private final class ClassDefChecker(classDef: ClassDef,
704719
case StoreModule() =>
705720
if (!classDef.kind.hasModuleAccessor)
706721
reportError(i"Illegal StoreModule inside class of kind ${classDef.kind}")
707-
if (!env.inConstructor)
722+
else if (!env.inConstructor)
708723
reportError(i"Illegal StoreModule outside of constructor")
709-
if (env.thisType == NoType) // can happen before JSSuperConstructorCall in JSModuleClass
724+
else if (env.thisType == NoType) // can happen before JSSuperConstructorCall in JSModuleClass
710725
reportError(i"Cannot find `this` in scope for StoreModule()")
726+
else if (env.isThisRestricted)
727+
reportError(i"Restricted use of `this` for StoreModule() before super constructor call")
711728

712729
case Select(qualifier, _) =>
713730
checkTree(qualifier, env)
@@ -915,6 +932,8 @@ private final class ClassDefChecker(classDef: ClassDef,
915932
reportError(i"Cannot find `this` in scope")
916933
else if (tree.tpe != env.thisType)
917934
reportError(i"`this` of type ${env.thisType} typed as ${tree.tpe}")
935+
else if (env.isThisRestricted)
936+
reportError(i"Restricted use of `this` before the super constructor call")
918937

919938
case Closure(arrow, captureParams, params, restParam, body, captureValues) =>
920939
/* Check compliance of captureValues wrt. captureParams in the current
@@ -1045,7 +1064,9 @@ object ClassDefChecker {
10451064
/** Return types by label. */
10461065
val returnLabels: Set[LabelName],
10471066
/** Whether we are in a constructor of the class. */
1048-
val inConstructor: Boolean
1067+
val inConstructor: Boolean,
1068+
/** Whether usages of `this` are restricted in this scope. */
1069+
val isThisRestricted: Boolean
10491070
) {
10501071
import Env._
10511072

@@ -1064,20 +1085,33 @@ object ClassDefChecker {
10641085
def withInConstructor(inConstructor: Boolean): Env =
10651086
copy(inConstructor = inConstructor)
10661087

1088+
def withIsThisRestricted(isThisRestricted: Boolean): Env =
1089+
copy(isThisRestricted = isThisRestricted)
1090+
10671091
private def copy(
10681092
hasNewTarget: Boolean = hasNewTarget,
10691093
thisType: Type = thisType,
10701094
locals: Map[LocalName, LocalDef] = locals,
10711095
returnLabels: Set[LabelName] = returnLabels,
1072-
inConstructor: Boolean = inConstructor
1096+
inConstructor: Boolean = inConstructor,
1097+
isThisRestricted: Boolean = isThisRestricted
10731098
): Env = {
1074-
new Env(hasNewTarget, thisType, locals, returnLabels, inConstructor)
1099+
new Env(hasNewTarget, thisType, locals, returnLabels, inConstructor,
1100+
isThisRestricted)
10751101
}
10761102
}
10771103

10781104
private object Env {
1079-
val empty: Env =
1080-
new Env(hasNewTarget = false, thisType = NoType, Map.empty, Set.empty, inConstructor = false)
1105+
val empty: Env = {
1106+
new Env(
1107+
hasNewTarget = false,
1108+
thisType = NoType,
1109+
locals = Map.empty,
1110+
returnLabels = Set.empty,
1111+
inConstructor = false,
1112+
isThisRestricted = false
1113+
)
1114+
}
10811115

10821116
def fromParams(params: List[ParamDef]): Env = {
10831117
val paramLocalDefs =
@@ -1089,7 +1123,8 @@ object ClassDefChecker {
10891123
thisType = NoType,
10901124
paramLocalDefs.toMap,
10911125
Set.empty,
1092-
inConstructor = false
1126+
inConstructor = false,
1127+
isThisRestricted = false
10931128
)
10941129
}
10951130
}

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

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -447,6 +447,55 @@ class ClassDefCheckerTest {
447447
"`this` of type any typed as Foo!")
448448
}
449449

450+
@Test
451+
def restrictedThis(): Unit = {
452+
val xParamDef = paramDef("x", IntType)
453+
val xFieldName = FieldName("Foo", "x")
454+
455+
def testRestrictedThisError(ctorStats: Tree*): Unit = {
456+
val ctorFlags = EMF.withNamespace(MemberNamespace.Constructor)
457+
458+
assertError(
459+
classDef(
460+
"Foo", superClass = Some(ObjectClass),
461+
methods = List(
462+
MethodDef(ctorFlags, MethodName.constructor(List(I)), NON,
463+
List(xParamDef), NoType, Some(Block(ctorStats: _*)))(EOH, UNV)
464+
)
465+
),
466+
"Restricted use of `this` before the super constructor call")
467+
}
468+
469+
val superCtorCall = trivialSuperCtorCall("Foo")
470+
val thiz = thisFor("Foo")
471+
472+
testRestrictedThisError(
473+
thiz,
474+
superCtorCall
475+
)
476+
477+
testRestrictedThisError(
478+
Select(thiz, xFieldName)(IntType),
479+
superCtorCall
480+
)
481+
482+
testRestrictedThisError(
483+
Assign(Select(Select(thiz, xFieldName)(IntType), xFieldName)(IntType), int(5)),
484+
superCtorCall
485+
)
486+
487+
testRestrictedThisError(
488+
Assign(Select(thiz, xFieldName)(IntType), Select(thiz, xFieldName)(IntType)),
489+
superCtorCall
490+
)
491+
492+
testRestrictedThisError(
493+
ApplyStatically(EAF.withConstructor(true), thiz, ObjectClass,
494+
MethodIdent(MethodName.constructor(List(O))),
495+
List(thiz))(NoType)
496+
)
497+
}
498+
450499
@Test
451500
def storeModule(): Unit = {
452501
val ctorFlags = EMF.withNamespace(MemberNamespace.Constructor)
@@ -470,6 +519,23 @@ class ClassDefCheckerTest {
470519
"Illegal StoreModule inside class of kind Class"
471520
)
472521

522+
assertError(
523+
classDef(
524+
"Foo",
525+
kind = ClassKind.ModuleClass,
526+
superClass = Some(ObjectClass),
527+
methods = List(
528+
MethodDef(ctorFlags, NoArgConstructorName, NON, Nil, NoType, Some {
529+
Block(
530+
StoreModule(),
531+
superCtorCall
532+
)
533+
})(EOH, UNV)
534+
)
535+
),
536+
"Restricted use of `this` for StoreModule() before super constructor call"
537+
)
538+
473539
assertError(
474540
classDef(
475541
"Foo",

0 commit comments

Comments
 (0)