proper error if attempting to do a containment check against non const range, and some cleanup in asmgen

codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt

@@ -210,25 +210,15 @@ internal class FunctionCallAsmGen(private val program: Program, private val asmg
     private fun registerArgsViaStackEvaluation(stmt: IFunctionCall, sub: Subroutine) {
         // this is called when one or more of the arguments are 'complex' and
         // cannot be assigned to a register easily or risk clobbering other registers.
-        // TODO find another way to prepare the arguments, without using the eval stack
+        // TODO find another way to prepare the arguments, without using the eval stack: use a few temporary variables instead, or use push()/pop() like replaceCallAsmSubStatementWithGosub() in the statement reorderer
 
         if(sub.parameters.isEmpty())
             return
 
-
-        // 1. load all arguments reversed onto the stack: first arg goes last (is on top).
-
+        // load all arguments reversed onto the stack: first arg goes last (is on top).
         for (arg in stmt.args.reversed())
             asmgen.translateExpression(arg)
 
-        // TODO here's an alternative to the above, but for now generates bigger code due to intermediate register steps:
-//        for (arg in stmt.args.reversed()) {
-//            // note this stuff below is needed to (eventually) avoid calling asmgen.translateExpression()
-//            // TODO also This STILL requires the translateNormalAssignment() to be fixed to avoid stack eval for expressions...
-//            val dt = arg.inferType(program).getOr(DataType.UNDEFINED)
-//            asmgen.assignExpressionTo(arg, AsmAssignTarget(TargetStorageKind.STACK, program, asmgen, dt, sub))
-//        }
-
         var argForCarry: IndexedValue<Pair<Expression, RegisterOrStatusflag>>? = null
         var argForXregister: IndexedValue<Pair<Expression, RegisterOrStatusflag>>? = null
         var argForAregister: IndexedValue<Pair<Expression, RegisterOrStatusflag>>? = null

codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AsmAssignment.kt

@@ -36,7 +36,7 @@ internal class AsmAssignTarget(val kind: TargetStorageKind,
                                val array: ArrayIndexedExpression? = null,
                                val memory: DirectMemoryWrite? = null,
                                val register: RegisterOrPair? = null,
-                               val origAstTarget: AssignTarget? = null
+                               val origAstTarget: AssignTarget? = null      // TODO look into removing the need to store this
                                )
 {
     val constMemoryAddress by lazy { memory?.addressExpression?.constValue(program)?.number?.toUInt() ?: 0u}

codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt

@@ -308,22 +308,17 @@ internal class AssignmentAsmGen(private val program: Program,
                                 asmgen.translate(ifelse)
                             }
                             else {
-                                // no orig ast assign target, can't use the workaround, so fallback to stack eval
-                                fallbackToStackEval(value, assign)
+                                // no orig ast assign target so can't use the workaround, so fallback to stack eval
+                                fallbackToStackEval(assign)
                             }
                         } else {
-                            // Everything else just evaluate via the stack.
+                            // All remaining binary expressions just evaluate via the stack for now.
                             // (we can't use the assignment helper functions (assignExpressionTo...) to do it via registers here,
                             // because the code here is the implementation of exactly that...)
-                            fallbackToStackEval(value, assign)
+                            fallbackToStackEval(assign)
                         }
                     }
-                    else -> {
-                        // Everything else just evaluate via the stack.
-                        // (we can't use the assignment helper functions (assignExpressionTo...) to do it via registers here,
-                        // because the code here is the implementation of exactly that...)
-                        fallbackToStackEval(value, assign)
-                    }
+                    else -> throw AssemblyError("weird assignment value type $value")
                 }
             }
             SourceStorageKind.REGISTER -> {
@@ -336,9 +331,13 @@ internal class AssignmentAsmGen(private val program: Program,
         }
     }
 
-    private fun fallbackToStackEval(value: Expression, assign: AsmAssignment) {
-        // TODO DON'T STACK-EVAL THIS... by using a temp var? so that it becomes augmentable assignment expression?
-        asmgen.translateExpression(value)
+    private fun fallbackToStackEval(assign: AsmAssignment) {
+        // TODO DON'T STACK-EVAL... perhaps by using a temp var? so that it becomes augmentable assignment expression?
+        //      or don't try to solve it here in this one case and rather rewrite the whole stack based value evaluation.
+        // this routine is called for assigning a binaryexpression value:
+        // - if it's a boolean comparison expression and the workaround isn't possible (no origTarget ast node)
+        // - for all other binary expressions.
+        asmgen.translateExpression(assign.source.expression!!)
         if (assign.target.datatype in WordDatatypes && assign.source.datatype in ByteDatatypes)
             asmgen.signExtendStackLsb(assign.source.datatype)
         if (assign.target.kind != TargetStorageKind.STACK || assign.target.datatype != assign.source.datatype)
@@ -352,7 +351,7 @@ internal class AssignmentAsmGen(private val program: Program,
             val constRange = range.toConstantIntegerRange()
             if(constRange!=null)
                 return containmentCheckIntoA(containment.element, elementDt.getOr(DataType.UNDEFINED), constRange.toList())
-            TODO("non-const range containment check ${containment.position}")
+            throw AssemblyError("non const range containment check not supported")
         }
         val variable = (containment.iterable as? IdentifierReference)?.targetVarDecl(program)
         if(variable!=null) {

codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt

@@ -427,7 +427,7 @@ class ConstantFoldingOptimizer(private val program: Program) : AstWalker() {
     {
         // NOTE: THIS IS ONLY VALID ON FLOATING POINT CONSTANTS
 
-        // todo: this implements only a small set of possible reorderings at this time, we could think of more
+        // TODO: this implements only a small set of possible reorderings at this time, we could think of more
         if(expr.operator==subExpr.operator) {
             // both operators are the same.
 

codeOptimizers/src/prog8/optimizer/ExpressionSimplifier.kt

@@ -16,13 +16,7 @@ import kotlin.math.abs
 import kotlin.math.log2
 import kotlin.math.pow
 
-/*
-    todo add more peephole expression optimizations
-
-    Investigate what optimizations binaryen has, also see  https://egorbo.com/peephole-optimizations.html
-
- */
-
+// TODO add more peephole expression optimizations? Investigate what optimizations binaryen has, also see  https://egorbo.com/peephole-optimizations.html
 
 class ExpressionSimplifier(private val program: Program, private val errors: IErrorReporter) : AstWalker() {
     private val powersOfTwo = (1..16).map { (2.0).pow(it) }.toSet()

compiler/src/prog8/compiler/Compiler.kt

@@ -275,15 +275,10 @@ fun determineCompilationOptions(program: Program, compTarget: ICompilationTarget
 }
 
 private fun processAst(program: Program, errors: IErrorReporter, compilerOptions: CompilationOptions) {
-    // perform initial syntax checks and processings
     println("Analyzing code...")
     program.preprocessAst(errors)
     program.checkIdentifiers(errors, compilerOptions)
     errors.report()
-    // TODO: turning char literals into UBYTEs via an encoding should really happen in code gen - but for that we'd need DataType.CHAR
-    //       ...but what do we gain from this? We can leave it as it is now: where a char literal is no more than syntactic sugar for an UBYTE value.
-    //       By introduction a CHAR dt, we will also lose the opportunity to do constant-folding on any expression containing a char literal.
-    //       Yes this is different from strings that are only encoded in the code gen phase.
     program.charLiteralsToUByteLiterals(compilerOptions.compTarget, errors)
     errors.report()
     program.constantFold(errors, compilerOptions.compTarget)
@@ -303,13 +298,10 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions
 }
 
 private fun optimizeAst(program: Program, compilerOptions: CompilationOptions, errors: IErrorReporter, functions: IBuiltinFunctions, compTarget: ICompilationTarget) {
-    // optimize the parse tree
     println("Optimizing...")
-
     val remover = UnusedCodeRemover(program, errors, compTarget)
     remover.visit(program)
     remover.applyModifications()
-
     while (true) {
         // keep optimizing expressions and statements until no more steps remain
         val optsDone1 = program.simplifyExpressions(errors)
@@ -346,7 +338,6 @@ private fun writeAssembly(program: Program,
                           errors: IErrorReporter,
                           compilerOptions: CompilationOptions
 ): WriteAssemblyResult {
-    // asm generation directly from the Ast
     compilerOptions.compTarget.machine.initializeZeropage(compilerOptions)
     val variables = VariableExtractor().extractVars(program)
     program.processAstBeforeAsmGeneration(compilerOptions, variables, errors)

compiler/src/prog8/compiler/astprocessing/AstChecker.kt

@@ -1221,6 +1221,10 @@ internal class AstChecker(private val program: Program,
         if(containment.parent is BinaryExpression)
             errors.err("containment check is currently not supported in complex expressions", containment.position)
 
+        val range = containment.iterable as? RangeExpression
+        if(range!=null && range.toConstantIntegerRange()==null)
+            errors.err("containment check requires a constant integer range", range.position)
+
         if(iterableDt.isIterable) {
             val iterableEltDt = ArrayToElementTypes.getValue(iterableDt.getOr(DataType.UNDEFINED))
             val invalidDt = if (elementDt.isBytes) {

compiler/test/ast/TestProg8Parser.kt

@@ -872,7 +872,6 @@ class TestProg8Parser: FunSpec( {
         val ff = start.statements[4] as Assignment
         ff.value shouldBe NumericLiteral(DataType.UBYTE, 255.0, Position.DUMMY)
         val letter = start.statements[6] as Assignment
-        // TODO characters should perhaps not be encoded until code generation, like strings... however this will prevent optimizing expressions with characters
         val encodedletter = PetsciiEncoding.encodePetscii("A", true).getOrElse { fail("petscii error") }.single()
         letter.value shouldBe NumericLiteral(DataType.UBYTE, encodedletter.toDouble(), Position.DUMMY)
     }

docs/source/todo.rst

@@ -3,7 +3,8 @@ TODO
 
 For next release
 ^^^^^^^^^^^^^^^^
-- ...
+- attempt to rework registerArgsViaStackEvaluation() to use tempvars or push()/pop() instead of evalstack based evaluation
+  actually, all function call asmgen code should use the same routine to pass arguments (replaceCallAsmSubStatementWithGosub ?)
 
 
 Need help with

examples/test.p8

@@ -1,14 +1,31 @@
+%import textio
+
 main {
-    str string1 = "default"
-    str string2 = sc:"screencodes"
-    str string3 = iso:"iso"
-    str string4 = petscii:"petscii"
-
-    ubyte char1 = 'd'
-    ubyte char2 = sc:'s'
-    ubyte char3 = iso:'i'
-    ubyte char4 = petscii:'p'
-
     sub start() {
+        ubyte xx = 10
+        ubyte yy = 10
+
+        routine(xx+yy, yy+99, 99, true)
+
+    }
+
+    uword @shared r_arg
+    ubyte @shared r_arg2
+    ubyte @shared r_arg3
+    ubyte @shared r_arg4
+
+    asmsub routine(uword arg @AY, ubyte arg2 @X, ubyte arg3 @R0, ubyte arg4 @Pc) {
+        %asm {{
+            pha
+            adc  #0
+            sta  r_arg4
+            pla
+            sta  r_arg
+            sty  r_arg+1
+            stx  r_arg2
+            lda  cx16.r0
+            sta  r_arg3
+            rts
+        }}
     }
 }