fix problems with moving vardecls from inner scope to subroutine scope

compiler/src/prog8/compiler/BeforeAsmGenerationAstChanger.kt

@@ -18,15 +18,17 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I
     override fun after(decl: VarDecl, parent: Node): Iterable<IAstModification> {
         subroutineVariables.add(decl.name to decl)
         if (decl.value == null && !decl.autogeneratedDontRemove && decl.type == VarDeclType.VAR && decl.datatype in NumericDatatypes) {
-            // a numeric vardecl without an initial value is initialized with zero,
+            // A numeric vardecl without an initial value is initialized with zero,
-            // unless there's already an assignment below, that initializes the value
+            // unless there's already an assignment below, that initializes the value.
+            // This allows you to restart the program and have the same starting values of the variables
             if(decl.allowInitializeWithZero)
             {
                 val nextAssign = decl.definingScope().nextSibling(decl) as? Assignment
                 if (nextAssign != null && nextAssign.target isSameAs IdentifierReference(listOf(decl.name), Position.DUMMY))
                     decl.value = null
-                else
+                else {
                     decl.value = decl.zeroElementValue()
+                }
             }
         }
         return noModifications
@@ -80,17 +82,22 @@ internal class BeforeAsmGenerationAstChanger(val program: Program, val errors: I
         val sub = scope.definingSubroutine()
         if (sub != null) {
             // move vardecls of the scope into the upper scope. Make sure the position remains the same!
-            val numericVarsWithValue = decls.filter { it.value != null && it.datatype in NumericDatatypes }
+            val replacements = mutableListOf<IAstModification>()
-            val replaceVardecls =numericVarsWithValue.map {
+            val movements = mutableListOf<IAstModification.InsertFirst>()
-                val initValue = it.value!!  // assume here that value has always been set by now
+
-                it.value = null     // make sure no value init assignment for this vardecl will be created later (would be superfluous)
+            for(decl in decls) {
-                val target = AssignTarget(IdentifierReference(listOf(it.name), it.position), null, null, it.position)
+                if(decl.value!=null && decl.datatype in NumericDatatypes) {
-                val assign = Assignment(target, initValue, it.position)
+                    val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position)
-                initValue.parent = assign
+                    val assign = Assignment(target, decl.value!!, decl.position)
-                IAstModification.ReplaceNode(it, assign, scope)
+                    replacements.add(IAstModification.ReplaceNode(decl, assign, scope))
+                    decl.value = null
+                    decl.allowInitializeWithZero = false
+                } else {
+                    replacements.add(IAstModification.Remove(decl, scope))
+                }
+                movements.add(IAstModification.InsertFirst(decl, sub))
             }
-            val moveVardeclsUp = decls.map { IAstModification.InsertFirst(it, sub) }
+            return replacements + movements
-            return replaceVardecls + moveVardeclsUp
         }
         return noModifications
     }

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

@@ -1,9 +1,6 @@
 package prog8.compiler.astprocessing
 
-import prog8.ast.IFunctionCall
+import prog8.ast.*
-import prog8.ast.Module
-import prog8.ast.Node
-import prog8.ast.Program
 import prog8.ast.base.*
 import prog8.ast.expressions.*
 import prog8.ast.statements.*
@@ -201,18 +198,15 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport
         if(declValue!=null && decl.type== VarDeclType.VAR && decl.datatype in NumericDatatypes) {
             val declConstValue = declValue.constValue(program)
             if(declConstValue==null) {
-                // move the vardecl (without value) to the scope and replace this with a regular assignment
+                // move the vardecl (without value) to the scope of the defining subroutine and put a regular assignment in its place here.
-                // Unless we're dealing with a floating point variable because that will actually make things less efficient at the moment (because floats are mostly calcualated via the stack)
+                decl.value = null
-                if(decl.datatype!=DataType.FLOAT) {
+                decl.allowInitializeWithZero = false
-                    decl.value = null
+                val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position)
-                    decl.allowInitializeWithZero = false
+                val assign = Assignment(target, declValue, decl.position)
-                    val target = AssignTarget(IdentifierReference(listOf(decl.name), decl.position), null, null, decl.position)
+                return listOf(
-                    val assign = Assignment(target, declValue, decl.position)
+                        IAstModification.ReplaceNode(decl, assign, parent),
-                    return listOf(
+                        IAstModification.InsertFirst(decl, decl.definingSubroutine() as INameScope)
-                            IAstModification.ReplaceNode(decl, assign, parent),
+                )
-                            IAstModification.InsertFirst(decl.copy(), decl.definingScope())
-                    )
-                }
             }
         }
         return noModifications

compilerAst/src/prog8/ast/statements/AstStatements.kt

@@ -281,7 +281,12 @@ open class VarDecl(val type: VarDeclType,
         return result
     }
 
-    fun copy() = VarDecl(type, declaredDatatype, zeropage, arraysize, name, structName, value, isArray, autogeneratedDontRemove, position)
+    fun copy(): VarDecl {
+        val c = VarDecl(type, declaredDatatype, zeropage, arraysize, name, structName, value, isArray, autogeneratedDontRemove, position)
+        c.allowInitializeWithZero = this.allowInitializeWithZero
+        c.structHasBeenFlattened = this.structHasBeenFlattened
+        return c
+    }
 }
 
 // a vardecl used only for subroutine parameters

docs/source/todo.rst

@@ -2,9 +2,6 @@
 TODO
 ====
 
-- fix: cube3d is suddenly way bigger than it was a few changes ago
-
-
 - make sure that in if statements, the left and right operand of the comparison is never a complex expression
   anymore (only number, variable, addressof or memread) by rewriting if  <left> <op> <right>  {..} into:
     if_eval_left = left

examples/test.p8

@@ -2,35 +2,15 @@
 %zeropage basicsafe
 
 main {
+    sub start()  {
+        word total
 
-    sub start() {
+        repeat 10 {
-        ubyte yy
+            word wcosa
-        ubyte joy=1
+            word zz=1
-        ubyte zz
+            total += zz
-
-        joy >>= 1
-        if_cs
-            yy++
-
-        joy >>= 1
-        if_cs
-            yy++
-
-; TODO the shifting checks above result in way smaller code than this:
-        if joy+44 > 33 {
-            yy++
         }
 
-        yy=joy+44>33
+        txt.print_w(total)
-        if yy {
-            yy++
-        }
-
-        if joy & %00000001
-            yy++
-        if joy & %00000010
-            yy++
-
     }
 }
-