fixing label names

compiler/src/prog8/ast/AST.kt

@@ -326,8 +326,10 @@ inline fun <reified T> findParentNode(node: Node): T? {
 interface IStatement : Node {
     fun process(processor: IAstProcessor) : IStatement
     fun makeScopedName(name: String): String {
-        // TODO eventually get rid of this scopedName
-        // this is usually cached in a lazy property on the statement object itself (label, subroutine, vardecl)
+        // easy way out is to always return the full scoped name.
+        // it would be nicer to find only the minimal prefixed scoped name, but that's too much hassle for now.
+        // and like this, we can cache the name even,
+        // like in a lazy property on the statement object itself (label, subroutine, vardecl)
         val scope = mutableListOf<String>()
         var statementScope = this.parent
         while(statementScope !is ParentSentinel && statementScope !is Module) {
@@ -336,7 +338,8 @@ interface IStatement : Node {
             }
             statementScope = statementScope.parent
         }
-        scope.add(name)
+        if(name.isNotEmpty())
+            scope.add(name)
         return scope.joinToString(".")
     }
 }
@@ -547,7 +550,6 @@ data class DirectiveArg(val str: String?, val name: String?, val int: Int?, over
 
 data class Label(val name: String, override val position: Position) : IStatement {
     override lateinit var parent: Node
-    val scopedname: String by lazy { makeScopedName(name) }
 
     override fun linkParents(parent: Node) {
         this.parent = parent
@@ -558,6 +560,8 @@ data class Label(val name: String, override val position: Position) : IStatement
     override fun toString(): String {
         return "Label(name=$name, pos=$position)"
     }
+
+    val scopedname: String by lazy { makeScopedName(name) }
 }
 
 

compiler/src/prog8/compiler/Compiler.kt

@@ -159,7 +159,6 @@ internal class Compiler(private val rootModule: Module,
     override fun process(block: Block): IStatement {
         prog.newBlock(block.name, block.address, block.options())
         processVariables(block)
-        prog.label("block."+block.name, false)
         prog.line(block.position)
         translate(block.statements)
         return super.process(block)
@@ -461,8 +460,8 @@ internal class Compiler(private val rootModule: Module,
             }
         } else {
             // regular if..else branching
-            val labelElse = makeLabel("else")
-            val labelEnd = makeLabel("end")
+            val labelElse = makeLabel(branch, "else")
+            val labelEnd = makeLabel(branch, "end")
             val opcode = branchOpcode(branch, true)
             if (branch.elsepart.isEmpty()) {
                 prog.instr(opcode, callLabel = labelEnd)
@@ -480,9 +479,9 @@ internal class Compiler(private val rootModule: Module,
         }
     }
 
-    private fun makeLabel(postfix: String): String {
+    private fun makeLabel(scopeStmt: IStatement, postfix: String): String {
         generatedLabelSequenceNumber++
-        return "_prog8stmt_${generatedLabelSequenceNumber}_$postfix"
+        return "${scopeStmt.makeScopedName("")}.<s-$generatedLabelSequenceNumber-$postfix>"
     }
 
     private fun translate(stmt: IfStatement) {
@@ -528,13 +527,13 @@ internal class Compiler(private val rootModule: Module,
             in WordDatatypes -> Opcode.JZW
             else -> throw CompilerException("invalid condition datatype (expected byte or word) $stmt")
         }
-        val labelEnd = makeLabel("end")
+        val labelEnd = makeLabel(stmt, "end")
         if(stmt.elsepart.isEmpty()) {
             prog.instr(conditionJumpOpcode, callLabel = labelEnd)
             translate(stmt.truepart)
             prog.label(labelEnd)
         } else {
-            val labelElse = makeLabel("else")
+            val labelElse = makeLabel(stmt, "else")
             prog.instr(conditionJumpOpcode, callLabel = labelElse)
             translate(stmt.truepart)
             prog.instr(Opcode.JUMP, callLabel = labelEnd)
@@ -1764,9 +1763,9 @@ internal class Compiler(private val rootModule: Module,
          * break:
          *      nop
          */
-        val loopLabel = makeLabel("loop")
-        val continueLabel = makeLabel("continue")
-        val breakLabel = makeLabel("break")
+        val loopLabel = makeLabel(loop, "loop")
+        val continueLabel = makeLabel(loop, "continue")
+        val breakLabel = makeLabel(loop, "break")
         val indexVarType = if (numElements <= 255) DataType.UBYTE else DataType.UWORD
         val indexVar = loop.body.getLabelOrVariable(ForLoop.iteratorLoopcounterVarname) as VarDecl
 
@@ -1825,9 +1824,9 @@ internal class Compiler(private val rootModule: Module,
          * break:
          *      nop
          */
-        val loopLabel = makeLabel("loop")
-        val continueLabel = makeLabel("continue")
-        val breakLabel = makeLabel("break")
+        val loopLabel = makeLabel(body, "loop")
+        val continueLabel = makeLabel(body, "continue")
+        val breakLabel = makeLabel(body, "break")
 
         continueStmtLabelStack.push(continueLabel)
         breakStmtLabelStack.push(breakLabel)
@@ -1930,9 +1929,9 @@ internal class Compiler(private val rootModule: Module,
         startAssignment.linkParents(body)
         translate(startAssignment)
 
-        val loopLabel = makeLabel("loop")
-        val continueLabel = makeLabel("continue")
-        val breakLabel = makeLabel("break")
+        val loopLabel = makeLabel(body, "loop")
+        val continueLabel = makeLabel(body, "continue")
+        val breakLabel = makeLabel(body, "break")
         val literalStepValue = (range.step as? LiteralValue)?.asNumericValue?.toInt()
 
         continueStmtLabelStack.push(continueLabel)
@@ -2034,9 +2033,9 @@ internal class Compiler(private val rootModule: Module,
          *  break:
          *      nop
          */
-        val loopLabel = makeLabel("loop")
-        val breakLabel = makeLabel("break")
-        val continueLabel = makeLabel("continue")
+        val loopLabel = makeLabel(stmt, "loop")
+        val breakLabel = makeLabel(stmt, "break")
+        val continueLabel = makeLabel(stmt, "continue")
         prog.line(stmt.position)
         breakStmtLabelStack.push(breakLabel)
         continueStmtLabelStack.push(continueLabel)
@@ -2072,9 +2071,9 @@ internal class Compiler(private val rootModule: Module,
          *  break:
          *      nop
          */
-        val loopLabel = makeLabel("loop")
-        val continueLabel = makeLabel("continue")
-        val breakLabel = makeLabel("break")
+        val loopLabel = makeLabel(stmt, "loop")
+        val continueLabel = makeLabel(stmt, "continue")
+        val breakLabel = makeLabel(stmt, "break")
         prog.line(stmt.position)
         breakStmtLabelStack.push(breakLabel)
         continueStmtLabelStack.push(continueLabel)

compiler/src/prog8/compiler/target/c64/AsmGen.kt

@@ -26,7 +26,7 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
         // Because 64tass understands scoped names via .proc / .block,
         // we'll strip the block prefix from all scoped names in the program.
         // Also, convert invalid label names (such as "<anon-1>") to something that's allowed.
-        // Also have to do that for the variablesMarkedForZeropage!
+        // Also have to do that for the variablesMarkedForZeropage!  TODO is this true?
         val newblocks = mutableListOf<IntermediateProgram.ProgramBlock>()
         for(block in program.blocks) {
             val newvars = block.variables.map { symname(it.key, block) to it.value }.toMap().toMutableMap()
@@ -58,9 +58,10 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
         program.blocks.clear()
         program.blocks.addAll(newblocks)
 
-        val newAllocatedZp = program.allocatedZeropageVariables.map { symname(it.key, null) to it.value}
-        program.allocatedZeropageVariables.clear()
-        program.allocatedZeropageVariables.putAll(newAllocatedZp)
+        // TODO is this still needed?????
+//        val newAllocatedZp = program.allocatedZeropageVariables.map { symname(it.key, null) to it.value}
+//        program.allocatedZeropageVariables.clear()
+//        program.allocatedZeropageVariables.putAll(newAllocatedZp)
 
         // make a list of all const floats that are used
         for(block in program.blocks) {
@@ -105,27 +106,18 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
     }
 
 
-    private fun symname(scoped: String, block: IntermediateProgram.ProgramBlock?): String {
+    // convert a fully scoped name (defined in the given block) to a valid assembly symbol name
+    private fun symname(scoped: String, block: IntermediateProgram.ProgramBlock): String {
         if(' ' in scoped)
             return scoped
         val blockLocal: Boolean
-        var name = when {
-            block==null -> {
-                blockLocal=true
-                scoped
-            }
-            scoped.startsWith("${block.name}.") -> {            // TODO DOES THIS EVER OCCUR?
-                blockLocal = true
-                scoped.substring(block.name.length+1)
-            }
-            scoped.startsWith("block.") -> {
-                blockLocal = false
-                scoped
-            }
-            else -> {
-                blockLocal = false
-                scoped
-            }
+        var name = if (scoped.startsWith("${block.name}.")) {
+            blockLocal = true
+            scoped.substring(block.name.length+1)
+        }
+        else {
+            blockLocal = false
+            scoped
         }
         name = name.replace("<", "prog8_").replace(">", "")     // take care of the autogenerated invalid (anon) label names
         if(name=="-")
@@ -229,9 +221,9 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
             out("* = ${block.address?.toHex()}")
         }
 
-        // deal with zeropage variables
+        // deal with zeropage variables    TODO does this still work correctly (the symname was changed a little)
         for(variable in blk.variables) {
-            val sym = symname(blk.name+"."+variable.key, null)
+            val sym = symname(blk.name+"."+variable.key, blk)
             val zpVar = program.allocatedZeropageVariables[sym]
             if(zpVar==null) {
                 // This var is not on the ZP yet. Attempt to move it there (if it's not a float, those take up too much space)
@@ -426,9 +418,6 @@ class AsmGen(val options: CompilationOptions, val program: IntermediateProgram,
     private fun simpleInstr2Asm(ins: Instruction): String? {
         // a label 'instruction' is simply translated into a asm label
         if(ins is LabelInstr) {
-            if(ins.name.startsWith("block."))
-                return ""
-
             val labelresult =
                     if(ins.name.startsWith("${block.name}."))
                         ins.name.substring(block.name.length+1)

examples/test.p8

@@ -2,29 +2,59 @@
 
 ; @todo fix this loop labeling problem (issue #11 on github): it generates invalid asm due to improper label names
 
+
 ~ main {
 
+label2:
+
     sub start() {
 
-        byte var1
+label3:
         byte var1
 
+label4:
 
         sub subsub() {
-            byte var1
+
+label3:
+label4:
             byte var1
         }
 
-        sub subsub() {
+        sub subsub2() {
+label3:
+label4:
             byte var1
             byte var2
             byte var3
-            byte var2
+
+            label5ss2:
+
+                    if A>10 {
+
+            label6ss2:
+                        A=44
+                        while true {
+            label7ss2:
+                            ;derp
+                        }
+                    } else {
+
+                        gameoverss2:
+                            goto gameoverss2
+                    }
+            label8ss2:
+
         }
 
+label5:
+
         if A>10 {
+
+label6:
             A=44
             while true {
+label7:
                 ;derp
             }
         } else {
@@ -32,6 +62,7 @@
             gameover:
                 goto gameover
         }
+label8:
 
     }