From c0cb2438d59698e957b3e69857ad0d6f85d636ce Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Wed, 28 Jun 2023 23:17:59 +0200
Subject: [PATCH] 1-letter symbols now also prefixed with 'p8p_' to avoid
 assembly errors caused by confusing variable 'a' with register 'a' etc.

---
 .../astprocessing/AsmInstructionNamesFinder.kt      | 10 +++++-----
 .../astprocessing/AsmInstructionNamesReplacer.kt    |  4 ++--
 docs/source/technical.rst                           | 13 +++++++------
 docs/source/todo.rst                                |  4 ++++
 4 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesFinder.kt b/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesFinder.kt
index e5dc4ac06..1e67b44f0 100644
--- a/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesFinder.kt
+++ b/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesFinder.kt
@@ -15,33 +15,33 @@ class  AsmInstructionNamesFinder(val target: ICompilationTarget): IAstVisitor {
     val labels = mutableSetOf<Label>()
     val subroutines = mutableSetOf<Subroutine>()
 
-    private fun isPossibleInstructionName(name: String) = name.length==3 && name.all { it.isLetter() }
+    private fun isPossibleConfusingAsmName(name: String) = (name.length==3 || name.length==1) && name.all { it.isLetter() }
 
     fun foundAny(): Boolean = blocks.isNotEmpty() || variables.isNotEmpty() || subroutines.isNotEmpty() || labels.isNotEmpty()
 
     override fun visit(block: Block) {
-        if(target.name!=VMTarget.NAME && !block.isInLibrary && isPossibleInstructionName(block.name)) {
+        if(target.name!=VMTarget.NAME && !block.isInLibrary && isPossibleConfusingAsmName(block.name)) {
             blocks += block
         }
         super.visit(block)
     }
 
     override fun visit(decl: VarDecl) {
-        if(target.name!=VMTarget.NAME && !decl.definingModule.isLibrary && isPossibleInstructionName(decl.name)) {
+        if(target.name!=VMTarget.NAME && !decl.definingModule.isLibrary && isPossibleConfusingAsmName(decl.name)) {
             variables += decl
         }
         super.visit(decl)
     }
 
     override fun visit(label: Label) {
-        if(target.name!=VMTarget.NAME && !label.definingModule.isLibrary && isPossibleInstructionName(label.name)) {
+        if(target.name!=VMTarget.NAME && !label.definingModule.isLibrary && isPossibleConfusingAsmName(label.name)) {
             labels += label
         }
         super.visit(label)
     }
 
     override fun visit(subroutine: Subroutine) {
-        if(target.name!=VMTarget.NAME && !subroutine.definingModule.isLibrary && isPossibleInstructionName(subroutine.name)) {
+        if(target.name!=VMTarget.NAME && !subroutine.definingModule.isLibrary && isPossibleConfusingAsmName(subroutine.name)) {
             subroutines += subroutine
         }
         super.visit(subroutine)
diff --git a/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesReplacer.kt b/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesReplacer.kt
index 90aa68894..b52e654bc 100644
--- a/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesReplacer.kt
+++ b/compiler/src/prog8/compiler/astprocessing/AsmInstructionNamesReplacer.kt
@@ -22,7 +22,7 @@ class AsmInstructionNamesReplacer(
         }
 
         val newName = identifier.nameInSource.map { ident ->
-            if(ident.length==3 && !identifier.definingModule.isLibrary) {
+            if((ident.length==3 || ident.length==1) && !identifier.definingModule.isLibrary) {
                 val blockTarget = blocks.firstOrNull { it.name==ident }
                 val subTarget = subroutines.firstOrNull {it.name==ident }
                 val varTarget = variables.firstOrNull { it.name==ident }
@@ -84,7 +84,7 @@ class AsmInstructionNamesReplacer(
     override fun after(subroutine: Subroutine, parent: Node): Iterable<IAstModification> {
         val changedParams = mutableListOf<Pair<Int, SubroutineParameter>>()
         subroutine.parameters.withIndex().forEach { (index, param) ->
-            if(param.name.length==3 && param.name.all { it.isLetter() } && !param.definingModule.isLibrary) {
+            if((param.name.length==3 || param.name.length==1) && param.name.all { it.isLetter() } && !param.definingModule.isLibrary) {
                 changedParams.add(index to SubroutineParameter("p8p_${param.name}", param.type, param.position))
             }
         }
diff --git a/docs/source/technical.rst b/docs/source/technical.rst
index 17f739258..23122562f 100644
--- a/docs/source/technical.rst
+++ b/docs/source/technical.rst
@@ -27,14 +27,15 @@ so that more system ram is available for the program code itself.
 Three-letter symbols prefixing in Assembly
 ------------------------------------------
 
-Symbols consisting of three letters such as "brk" or "tax" will confuse the assembler that
-thinks these are cpu instructions. It will likely fail to assemble the program correctly.
-Because of this, prog8 will prefix every 3-letter symbol with "``p8p_``" automatically during compilation.
-So "tax" will become "p8p_tax" in the resulting assembly code.
+Symbols consisting of three letters such as "brk" or "tax", or variables named "a", "x" or "y" could
+confuse the assembler to think these are cpu instructions or registers.
+It will likely fail to assemble the program correctly.
+Because of this, prog8 will prefix every 1- and 3-letter symbol with "``p8p_``" automatically during compilation.
+So "tax" will become "p8p_tax", "a" will become "p8p_a" in the resulting assembly code.
 
 If you're referencing symbols from the prog8 program in hand-written assembly code, you have to take
-this into account. Either prefix the 3-letter symbols in the assembly with "``p8p_``" as well, or just
-choose a shorter or longer symbol name in the first place.
+this into account. Either prefix the 1- and 3-letter symbols in the assembly with "``p8p_``" as well, or just
+choose a symbol name of a different length in the first place.
 
 
 Software stack for expression evaluation
diff --git a/docs/source/todo.rst b/docs/source/todo.rst
index b73b1e786..69517ae09 100644
--- a/docs/source/todo.rst
+++ b/docs/source/todo.rst
@@ -4,6 +4,10 @@ TODO
 - fix array expression assignment bug from Mark in the discord. ("TODO fix temp var name")
 - fix double code gen with tryInplaceModifyWithRemovedRedundantCast() ?   word birdX[j] += testbyte
 
+- prog8->asm symbol name prefixing: prefix ALL symbols with p8_ Also update manual.
+   EXCEPTION: library symbols such as cbm.CHROUT, cx16.r0 etc. should NOT be prefixed.
+   Solution: add %option no_symbol_prefix  to those blocks?
+
 ...