From 19e99204b9629e3008fe5ae3ceb564bfd4b111ad Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Fri, 4 Jun 2021 22:42:28 +0200 Subject: [PATCH] fix asm symbol name scoping bug and add unit tests for this --- .../compiler/target/cpu6502/codegen/AsmGen.kt | 47 ++++++++++++++--- compiler/test/AsmgenTests.kt | 52 +++++++++---------- .../src/prog8/ast/statements/AstStatements.kt | 2 +- docs/source/todo.rst | 1 - examples/test.p8 | 6 +-- 5 files changed, 71 insertions(+), 37 deletions(-) diff --git a/compiler/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt b/compiler/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt index e5d70fcc0..cf4911d18 100644 --- a/compiler/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt +++ b/compiler/src/prog8/compiler/target/cpu6502/codegen/AsmGen.kt @@ -494,9 +494,47 @@ internal class AsmGen(private val program: Program, } internal fun asmSymbolName(identifier: IdentifierReference): String { - val target = identifier.targetStatement(program) - val prefix = if(target is Label) "_" else "" - return fixNameSymbols(prefix+identifier.nameInSource.joinToString(".")) + if(identifier.nameInSource.size==2 && identifier.nameInSource[0]=="prog8_slabs") + return identifier.nameInSource.joinToString(".") + + val tgt2 = identifier.targetStatement(program) + if(tgt2==null && (identifier.nameInSource[0].startsWith("_prog8") || identifier.nameInSource[0].startsWith("prog8"))) + return identifier.nameInSource.joinToString(".") + + val target = identifier.targetStatement(program)!! + val targetScope = target.definingSubroutine() + val identScope = identifier.definingSubroutine() + return if(targetScope !== identScope) { + val scopedName = getScopedSymbolNameForTarget(identifier.nameInSource.last(), target) + if(target is Label) { + val last = scopedName.removeLast() + scopedName.add("_$last") + } + fixNameSymbols(scopedName.joinToString(".")) + } else { + if(target is Label) { + val scopedName = identifier.nameInSource.toMutableList() + val last = scopedName.removeLast() + scopedName.add("_$last") + fixNameSymbols(scopedName.joinToString(".")) + } + else fixNameSymbols(identifier.nameInSource.joinToString(".")) + } + } + + internal fun asmVariableName(identifier: IdentifierReference) = + fixNameSymbols(identifier.nameInSource.joinToString(".")) + + private fun getScopedSymbolNameForTarget(actualName: String, target: Statement): MutableList { + val scopedName = mutableListOf(actualName) + var node: Node = target + while (node !is Block) { + node = node.parent + if(node is INameScope) { + scopedName.add(0, node.name) + } + } + return scopedName } internal fun asmSymbolName(regs: RegisterOrPair): String = @@ -505,9 +543,6 @@ internal class AsmGen(private val program: Program, else throw AssemblyError("no symbol name for register $regs") - internal fun asmVariableName(identifier: IdentifierReference) = - fixNameSymbols(identifier.nameInSource.joinToString(".")) - internal fun asmSymbolName(name: String) = fixNameSymbols(name) internal fun asmVariableName(name: String) = fixNameSymbols(name) internal fun asmSymbolName(name: Iterable) = fixNameSymbols(name.joinToString(".")) diff --git a/compiler/test/AsmgenTests.kt b/compiler/test/AsmgenTests.kt index 148140ca2..c087f1856 100644 --- a/compiler/test/AsmgenTests.kt +++ b/compiler/test/AsmgenTests.kt @@ -47,8 +47,13 @@ locallabel: tgt = &locallabel tgt = &var_outside tgt = &label_outside + tgt = &main.start.localvar + tgt = &main.start.locallabel + tgt = &main.var_outside + tgt = &main.label_outside } } + */ val varInSub = VarDecl(VarDeclType.VAR, DataType.UWORD, ZeropageWish.DONTCARE, null, "localvar", NumericLiteralValue.optimalInteger(1234, Position.DUMMY), false, false, false, Position.DUMMY) val var2InSub = VarDecl(VarDeclType.VAR, DataType.UWORD, ZeropageWish.DONTCARE, null, "tgt", null, false, false, false, Position.DUMMY) @@ -59,8 +64,12 @@ locallabel: val assign2 = Assignment(tgt, AddressOf(IdentifierReference(listOf("locallabel"), Position.DUMMY), Position.DUMMY), Position.DUMMY) val assign3 = Assignment(tgt, AddressOf(IdentifierReference(listOf("var_outside"), Position.DUMMY), Position.DUMMY), Position.DUMMY) val assign4 = Assignment(tgt, AddressOf(IdentifierReference(listOf("label_outside"), Position.DUMMY), Position.DUMMY), Position.DUMMY) + val assign5 = Assignment(tgt, AddressOf(IdentifierReference(listOf("main","start","localvar"), Position.DUMMY), Position.DUMMY), Position.DUMMY) + val assign6 = Assignment(tgt, AddressOf(IdentifierReference(listOf("main","start","locallabel"), Position.DUMMY), Position.DUMMY), Position.DUMMY) + val assign7 = Assignment(tgt, AddressOf(IdentifierReference(listOf("main","var_outside"), Position.DUMMY), Position.DUMMY), Position.DUMMY) + val assign8 = Assignment(tgt, AddressOf(IdentifierReference(listOf("main","label_outside"), Position.DUMMY), Position.DUMMY), Position.DUMMY) - val statements = mutableListOf(varInSub, var2InSub, labelInSub, assign1, assign2, assign3, assign4) + val statements = mutableListOf(varInSub, var2InSub, labelInSub, assign1, assign2, assign3, assign4, assign5, assign6, assign7, assign8) val subroutine = Subroutine("start", emptyList(), emptyList(), emptyList(), emptyList(), emptySet(), null, false, false, statements, Position.DUMMY) val labelInBlock = Label("label_outside", Position.DUMMY) val varInBlock = VarDecl(VarDeclType.VAR, DataType.UWORD, ZeropageWish.DONTCARE, null, "var_outside", null, false, false, false, Position.DUMMY) @@ -69,6 +78,7 @@ locallabel: val module = Module("test", mutableListOf(block), Position.DUMMY, false, Path.of("")) module.linkParents(ParentSentinel) val program = Program("test", mutableListOf(module), DummyFunctions(), DummyMemsizer()) + module.program = program return program } @@ -85,8 +95,6 @@ locallabel: val program = createTestProgram() val asmgen = createTestAsmGen(program) - printAst(program) // TODO weg - assertThat(asmgen.asmSymbolName("name"), equalTo("name")) assertThat(asmgen.asmSymbolName(""), equalTo("prog8_name")) assertThat(asmgen.asmSymbolName(RegisterOrPair.R15), equalTo("cx16.r15")) @@ -96,24 +104,6 @@ locallabel: assertThat(asmgen.asmVariableName(listOf("a", "b", "name")), equalTo("a.b.name")) } - /* -main { - -label_outside: - uword var_outside - - sub start () { - uword localvar = 1234 - uword tgt - -locallabel: - tgt = localvar - tgt = &locallabel - tgt = &var_outside - tgt = &label_outside - } -} - */ @Test fun testSymbolNameFromVarIdentifier() { val program = createTestProgram() @@ -124,16 +114,21 @@ locallabel: val localvarIdent = sub.statements.filterIsInstance().first { it.value is IdentifierReference }.value as IdentifierReference assertThat(asmgen.asmSymbolName(localvarIdent), equalTo("localvar")) assertThat(asmgen.asmVariableName(localvarIdent), equalTo("localvar")) + val localvarIdentScoped = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("main", "start", "localvar") }.value as AddressOf).identifier + assertThat(asmgen.asmSymbolName(localvarIdentScoped), equalTo("main.start.localvar")) + assertThat(asmgen.asmVariableName(localvarIdentScoped), equalTo("main.start.localvar")) - // variable from outer scope (note that for Variables, no scoping prefixes are required, + // variable from outer scope (note that for Variables, no scoping prefix symbols are required, // because they're not outputted as locally scoped symbols for the assembler val scopedVarIdent = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("var_outside") }.value as AddressOf).identifier - assertThat(asmgen.asmSymbolName(scopedVarIdent), equalTo("var_outside")) + assertThat(asmgen.asmSymbolName(scopedVarIdent), equalTo("main.var_outside")) assertThat(asmgen.asmVariableName(scopedVarIdent), equalTo("var_outside")) + val scopedVarIdentScoped = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("main", "var_outside") }.value as AddressOf).identifier + assertThat(asmgen.asmSymbolName(scopedVarIdentScoped), equalTo("main.var_outside")) + assertThat(asmgen.asmVariableName(scopedVarIdentScoped), equalTo("main.var_outside")) } @Test - @Disabled("fails until bug is fixed") fun testSymbolNameFromLabelIdentifier() { val program = createTestProgram() val asmgen = createTestAsmGen(program) @@ -143,11 +138,16 @@ locallabel: val localLabelIdent = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("locallabel") }.value as AddressOf).identifier assertThat(asmgen.asmSymbolName(localLabelIdent), equalTo("_locallabel")) assertThat("as a variable it uses different naming rules (no underscore prefix)", asmgen.asmVariableName(localLabelIdent), equalTo("locallabel")) + val localLabelIdentScoped = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("main","start","locallabel") }.value as AddressOf).identifier + assertThat(asmgen.asmSymbolName(localLabelIdentScoped), equalTo("main.start._locallabel")) + assertThat("as a variable it uses different naming rules (no underscore prefix)", asmgen.asmVariableName(localLabelIdentScoped), equalTo("main.start.locallabel")) // label from outer scope needs sope prefixes because it is outputted as a locally scoped symbol for the assembler - // TODO fix this; these both fail now! val scopedLabelIdent = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("label_outside") }.value as AddressOf).identifier assertThat(asmgen.asmSymbolName(scopedLabelIdent), equalTo("main._label_outside")) - assertThat("as a variable it uses different naming rules (no underscore prefix)", asmgen.asmVariableName(scopedLabelIdent), equalTo("main.label_outside")) + assertThat("as a variable it uses different naming rules (no underscore prefix)", asmgen.asmVariableName(scopedLabelIdent), equalTo("label_outside")) + val scopedLabelIdentScoped = (sub.statements.filterIsInstance().first { (it.value as? AddressOf)?.identifier?.nameInSource==listOf("main","label_outside") }.value as AddressOf).identifier + assertThat(asmgen.asmSymbolName(scopedLabelIdentScoped), equalTo("main._label_outside")) + assertThat("as a variable it uses different naming rules (no underscore prefix)", asmgen.asmVariableName(scopedLabelIdentScoped), equalTo("main.label_outside")) } } diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 2b77496a3..a701d9959 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -537,7 +537,7 @@ class InlineAssembly(val assembly: String, override val position: Position) : St } class AnonymousScope(override var statements: MutableList, - override val position: Position) : INameScope, Statement() { + override val position: Position) : INameScope, Statement() { // TODO this isn't really a namescope...(names are scoped to the subroutine level) override val name: String = "" override lateinit var parent: Node diff --git a/docs/source/todo.rst b/docs/source/todo.rst index fc71a8658..150a29c1b 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,7 +2,6 @@ TODO ==== -- fix asm symbol name scoping bug and complete the unit tests for this (testSymbolNameFromLabelIdentifier) - add example in documentation for %asminclude and %asmbinary on how to refer to its contents via label or w/e - test all examples (including imgviewer, assembler and petaxian) before release of the new version diff --git a/examples/test.p8 b/examples/test.p8 index cb9b0dbd1..0fda1c93c 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,9 +1,6 @@ %import textio %zeropage dontuse -; TODO FIX ASM SYMBOL NAME SCOPING BUGS -; TODO make unit tests for this - main { @@ -28,6 +25,9 @@ label: xx = main.sub2.sub2var txt.print_uwhex(xx, true) txt.nl() + xx = &main.start.label_local + txt.print_uwhex(xx, true) + txt.nl() label_local: return