Class static initialization incorrect ordering #8

New Issue

deavmi · 2022-10-13T12:17:14+01:00

deavmi commented

2022-10-13 12:17:14 +01:00

The following code:

module simple_class_ref;

class TestClass
{
    static int value = 2;
}

TestClass p;
int testValue = TestClass.value;

Gives us a code queue where ClassStaticAlloc is at end, this should not be (probably not implemented):

INFO] 1/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.TestClass.value]
[INFO] 2/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_class_ref.TestClass.value, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]
[INFO] 3/6: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_class_ref.TestClass]
[INFO] 4/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.p]
[INFO] 5/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.testValue]
[INFO] 6/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_class_ref.testValue, valInstr: [Instruction: compiler.codegen.instruction.FetchValueVar:fetchVarValName: simple_class_ref.TestClass.value, VarLen: 4]]

The following code: ```d module simple_class_ref; class TestClass { static int value = 2; } TestClass p; int testValue = TestClass.value; ``` Gives us a code queue where `ClassStaticAlloc` is at end, this should not be (probably not implemented): ``` INFO] 1/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.TestClass.value] [INFO] 2/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_class_ref.TestClass.value, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]] [INFO] 3/6: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_class_ref.TestClass] [INFO] 4/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.p] [INFO] 5/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_class_ref.testValue] [INFO] 6/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_class_ref.testValue, valInstr: [Instruction: compiler.codegen.instruction.FetchValueVar:fetchVarValName: simple_class_ref.TestClass.value, VarLen: 4]] ```

deavmi self-assigned this 2022-10-13 12:17:21 +01:00

deavmi added the

typing

label 2022-10-13 12:17:26 +01:00

deavmi added this to the Dependency tree, type-checking and codegen project 2022-10-13 12:17:31 +01:00

deavmi changed title from ~~Class static initlaization incorrec ordering~~ to Class static initialization incorrect ordering

2022-10-13 14:55:28 +01:00

deavmi added the

dependency

needsfix

labels 2022-10-15 15:57:20 +01:00

deavmi commented

2022-10-15 16:33:17 +01:00

Bug found

It is definately this code:

/* TODO: Add class init, see #8 */
/* FIXME: This is probably wrong */
else if(cast(compiler.typecheck.dependency.classes.classStaticDep.ClassStaticNode)dnode)
{
	//....
}

Which is causing the problem. Disabling it fixes stuff, (which makes sense seeing how I rushed that code). The one thing we lose then though, is the ClassStaticAllocate node in it. But instead of trying to re-write that code and do matching and swapping we could look at re-working it maybe? Nah, can't have multiple vardecs depend on it... 👀

We should look into that or simply implementing it with swapping (ew, if not the only way - needs thinking!)

### Bug found It is definately **this** code: ```d /* TODO: Add class init, see #8 */ /* FIXME: This is probably wrong */ else if(cast(compiler.typecheck.dependency.classes.classStaticDep.ClassStaticNode)dnode) { //.... } ``` Which is causing the problem. Disabling it fixes stuff, (which makes sense seeing how I rushed that code). The one thing we lose then though, is the `ClassStaticAllocate` node in it. But instead of trying to re-write that code and do matching and swapping we _could_ look at re-working it maybe? Nah, can't have multiple vardecs depend on it... :eyes: We should look into that or simply implementing it with swapping (ew, if not the only way - needs thinking!)

deavmi commented

2022-10-15 16:36:01 +01:00

Logically speaking it doesn't make sense to make things depend on class allocate. The class allocator depends on them..
In another way it does make sense to have all class stuff depdnd on allocate - it would properly linearize then without swapping. Mmmm....

1. Logically speaking it doesn't make sense to make things depend on class allocate. The class allocator depends on them.. 2. In another way it does make sense to have all class stuff depdnd on allocate - it would properly linearize then without swapping. Mmmm....

deavmi commented

2022-10-15 16:37:09 +01:00

Perhaps the code (where it rears its head in one case):

/* Class-type */
else if(cast(Clazz)variableType)
{
	writeln("Literally hello");
    
    /* Get the static class dependency */
    ClassStaticNode classDependency = classPassStatic(cast(Clazz)variableType);

    /* Make this variable declaration depend on static initalization of the class */
    variableDNode.needs(classDependency);
}

Needs a similiar fix to that of 31c52c0bebeb77e36155e3c62f5fb3d7a7ed7707 on branch vardec_varass_dependency for issue #11.

In other words, the node (module/container) should RATHER depend on the class which THEN depends on the variable.

WRONG - THIS IS WRONG - makes no sense

Perhaps the code (where it rears its head in one case): ```d /* Class-type */ else if(cast(Clazz)variableType) { writeln("Literally hello"); /* Get the static class dependency */ ClassStaticNode classDependency = classPassStatic(cast(Clazz)variableType); /* Make this variable declaration depend on static initalization of the class */ variableDNode.needs(classDependency); } ``` Needs a similiar fix to that of `31c52c0bebeb77e36155e3c62f5fb3d7a7ed7707` on branch `vardec_varass_dependency` for issue #11. --- In other words, the `node` (module/container) should **RATHER** depend on the class which _THEN_ depends on the variable. ## WRONG - THIS IS WRONG - makes no sense

deavmi commented

2022-10-15 16:40:03 +01:00

Two issues

Class allocation for variable types:
- A i;
- A is a class-type
Path initialization x.y.z where y is a class

So we need to analyse those both. The current way we have things seems to make sense for point 1. As for 2 maybe that is causing issues as I haven't actually tried out 2 since the changes made on branch vardec_varass_dependency for issue #11.

For issue 1, on #11's branch we get correct output for this code:

module simple_variables_class;

class A
{
    
}

A x = 1+2;
int y = 2;

Output:

[INFO] 1/5: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A]
[INFO] 2/5: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x]
[INFO] 3/5: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]]
[INFO] 4/5: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y]
[INFO] 5/5: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]

Now also checking this code:

module simple_variables_class;

class A
{
    static int j;
}

A x = 1+2;
int y = 2;

We get this output:

 1/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.j]
[INFO] 2/6: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A]
[INFO] 3/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x]
[INFO] 4/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]]
[INFO] 5/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y]
[INFO] 6/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]

Now how would we deal with this? (We get a "NO MATCHES FIX FOR ME" eror too)

## Two issues 1. Class allocation for variable types: * `A i;` * `A` is a class-type 2. Path initialization `x.y.z` where `y` is a class So we need to analyse those both. The current way we have things seems to make sense for point 1. As for 2 maybe that is causing issues as I haven't actually tried out `2` since the changes made on branch `vardec_varass_dependency` for issue #11. --- For issue 1, on #11's branch we get correct output for this code: ```d module simple_variables_class; class A { } A x = 1+2; int y = 2; ``` Output: ``` [INFO] 1/5: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A] [INFO] 2/5: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x] [INFO] 3/5: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]] [INFO] 4/5: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y] [INFO] 5/5: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]] ``` --- Now **also** checking this code: ```d module simple_variables_class; class A { static int j; } A x = 1+2; int y = 2; ``` We get this output: ``` 1/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.j] [INFO] 2/6: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A] [INFO] 3/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x] [INFO] 4/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]] [INFO] 5/6: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y] [INFO] 6/6: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]] ``` Now how would we deal with this? (We get a "NO MATCHES FIX FOR ME" eror too)

deavmi commented

2022-10-15 16:48:13 +01:00

I may have found it 🤠

deavmi commented

2022-10-15 19:19:14 +01:00

Mmmh, nah....

More complicated examples such as:

module simple_variables_class;

class A
{
    static int j;
    static int k;
}

A x = 1+2;
int y = 2;

Generate:

[INFO] 1/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.j]
[INFO] 2/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.k]
[INFO] 3/7: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A]
[INFO] 4/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x]
[INFO] 5/7: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]]
[INFO] 6/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y]
[INFO] 7/7: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]

What I am thinking is that we may need to just bubble class declarations up to them top or something. As everything but the allocation order is correct above (actual allocation NOT initialization (which is correct above))

Mmmh, nah.... More complicated examples such as: ```d module simple_variables_class; class A { static int j; static int k; } A x = 1+2; int y = 2; ``` Generate: ``` [INFO] 1/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.j] [INFO] 2/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.A.k] [INFO] 3/7: [Instruction: compiler.codegen.instruction.ClassStaticInitAllocate:classStaticInitAllocate: simple_variables_class.A] [INFO] 4/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.x] [INFO] 5/7: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.x, valInstr: [Instruction: compiler.codegen.instruction.BinOpInstr:BinOpType: ADD, LhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 1, Length: 4], RhsValInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]]] [INFO] 6/7: [Instruction: compiler.codegen.instruction.VariableDeclaration:varName: simple_variables_class.y] [INFO] 7/7: [Instruction: compiler.codegen.instruction.VariableAssignmentInstr:assignTo: simple_variables_class.y, valInstr: [Instruction: compiler.codegen.instruction.LiteralValue:Data: 2, Length: 4]] ``` What I am thinking is that we may need to just bubble class declarations up to them top or something. As everything but the allocation order is correct above (actual allocation NOT initialization (which is correct above))

deavmi commented

2022-10-15 19:22:30 +01:00

Perhaps whenever we process these we should add them to their own code queue? I don't know if that makes sense but it is a possibility.

I think the problem we have now is using one single queue for those things. Dependency stuff like the vardecs are all correct.

Perhaps what we could do is idk have a classInitQueue, and then we combine these queus in the order

initQueue (static allocation of memory, no init)
- We won't get things initting in the interleavings wanted with such a solution however, but it will still be valid.
- Eh, we don't care about allocation order, init order (which is working) is what the main focus is
codeQueue (main queue - would include init, vardecs and varassigbnments)

Perhaps whenever we process these we should add them to their own code queue? I don't know if that makes sense but it is a possibility. I think the problem we have now is using one single queue for those things. Dependency stuff like the vardecs are all correct. Perhaps what we could do is idk have a `classInitQueue`, and then we combine these queus in the order 1. `initQueue` (static allocation of memory, no init) * We won't get things initting in the interleavings wanted with such a solution however, but it will still be valid. * Eh, we don't care about allocation order, init order (which is working) is what the main focus is 2. `codeQueue` (main queue - would include init, vardecs and varassigbnments)

deavmi referenced this issue

2022-10-15 20:58:40 +01:00

Clean ups #13

deavmi added a new dependency 2022-10-15 20:59:25 +01:00

#13 Clean ups

deavmi commented

2022-10-15 23:32:21 +01:00

Well well well, this may very well be working.

soy.jpg

74 KiB

❤️ 1

deavmi commented