From 5b63590ebf96f5f95abcf138b5915f4d71880927 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Mon, 3 Mar 2025 22:26:19 +0100 Subject: [PATCH] fix symbol prefixing bug triggered by certain usage of %option no_symbol_prefixing --- .../src/prog8/codegen/cpu6502/AsmGen.kt | 26 +++++++++++ .../codegen/cpu6502/FunctionCallAsmGen.kt | 4 +- compiler/test/TestScoping.kt | 29 +++++++++++++ docs/source/todo.rst | 2 - examples/test.p8 | 43 +++---------------- 5 files changed, 64 insertions(+), 40 deletions(-) diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index e126ca791..c37167fe6 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -93,9 +93,35 @@ class AsmGen6502(val prefixSymbols: Boolean, private val lastGeneratedLabelSeque node.children.forEach { prefixSymbols(it) } } + fun maybePrefixFunctionCallsAndIdentifierReferences(node: PtNode) { + if(node is PtFunctionCall) { + // function calls to subroutines defined in a block that does NOT have NoSymbolPrefixing, still have to be prefixed at the call site + val stNode = st.lookup(node.name)!! + if(stNode.astNode!!.definingBlock()?.options?.noSymbolPrefixing!=true) { + val index = node.parent.children.indexOf(node) + functionCallsToPrefix += node.parent to index + } + } + else if (node is PtIdentifier) { + // identifier references to things defined in a block that does NOT have NoSymbolPrefixing, still have to be prefixed at the referencing point + var lookupName = node.name + if(node.type.isSplitWordArray && (lookupName.endsWith("_lsb") || lookupName.endsWith("_msb"))) { + lookupName = lookupName.dropLast(4) + } + val stNode = st.lookup(lookupName) ?: throw AssemblyError("unknown identifier $node") + if(stNode.astNode!!.definingBlock()?.options?.noSymbolPrefixing!=true) { + val index = node.parent.children.indexOf(node) + nodesToPrefix += node.parent to index + } + } + node.children.forEach { maybePrefixFunctionCallsAndIdentifierReferences(it) } + } + program.allBlocks().forEach { block -> if (!block.options.noSymbolPrefixing) { prefixSymbols(block) + } else { + maybePrefixFunctionCallsAndIdentifierReferences(block) } } diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt index cfdcdeb89..71cc2aae8 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/FunctionCallAsmGen.kt @@ -32,8 +32,8 @@ internal class FunctionCallAsmGen(private val program: PtProgram, private val as // NOTE: does NOT output code to save/restore the X register for this call! Every caller should deal with this in their own way!! // (you can use subroutine.shouldSaveX() and saveX()/restoreX() routines as a help for this) - val symbol = asmgen.symbolTable.lookup(call.name) - val sub = symbol?.astNode as IPtSubroutine + val symbol = asmgen.symbolTable.lookup(call.name)!! + val sub = symbol.astNode as IPtSubroutine val subAsmName = asmgen.asmSymbolName(call.name) if(sub is PtAsmSub) { diff --git a/compiler/test/TestScoping.kt b/compiler/test/TestScoping.kt index 20e194750..f253fe6a2 100644 --- a/compiler/test/TestScoping.kt +++ b/compiler/test/TestScoping.kt @@ -445,4 +445,33 @@ WARN name 'var1Warn' shadows occurrence at... errors.errors[3] shouldContain "internalOk" errors.errors[3] shouldContain "line 11" } + + test("ast node linkage and lookups ok even with no symbol prefixing") { + val src=""" +main { + sub start() { + thing.routine() + } +} + +thing { + %option no_symbol_prefixing + + sub routine() { + other.something() + other.counter++ + } +} + +other { + sub something() { + } + + uword @shared counter +} +""" + + compileText(C64Target(), false, src, writeAssembly = true) shouldNotBe null + } + }) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 26f3be03c..3cc7ead87 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,8 +1,6 @@ TODO ==== -- fix scoping bug - - Look at github PR for improved romability (see github issue 149) Also search for "TODO: Romable" - Sorting module gnomesort_uw could be optimized more by fully rewriting it in asm? Shellshort seems consistently faster even if most of the words are already sorted. diff --git a/examples/test.p8 b/examples/test.p8 index 975a67890..d4a382eb5 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,52 +1,23 @@ -%import textio -%option no_sysinit -%zeropage basicsafe +; scoping bug: main { - const ubyte VALUE = 123 - sub start() { - uword @shared @nozp location = $4000 - - @($3fff) = 55 - @($4000) = 56 - @($4001) = 57 - - txt.print_ub(@($3fff)) - txt.spc() - txt.print_ub(@($4000)) - txt.spc() - txt.print_ub(@($4001)) - txt.nl() - - for location in $4000 to $4002 { - @(location-1) = VALUE - txt.print_ub(@($3fff)) - txt.spc() - txt.print_ub(@($4000)) - txt.spc() - txt.print_ub(@($4001)) - txt.nl() - } + thing.routine() } } - -/** TODO scoping bug -; scoping bug: - - -main { - +thing { %option no_symbol_prefixing - sub start() { + sub routine() { other.something() + other.counter++ } } other { sub something() { } + + uword @shared counter } -**/