Only allow StoreModule at the top-level of constructor bodies. #5057
Conversation
|
Hum actually we never emit Should we? Maybe not, and we should forbid them there as well. As a reminder, the purpose of |
This allows to get rid of `Env.inConstructor` in the `ClassDefChecker`. In theory, this is a breaking change. However, we never emitted IR with `StoreModule`s that would violate the new rule. In fact, all the IR we emit is even stricter: there is always a `StoreModule` *immediately after* the super constructor call of (JS) module class constructors, and never anywhere else.
5678e9a to
7840ce1
Compare
IMO that violates the principle of least surprise. The object part of JS objects, is really a Scala semantic, so I would not diverge unless we have to.
Would it make sense to make |
OK. In that case I propose the following sequence:
Does that make sense? |
|
Yes, that plan SGTM. |
This allows to get rid of
Env.inConstructorin theClassDefChecker.In theory, this is a breaking change. However, we never emitted IR with
StoreModules that would violate the new rule.In fact, all the IR we emit is even stricter: there is always a
StoreModuleimmediately after the super constructor call of (JS) module class constructors, and never anywhere else.Followup to #5020 (review)
@gzm0 Should we check the even stricter version?