[Bug fix] Module level user-defined types which are based on constants, have incorrect types

[SandaruJayawardana]

Related issue #33890,

const int X = 1 + 7;
X y = 2; // No errors, because currently X is resolved as `int`

Reason: In the current implementation, compiler resolves the constants at ConstantValueResolver in semantic analyzer phase. But type nodes of module level variables are already resolved before resolving the constants. Therefore, even compiler updates the constants with correct types in later stage, it will not fix the module level usages (as a type) of the constants.

After having offline discussions with @gimantha, @MaryamZi, @KRVPerera and @rdhananjaya, we came up with following possible ways to fix this issue.

1. After resolving the constants, we can search and update the module level usages which depend on constants.
   cons - I have already tried this method. It is getting complicated for complex types like record.

2. Change the current working flow by moving the ConstantValueResolver into SymbolEnter.
   cons - there can be dependencies which are not available at SymbolEnter

3. Use a middle man that we can update when we have all the information.
   We can take 2 approaches to this:
   Use this middleman everywhere instead of the original type/symbol.
   Register lambdas that know how to change all occurrences of type/symbol represented by this and invoke all of those lambdas to change all places.

• We might have to have 2 classes, one for the type and one for the symbol.
  This is just a holder class that holds the actual type, whenever we want to change the type, we can change the inner type field.
  The problem with this approach is that we need to make sure never to store the inner type value, instead whenever we need to store the type we store the holder type. Failing to adhere to this will cause hald to find bugs.

• We store list of lambdas that know how to change all occurrences of a type.
  For example when ever we store a type somewhere we need to register a lambda function that upon invocation will set a new type in the same location.
  When we want to store a new value (replace the type) we can just invoke those lambdas with the new type and it will be updated in all the places.

Highly appreciate your input on this discussion.

[rdhananjaya]

In my opinion approach number 2 is the ideal solution. I think this will take some considerable effort.

[KRVPerera]

We can also keep unresolved dependent types like X somewhere like a symbol table and resolve them after the ConstantValueResolver takes effect. It is better if we can identify these are in fact unresolved due to constants.

[SandaruJayawardana]

Identified possible scenarios that we have to consider during this fix

const int A = 2;
type B A;

const int A = 2;
A a = 2;

const int A = 1 + B;
const int B = 2;

const map<map<boolean>> A = { "key5": B};
const map<boolean> B = { "key1": true};

type X int;
const X A = 1;

type Foo record {
    string s;
    int i;
};
const Foo X = {s: "a", i: 2};

type Foo record {
    string s;
    int i;
};
type Foo2 record {
    X s;
};
const Foo X = {s: "a", i: 2};

const string A = "b" + "c";
type X A;
const X B = "bc";

[SandaruJayawardana]

const B A = 1 + B;
const int B = 2;

[SandaruJayawardana]

Considering the above examples, we can summarize the following.

1. Constants can depend on other constants or type definitions.
2. Type definitions can depend on constants.
3. Both type node and the expression are required to resolve a constant.

[SandaruJayawardana]

Adding some examples which were discussed during the offline discussion.
10.

type Foo record {
    string s;
    float i;
};
const Foo X = {s: "a", i: 2}; // The type node is required to get the type of `2`

type Foo record {
    string s;
    int i = 10;
};
const Foo X = {s: "a"}; // Defaultable field `i`

[SandaruJayawardana]

Related WIP PR #35927

[SandaruJayawardana]

After having a few offline discussions, we have identified the following options to move the ConstantValueResolver into SymbolEnter.

1. In current implementation, Constants, Type definitions and Class definitions are defined at the same time. Therefore, the type definitions which are dependent on constants may get incorrect types. To solve that, we can define constants first (may have to resolve dependent type definitions also) and call ConstantValueResolver. Next we can define Type definitions and Class definitions. But it is difficult to address scenarios like 6, 7, 8 with this approach.

2. We can individually call the ConstantValueResolver while we are visiting the constants at the SymbolEnter. Here, we have to check whether the constant expressions have any unresolved dependencies (constants which are not resolved yet). If so, skip it and try to resolve it again in the next iteration of defineTypeNodes().
   To find out unresolved dependencies, we can either use SymbolResolver or reorder the constants based on dependencies.
   We can solve the 8 with this approach. Even with this approach, we have to address 6, 7 separately.

[SandaruJayawardana]

6 and 7 are kind of special scenarios because,
For the record type nodes, first we define the shell of the record type and later we define the fields of that record.
For an example,

type Foo record {
    string s;
    int i;
};

As the first step, we define the record Foo with two null fields in SymbolEnter.java/defineTypeNodes(). Later we define the two fields of it in SymbolEnter.java/defineFields().

[SandaruJayawardana]

After having few offline discussions with @hasithaa, we have decided to change the type resolving algorithm similar to nballerina approach (depth first approach). Created a new issue to track this effort (#36538).