simplified name conflict check for sub params

compiler/src/prog8/compiler/Compiler.kt

@@ -288,8 +288,7 @@ private fun processAst(programAst: Program, errors: IErrorReporter, compilerOpti
     // TODO: turning char literals into UBYTEs via an encoding should really happen in code gen - but for that we'd need DataType.CHAR
     // NOTE: we will then lose the opportunity to do constant-folding on any expression containing a char literal, but how often will those occur?
     // Also they might be optimized away eventually in codegen or by the assembler even
-    programAst.charLiteralsToUByteLiterals(errors, compilerOptions.compTarget)
+    programAst.charLiteralsToUByteLiterals(compilerOptions.compTarget)
-    errors.report()
     programAst.constantFold(errors, compilerOptions.compTarget)
     errors.report()
     programAst.reorderStatements(errors)

compiler/src/prog8/compiler/astprocessing/AstExtensions.kt

@@ -88,7 +88,7 @@ internal fun Program.reorderStatements(errors: IErrorReporter) {
     }
 }
 
-internal fun Program.charLiteralsToUByteLiterals(errors: IErrorReporter, enc: IStringEncoding) {
+internal fun Program.charLiteralsToUByteLiterals(enc: IStringEncoding) {
     val walker = object : AstWalker() {
         override fun after(char: CharLiteral, parent: Node): Iterable<IAstModification> {
             return listOf(IAstModification.ReplaceNode(
@@ -123,7 +123,7 @@ internal fun Program.preprocessAst() {
 
 internal fun Program.checkIdentifiers(errors: IErrorReporter, options: CompilationOptions) {
 
-    val checker2 = AstIdentifiersChecker(this, errors, options.compTarget)
+    val checker2 = AstIdentifiersChecker(errors, options.compTarget)
     checker2.visit(this)
 
     if(errors.noErrors()) {

compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt

@@ -1,6 +1,5 @@
 package prog8.compiler.astprocessing
 
-import prog8.ast.Program
 import prog8.ast.base.Position
 import prog8.ast.expressions.StringLiteralValue
 import prog8.ast.statements.*
@@ -9,7 +8,7 @@ import prog8.compiler.IErrorReporter
 import prog8.compiler.functions.BuiltinFunctions
 import prog8.compiler.target.ICompilationTarget
 
-internal class AstIdentifiersChecker(private val program: Program, private val errors: IErrorReporter, private val compTarget: ICompilationTarget) : IAstVisitor {
+internal class AstIdentifiersChecker(private val errors: IErrorReporter, private val compTarget: ICompilationTarget) : IAstVisitor {
     private var blocks = mutableMapOf<String, Block>()
 
     private fun nameError(name: String, position: Position, existing: Statement) {
@@ -69,19 +68,15 @@ internal class AstIdentifiersChecker(private val program: Program, private val e
             if (existing != null && existing !== subroutine)
                 nameError(subroutine.name, subroutine.position, existing)
 
-            // check that there are no local variables, labels, or other subs that redefine the subroutine's parameters. Blocks are okay.
+            // check that there are no local symbols (variables, labels, subs) that redefine the subroutine's parameters.
             val symbolsInSub = subroutine.allDefinedSymbols
             val namesInSub = symbolsInSub.map{ it.first }.toSet()
             val paramNames = subroutine.parameters.map { it.name }.toSet()
             val paramsToCheck = paramNames.intersect(namesInSub)
             for(name in paramsToCheck) {
-                // TODO clean this up? no two separate lookups?
+                val symbol = subroutine.searchSymbol(name)
-                val labelOrVar = subroutine.searchLabelOrVariableNotSubscoped(name, false)
+                if(symbol!=null && symbol.position != subroutine.position)
-                if(labelOrVar!=null && labelOrVar.position != subroutine.position)
+                    nameError(name, symbol.position, subroutine)
-                    nameError(name, labelOrVar.position, subroutine)
-                val sub = subroutine.statements.firstOrNull { it is Subroutine && it.name==name}
-                if(sub!=null)
-                    nameError(name, subroutine.position, sub)
             }
 
             if(subroutine.isAsmSubroutine && subroutine.statements.any{it !is InlineAssembly}) {

compiler/src/prog8/compiler/target/cpu6502/codegen/ExpressionsAsmGen.kt

@@ -28,6 +28,7 @@ internal class ExpressionsAsmGen(private val program: Program, private val asmge
             is FunctionCall -> translateFunctionCallResultOntoStack(expression)
             is ArrayLiteralValue, is StringLiteralValue -> throw AssemblyError("no asm gen for string/array literal value assignment - should have been replaced by a variable")
             is RangeExpr -> throw AssemblyError("range expression should have been changed into array values")
+            is CharLiteral -> throw AssemblyError("charliteral should have been replaced by ubyte using certain encoding")
         }
     }
 

compiler/test/TestCompilerOnImportsAndIncludes.kt

@@ -105,8 +105,8 @@ class TestCompilerOnImportsAndIncludes {
                 val (where, p8Str, binStr) = it
                 dynamicTest("%asmbinary from ${where}folder") {
                     val p8Path = assumeReadableFile(fixturesDir, p8Str)
-                    val binPath = assumeReadableFile(fixturesDir, binStr)
+                    // val binPath = assumeReadableFile(fixturesDir, binStr)
-                    assertNotEquals( // the bug we're testing for (#54) was hidden if outputDir == workinDir
+                    assertNotEquals( // the bug we're testing for (#54) was hidden if outputDir == workingDir
                         workingDir.normalize().toAbsolutePath(),
                         outputDir.normalize().toAbsolutePath(),
                         "sanity check: workingDir and outputDir should not be the same folder"

compilerAst/src/prog8/ast/AstToplevel.kt

@@ -84,7 +84,7 @@ interface IStatementContainer {
     fun isEmpty(): Boolean = statements.isEmpty()
     fun isNotEmpty(): Boolean = statements.isNotEmpty()
 
-    fun searchLabelOrVariableNotSubscoped(name: String, alsoSubroutine: Boolean): Statement? {         // TODO return INamedStatement instead?  and rename to searchSymbol ?
+    fun searchSymbol(name: String): Statement? {
         // this is called quite a lot and could perhaps be optimized a bit more,
         // but adding a memoization cache didn't make much of a practical runtime difference...
         for (stmt in statements) {
@@ -99,47 +99,47 @@ interface IStatementContainer {
                     if(stmt.name==name) return stmt
                 }
                 is Subroutine -> {
-                    if(alsoSubroutine && stmt.name==name)
+                    if(stmt.name==name)
                         return stmt
                 }
                 is AnonymousScope -> {
-                    val found = stmt.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is IfStatement -> {
-                    val found = stmt.truepart.searchLabelOrVariableNotSubscoped(name, alsoSubroutine) ?: stmt.elsepart.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.truepart.searchSymbol(name) ?: stmt.elsepart.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is BranchStatement -> {
-                    val found = stmt.truepart.searchLabelOrVariableNotSubscoped(name, alsoSubroutine) ?: stmt.elsepart.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.truepart.searchSymbol(name) ?: stmt.elsepart.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is ForLoop -> {
-                    val found = stmt.body.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.body.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is WhileLoop -> {
-                    val found = stmt.body.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.body.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is RepeatLoop -> {
-                    val found = stmt.body.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.body.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is UntilLoop -> {
-                    val found = stmt.body.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                    val found = stmt.body.searchSymbol(name)
                     if(found!=null)
                         return found
                 }
                 is WhenStatement -> {
                     stmt.choices.forEach {
-                        val found = it.statements.searchLabelOrVariableNotSubscoped(name, alsoSubroutine)
+                        val found = it.statements.searchSymbol(name)
                         if(found!=null)
                             return found
                     }
@@ -203,7 +203,7 @@ interface INameScope: IStatementContainer, INamedStatement {
             if(scope==null)
                 return null
         }
-        return scope!!.searchLabelOrVariableNotSubscoped(name.last(), true)
+        return scope!!.searchSymbol(name.last())
     }
 
     private fun lookupUnqualified(name: String): Statement? {
@@ -219,7 +219,7 @@ interface INameScope: IStatementContainer, INamedStatement {
         // if it's not found there, jump up one higher in the namespaces and try again.
         var statementScope = this
         while(statementScope !is GlobalNamespace) {
-            val symbol = statementScope.searchLabelOrVariableNotSubscoped(name, true)
+            val symbol = statementScope.searchSymbol(name)
             if(symbol!=null)
                 return symbol
             else

examples/test.p8

@@ -1,85 +1,16 @@
-
-%import syslib
-%import textio
-%zeropage basicsafe
-%option no_sysinit
-
-;  Create a custom character set on the C64.
-
 main {
 
     sub start() {
-        txt.color(1)
+        fubar(1,2,3,4)
-        txt.print("creating charset...\n")
-        charset.make_custom_charset()
-
-        ; activate the new charset in RAM
-        ubyte block = c64.CIA2PRA
-        const ubyte PAGE1 = ((c64.Screen >> 6) & $F0) | ((charset.CHARSET >> 10) & $0E)
-
-        c64.CIA2PRA = (block & $FC) | (lsb(c64.Screen >> 14) ^ $03)
-        c64.VMCSB = PAGE1
-
-        txt.print("\n @ @ @ @\n")
-    }
-}
-
-charset {
-    const uword CHARSET = $2000
-
-    sub copy_rom_charset() {
-        ; copies the charset from ROM to RAM so we can modify it
-
-        sys.set_irqd()
-        ubyte bank = @($0001)
-        @($0001) = bank & %11111011     ; enable CHAREN, so the character rom accessible at $d000
-        sys.memcopy($d000, CHARSET, 256*8*2)  ; copy the charset to RAM
-
-        @($0001) = bank         ; reset previous memory banking
-        sys.clear_irqd()
     }
 
-    sub make_custom_charset() {
+    sub fubar(ubyte aa, ubyte bb, ubyte cc, ubyte dd) {
-        copy_rom_charset()
+        ubyte aa
 
-        ; make all characters italic
+bb:
-        ubyte c
-        for c in 0 to 255 {
-            uword ptr = CHARSET + c*$0008
-            @(ptr) >>= 2
-            @(ptr+1) >>= 2
-            @(ptr+2) >>= 1
-            @(ptr+3) >>= 1
-            ;@(ptr+4) >>= 0
-            ;@(ptr+5) >>= 0
-            @(ptr+6) <<= 1
-            @(ptr+7) <<= 1
 
-            ptr = CHARSET + 256*8 + c*$0008
+        sub cc() {
-            @(ptr) >>= 2
+            dd++
-            @(ptr+1) >>= 2
-            @(ptr+2) >>= 1
-            @(ptr+3) >>= 1
-            ;@(ptr+4) >>= 0
-            ;@(ptr+5) >>= 0
-            @(ptr+6) <<= 1
-            @(ptr+7) <<= 1
         }
-
-        ; add a smiley over the '@'
-
-        ubyte[] smiley = [
-            %00111100,
-            %01000010,
-            %10100101,
-            %10000001,
-            %10100101,
-            %10011001,
-            %01000010,
-            %00111100
-        ]
-
-        sys.memcopy(smiley, CHARSET, len(smiley))
-
     }
 }