-
Notifications
You must be signed in to change notification settings - Fork 396
Restrict usages of StoreModule even further. #5059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Restrict usages of StoreModule even further. #5059
Conversation
linker/shared/src/main/scala/org/scalajs/linker/checker/ClassDefChecker.scala
Show resolved
Hide resolved
@@ -331,9 +331,7 @@ private final class ClassDefChecker(classDef: ClassDef, | |||
val envJustBeforeSuper = checkBlockStats(body.beforeSuper, startEnv) | |||
checkTreeOrSpreads(body.superCall.args, envJustBeforeSuper) | |||
val envJustAfterSuper = envJustBeforeSuper.withThisType(instanceThisType) | |||
val afterSuperStoreModulesHandled = | |||
handleStoreModulesAfterSuperCtorCall(body.afterSuper) | |||
checkBlockStats(afterSuperStoreModulesHandled, envJustAfterSuper) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I commented on #5057, I don't think we should do this (but start emitting store module for JS modules as well).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a test that tests a StoreModule
at the top level after the the super ctor call (but not immediately after) is not allowed.
837a819
to
b483cb6
Compare
85a8eb5
to
68bfc6f
Compare
* 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.
And only enable the deserialization when reading IR < 1.18.
85a8eb5
to
ff197d8
Compare
Fixed scalastyle 🤦♂️ |
And port the changes that were made to the compiler backend. Most nobably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * Introduce non-nullable reference types in the IR. scala-js/scala-js#5018 * Opt: Remove useless *Ident indirections in the IR model. scala-js/scala-js#5092 * Merge the IR node This into VarRef, with a magic LocalName. scala-js/scala-js#5090 * Merge several IR nodes into UnaryOp. scala-js/scala-js#5088 * Restrict usages of StoreModule even further. scala-js/scala-js#5059 * Allow Return arg to be a void if the target Labeled is a void. scala-js/scala-js#5074 * Rename NoType to VoidType and print it as "void". scala-js/scala-js#5061
And port the changes that were made to the compiler backend. Most notably, the changes from the following upstream PRs: * scala-js/scala-js#5018 * scala-js/scala-js#5092 * scala-js/scala-js#5090 * scala-js/scala-js#5088 * scala-js/scala-js#5059 * scala-js/scala-js#5074 * scala-js/scala-js#5061
Alternative to #5057. The first commit is the same as the one in #5057.