fix improperly changed behavior about =0 initializer

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

@@ -56,15 +56,12 @@ internal class StatementReorderer(val program: Program, val errors: IErrorReport
                             return noModifications
                         }
                         val nextStmt  = decl.nextSibling()
-                        val nextAssign = nextStmt as? Assignment
                         val nextFor = nextStmt as? ForLoop
                         val hasNextForWithThisLoopvar = nextFor?.loopVar?.nameInSource==listOf(decl.name)
-                        val hasNextAssignmentThatAlsoInitializes = nextAssign!=null
+                        if (!hasNextForWithThisLoopvar) {
-                                && nextAssign.target isSameAs IdentifierReference(listOf(decl.name), Position.DUMMY)
-                                && !nextAssign.isAugmentable
-                        if (!hasNextAssignmentThatAlsoInitializes && !hasNextForWithThisLoopvar) {
                             // Add assignment to initialize with zero
                             // Note: for block-level vars, this will introduce assignments in the block scope. These have to be dealt with correctly later.
+                            // TODO optimize this a bit so that we don't add this statement if the next one would already be an assignment to the same variable
                             val identifier = IdentifierReference(listOf(decl.name), decl.position)
                             val assignzero = Assignment(AssignTarget(identifier, null, null, decl.position), decl.zeroElementValue(), decl.position)
                             return listOf(IAstModification.InsertAfter(

compiler/src/prog8/compiler/astprocessing/VariousCleanups.kt

@@ -97,8 +97,20 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter)
 
         val nextAssign = assignment.nextSibling() as? Assignment
         if(nextAssign!=null && nextAssign.target.isSameAs(assignment.target, program)) {
+            // TODO hmm, if both assignments assign to the same thing, can't we just remove the first altogether???
+
             if(nextAssign.value isSameAs assignment.value)
                 return listOf(IAstModification.Remove(assignment, parent as IStatementContainer))
+
+            if((assignment.value as? NumericLiteralValue)?.number==0.0 && nextAssign.isAugmentable) {
+                val value = nextAssign.value as BinaryExpression
+                require(value.left isSameAs assignment.target)
+                val assign = Assignment(assignment.target, value.right, nextAssign.position)
+                return listOf(
+                    IAstModification.Remove(assignment, parent as IStatementContainer),
+                    IAstModification.ReplaceNode(nextAssign, assign, parent)
+                )
+            }
         }
 
         return noModifications

compiler/test/TestOptimization.kt

@@ -250,22 +250,21 @@ class TestOptimization: FunSpec({
             main {
                 sub start() {
                     ubyte ub
-                    float ff
+                    float ff = 1.0
                     ff += (ub as float)         ; operator doesn't matter
                 }
             }
         """
-        val result1 = compileText(C64Target, optimize=false, src, writeAssembly = false).assertSuccess()
+        val result = compileText(C64Target, optimize=false, src, writeAssembly = false).assertSuccess()
-
+        val assignFF = result.program.entrypoint.statements.last() as Assignment
-        val assignYY = result1.program.entrypoint.statements.last() as Assignment
+        assignFF.isAugmentable shouldBe true
-        assignYY.isAugmentable shouldBe true
+        assignFF.target.identifier!!.nameInSource shouldBe listOf("ff")
-        assignYY.target.identifier!!.nameInSource shouldBe listOf("ff")
+        val value = assignFF.value as BinaryExpression
-        val value = assignYY.value as BinaryExpression
         value.operator shouldBe "+"
         value.left shouldBe IdentifierReference(listOf("ff"), Position.DUMMY)
         value.right shouldBe instanceOf<TypecastExpression>()
 
-        val asm = generateAssembly(result1.program)
+        val asm = generateAssembly(result.program)
         asm.valid shouldBe true
     }
 
@@ -326,10 +325,12 @@ class TestOptimization: FunSpec({
         val src="""
             main {
                 sub start()  {
-                    ubyte z1
+                    ubyte @shared z1
                     z1 = 10
-                    ubyte z2
+                    ubyte @shared z2
                     z2 = z1+z2+5      
+                    ubyte @shared z3
+                    z3 = z1+z3-5      
                 }
             }"""
         val result = compileText(C64Target, optimize=true, src, writeAssembly=false).assertSuccess()
@@ -337,27 +338,34 @@ class TestOptimization: FunSpec({
         ubyte z1
         z1 = 10
         ubyte z2
-        z2 = 0
+        z2 = z1
-        z2 += z1        ; TODO actually add optimization to make this even better: no =0, and this should become z2 = z1
         z2 += 5
+        ubyte z3
+        z3 = z1
+        z3 -= 5
          */
         val statements = result.program.entrypoint.statements
-        statements.size shouldBe 6        // TODO 5
+        statements.size shouldBe 8
         val z1decl = statements[0] as VarDecl
         val z1init = statements[1] as Assignment
         val z2decl = statements[2] as VarDecl
         val z2init = statements[3] as Assignment
-        val z2plus1 = statements[4] as Assignment
+        val z2plus = statements[4] as Assignment
-        val z2plus2= statements[5] as Assignment
+        val z3decl = statements[5] as VarDecl
+        val z3init = statements[6] as Assignment
+        val z3plus = statements[7] as Assignment
 
         z1decl.name shouldBe "z1"
         z1init.value shouldBe NumericLiteralValue(DataType.UBYTE, 10.0, Position.DUMMY)
         z2decl.name shouldBe "z2"
-        z2init.value shouldBe NumericLiteralValue(DataType.UBYTE, 0.0, Position.DUMMY)
+        z2init.value shouldBe IdentifierReference(listOf("z1"), Position.DUMMY)
-        z2plus1.isAugmentable shouldBe true
+        z2plus.isAugmentable shouldBe true
-        (z2plus1.value as BinaryExpression).operator shouldBe "+"
+        (z2plus.value as BinaryExpression).operator shouldBe "+"
-        (z2plus1.value as BinaryExpression).right shouldBe IdentifierReference(listOf("z1"), Position.DUMMY)
+        (z2plus.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY)
-        (z2plus2.value as BinaryExpression).operator shouldBe "+"
+        z3decl.name shouldBe "z3"
-        (z2plus2.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY)
+        z3init.value shouldBe IdentifierReference(listOf("z1"), Position.DUMMY)
+        z3plus.isAugmentable shouldBe true
+        (z3plus.value as BinaryExpression).operator shouldBe "-"
+        (z3plus.value as BinaryExpression).right shouldBe NumericLiteralValue(DataType.UBYTE, 5.0, Position.DUMMY)
     }
 })

compiler/test/TestSubroutines.kt

@@ -9,6 +9,7 @@ import io.kotest.matchers.types.instanceOf
 import prog8.ast.base.DataType
 import prog8.ast.expressions.*
 import prog8.ast.statements.*
+import prog8.compiler.printAst
 import prog8.compiler.target.C64Target
 import prog8tests.helpers.ErrorReporterForTests
 import prog8tests.helpers.assertFailure
@@ -191,10 +192,10 @@ class TestSubroutines: FunSpec({
         val text="""
             main {
               sub thing(uword rr) {
-                ubyte xx = rr[1]    ; should still work as var initializer that will be rewritten
+                ubyte @shared xx = rr[1]    ; should still work as var initializer that will be rewritten
-                ubyte yy
+                ubyte @shared yy
                 yy = rr[2]
-                uword other
+                uword @shared other
                 ubyte zz = other[3]
               }
             
@@ -210,14 +211,14 @@ class TestSubroutines: FunSpec({
         val block = module.statements.single() as Block
         val thing = block.statements.filterIsInstance<Subroutine>().single {it.name=="thing"}
         block.name shouldBe "main"
-        thing.statements.size shouldBe 10          // rr, xx, xx assign, yy, yy assign, other, other assign 0, zz, zz assign, return
+        thing.statements.size shouldBe 11          // rr paramdecl, xx, xx assign, yy decl, yy init 0, yy assign, other, other assign 0, zz, zz assign, return
         val xx = thing.statements[1] as VarDecl
         withClue("vardecl init values must have been moved to separate assignments") {
             xx.value shouldBe null
         }
         val assignXX = thing.statements[2] as Assignment
-        val assignYY = thing.statements[4] as Assignment
+        val assignYY = thing.statements[5] as Assignment
-        val assignZZ = thing.statements[8] as Assignment
+        val assignZZ = thing.statements[9] as Assignment
         assignXX.target.identifier!!.nameInSource shouldBe listOf("xx")
         assignYY.target.identifier!!.nameInSource shouldBe listOf("yy")
         assignZZ.target.identifier!!.nameInSource shouldBe listOf("zz")

docs/source/todo.rst

@@ -5,6 +5,9 @@ For next compiler release (7.4)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 BUG: Fix C-64sound issue in petaxian (regression since 7.3, sound on c64 build works fine on older versions)
 
+optimize TODO in "Add assignment to initialize with zero" in StatementReorderer
+optimize TODO in after(assignment) in VariousCleanups
+
 
 Blocked by an official Commander-x16 v39 release
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^