r=(q+r)-c and r=q+(r-c) are now both also 'augmentable', and BinExprSplitter doesn't check for associativeOperator anymore

codeGeneration/src/prog8/compiler/target/cpu6502/codegen/assignment/AugmentableAssignmentAsmGen.kt

@@ -100,7 +100,7 @@ internal class AugmentableAssignmentAsmGen(private val program: Program,
             }
         }
 
-        throw FatalAstException("assignment should be augmentable $binExpr")
+        throw FatalAstException("assignment should follow augmentable rules $binExpr")
     }
 
     private fun inplaceModification(target: AsmAssignTarget, operator: String, origValue: Expression) {

codeOptimizers/src/prog8/optimizer/BinExprSplitter.kt

@@ -49,7 +49,7 @@ X =      BinExpr                                    X   =   LeftExpr
                 if(assignment.target isSameAs binExpr.left || assignment.target isSameAs binExpr.right)
                     return noModifications
 
-                if(binExpr.right.isSimple && !assignment.isAugmentable) {
+                if(binExpr.right.isSimple) {
                     val firstAssign = Assignment(assignment.target.copy(), binExpr.left, binExpr.left.position)
                     val targetExpr = assignment.target.toExpression()
                     val augExpr = BinaryExpression(targetExpr, binExpr.operator, binExpr.right, binExpr.right.position)

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

@@ -97,14 +97,14 @@ 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???
+            // TODO hmm, if both assignments assign to the same thing, can't we just remove the first altogether??? as long as there isn't a function call in the value!
 
             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)
+                if(value.left isSameAs assignment.target) {
                     val assign = Assignment(assignment.target, value.right, nextAssign.position)
                     return listOf(
                         IAstModification.Remove(assignment, parent as IStatementContainer),
@@ -112,6 +112,7 @@ internal class VariousCleanups(val program: Program, val errors: IErrorReporter)
                     )
                 }
             }
+        }
 
         return noModifications
     }

compiler/test/TestOptimization.kt

@@ -455,4 +455,21 @@ class TestOptimization: FunSpec({
         errors.errors[0] shouldContain  "type of value BYTE doesn't match target UBYTE"
         errors.errors[1] shouldContain "value '-1' out of range for unsigned byte"
     }
+
+    test("test augmented expression asmgen") {
+        val src = """
+        main {
+            sub start() {
+                ubyte c
+                ubyte r
+                ubyte q
+                r = (q+r)-c
+                q=r
+                r = q+(r-c)
+                q=r
+            }
+        }"""
+        val result = compileText(C64Target, optimize=false, src, writeAssembly=true).assertSuccess()
+        result.program.entrypoint.statements.size shouldBe 10
+    }
 })

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

@@ -351,6 +351,21 @@ open class Assignment(var target: AssignTarget, var value: Expression, final ove
                 if(binExpr.left isSameAs target)
                     return true  // A = A <operator> Something
 
+                if(binExpr.operator in "+-") {
+                    val leftBinExpr = binExpr.left as? BinaryExpression
+                    if(leftBinExpr!=null && leftBinExpr.operator in "+-") {
+                        // A = (A +- x) +- y
+                        if(leftBinExpr.left isSameAs target || leftBinExpr.right isSameAs target || binExpr.right isSameAs target)
+                            return true
+                    }
+                    val rightBinExpr = binExpr.right as? BinaryExpression
+                    if(rightBinExpr!=null && rightBinExpr.operator in "+-") {
+                        // A = y +- (A +- x)
+                        if(rightBinExpr.left isSameAs target || rightBinExpr.right isSameAs target || binExpr.left isSameAs target)
+                            return true
+                    }
+                }
+
                 if(binExpr.operator in associativeOperators) {
                     if (binExpr.left !is BinaryExpression && binExpr.right isSameAs target)
                         return true  // A = v <associative-operator> A

compilerAst/test/TestProg8Parser.kt

@@ -742,14 +742,11 @@ class TestProg8Parser: FunSpec( {
                     r = not q   ; #9 no
                     r = (q+r)+5 ; #10 yes
                     r = q+(r+5) ; #11 yes
-                    r = (q+r)-5 ; #12 yes            TODO FIX THIS ONE
-                    r = q+(r-5) ; #13 yes            TODO FIX THIS ONE
+                    r = (q+r)-5 ; #12 yes
+                    r = q+(r-5) ; #13 yes
                 }
             }""")
 
-        // TODO fix augmented result for the above mentioned assignments
-        // TODO certain typecast expressions are also augmentable?? what does this even mean, and does the generated code make a difference???
-
         val module = parseModule(src)
         val program = Program("test", DummyFunctions, DummyMemsizer, DummyStringEncoder)
         program.addModule(module)
@@ -766,7 +763,7 @@ class TestProg8Parser: FunSpec( {
         for((idx, pp) in stmts.drop(2).zip(expectedResults).withIndex()) {
             val assign = pp.first as Assignment
             val expected = pp.second
-            withClue("#${idx+1}: should be augmentable but isn't : $assign") {
+            withClue("#${idx+1}: should${if(expected) "" else "n't"} be augmentable: $assign") {
                 assign.isAugmentable shouldBe expected
                 assign.value shouldBe (instanceOf<PrefixExpression>() or instanceOf<BinaryExpression>() or instanceOf<TypecastExpression>())
             }

docs/source/todo.rst

@@ -3,7 +3,9 @@ TODO
 
 For next compiler release (7.4)
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-BUG: fix "assignment isAugmented correctness" test
+fix "test augmented expression asmgen" unittest  (and textelite compilation)
+
+TODO certain typecast expressions are also augmentable?? what does this even mean, and does the generated code make a difference???
 
 optimize TODO in "Add assignment to initialize with zero" in StatementReorderer
 optimize TODO in after(assignment) in VariousCleanups
@@ -12,6 +14,7 @@ optimize: there is an optimizations in AsmOptimizer that can only be done correc
  if it knows about regular ram vs io space ram distinction.
 
 
+
 Blocked by an official Commander-x16 v39 release
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - simplify cx16.joystick_get2() once this cx16 rom issue is resolved: https://github.com/commanderx16/x16-rom/issues/203
@@ -21,6 +24,7 @@ Blocked by an official Commander-x16 v39 release
 Future
 ^^^^^^
 - use UByte instead of Short
+- rethink the whole "isAugmentable" business.  Because the way this is determined, should always also be exactly mirrorred in the AugmentableAssignmentAsmGen or you'll get a crash at code gen time.
 - simplifyConditionalExpression() should not split expression if it still results in stack-based evaluation
 - remove special code generation for while and util expression
   by rewriting while and until expressions into if+jump (just consider them syntactic sugar)