wurstscript / WurstScript

Programming language and toolkit to create Warcraft III Maps
https://wurstlang.org
Apache License 2.0
226 stars 28 forks source link

Wrong initialization order with configuration packages #990

Closed Jampi0n closed 3 years ago

Jampi0n commented 3 years ago

Packages are initialized in the wrong order / an implicit cyclic dependency is not detected.

Four packages are required to reproduce:

package Configurable
public constant int FIRST_CONSTANT = 42
public tuple two_ints(int a, int b)
@configurable public var TWO_INTS = two_ints(0, 0)

package Something
import Configurable
public constant SECOND_CONSTANT = FIRST_CONSTANT

package Configurable_config
import Configurable
import Something
@config public var TWO_INTS = two_ints(FIRST_CONSTANT, SECOND_CONSTANT)

package Output
import Configurable
init
    print(TWO_INTS.a.toString() + ", "+ TWO_INTS.b.toString())

Output: 0,0

The observed initialization order is: Something -> Configurable_config -> Configurable -> Output SECOND_CONSTANT is initialized to 0 in Something. In Configurable_config TWO_INTS is set to (0,0). Only in Configurable FIRST_CONSTANT is initialized to 42.

I assume there is an implicit dependency due to the configuration. It makes sense that all packages that import Configurable also require that Configurable_config is initialized. So Something has an explicit dependency on Configurable and an implicit one on Configurable_config. However Configurable_config also has an explicit dependency on Something, resulting in a cyclic dependency.

Note: inlining may remove the issue in this case, since the variables are inlined and never initialized

Jampi0n commented 3 years ago

I experimented a little bit with this and I think a possible solution is to:

The downside of this approach is that the error message is shown for the configurable package, which would be confusing.

To fix this problem one can instead do:

With that, the error message will be shown for the config package.

I have a working implementation of that approach, but the error message in vscode is not always updated properly. That seems to also be the case with regular imports though, so maybe it's fine. Should I make a PR for this?