From 42db3085dfd8862e2f884150f1e64c370b4c28de Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 14 Nov 2023 22:47:31 +0100 Subject: [PATCH] improve the way %option merge works, you can now merge your own code with library code for instance. --- compiler/src/prog8/compiler/ModuleImporter.kt | 16 ++++++----- .../astprocessing/AstIdentifiersChecker.kt | 11 +++++--- .../TestImportedModulesOrderAndOptions.kt | 27 +++++++++++++++++++ docs/source/syntaxreference.rst | 2 +- docs/source/todo.rst | 2 -- examples/test.p8 | 22 ++++++++------- 6 files changed, 58 insertions(+), 22 deletions(-) diff --git a/compiler/src/prog8/compiler/ModuleImporter.kt b/compiler/src/prog8/compiler/ModuleImporter.kt index 7ff74a280..7fa648fac 100644 --- a/compiler/src/prog8/compiler/ModuleImporter.kt +++ b/compiler/src/prog8/compiler/ModuleImporter.kt @@ -110,14 +110,18 @@ class ModuleImporter(private val program: Program, // their content has to be merged into already existing other block with the same name. val blocks = importedModule.statements.filterIsInstance() for(block in blocks) { - if("merge" in block.options()) { - val existingBlock = program.allBlocks.firstOrNull { it.name==block.name && it !== block} - if(existingBlock!=null) { + val blockHasMergeOption = "merge" in block.options() + val existingBlock = program.allBlocks.firstOrNull { it.name==block.name && it !== block} + if(existingBlock!=null) { + val existingBlockHasMergeOption = "merge" in existingBlock.options() + if (blockHasMergeOption || existingBlockHasMergeOption) { + // transplant the contents existingBlock.statements.addAll(block.statements.filter { it !is Directive }) importedModule.statements.remove(block) - } else { - val merges = block.statements.filter { it is Directive && it.directive=="%option" && it.args.any { a->a.name=="merge" } } - block.statements.removeAll(merges.toSet()) + + if(blockHasMergeOption && !existingBlockHasMergeOption) { + existingBlock.statements.add(0, Directive("%option", listOf(DirectiveArg(null, "merge", null, block.position)), block.position)) + } } } } diff --git a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt index 811915071..04ebcce75 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt @@ -33,10 +33,13 @@ internal class AstIdentifiersChecker(private val errors: IErrorReporter, override fun visit(block: Block) { val existing = blocks[block.name] if(existing!=null) { - if(block.isInLibrary) - nameError(existing.name, existing.position, block) - else - nameError(block.name, block.position, existing) + // we allow duplicates if at least one of them has %option merge + if("merge" !in existing.options() + block.options()) { + if (block.isInLibrary) + nameError(existing.name, existing.position, block) + else + nameError(block.name, block.position, existing) + } } else blocks[block.name] = block diff --git a/compiler/test/TestImportedModulesOrderAndOptions.kt b/compiler/test/TestImportedModulesOrderAndOptions.kt index a877e71b2..3c894247c 100644 --- a/compiler/test/TestImportedModulesOrderAndOptions.kt +++ b/compiler/test/TestImportedModulesOrderAndOptions.kt @@ -3,10 +3,12 @@ package prog8tests import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldStartWith import prog8.code.core.ZeropageType import prog8.code.core.internedStringsModuleName import prog8.code.target.C64Target +import prog8.code.target.VMTarget import prog8.compiler.determineCompilationOptions import prog8.compiler.parseMainModule import prog8tests.helpers.ErrorReporterForTests @@ -108,4 +110,29 @@ main { } } + test("merge option works on library modules") { + val src=""" +%zeropage basicsafe +%import textio + +txt { + %option merge + sub println(uword string) { + txt.print(string) + txt.nl() + } +} + +main { + sub start() { + txt.lowercase() + txt.println("Hello, world1") + txt.println("Hello, world2") + txt.println("Hello, world3") + } +}""" + compileText(VMTarget(), optimize = false, src) shouldNotBe null + } + + }) diff --git a/docs/source/syntaxreference.rst b/docs/source/syntaxreference.rst index dd815091c..680f2eadc 100644 --- a/docs/source/syntaxreference.rst +++ b/docs/source/syntaxreference.rst @@ -143,7 +143,7 @@ Directives Warning: if you use this to align array variables in the block, these have to be initialized with a value to make them stay in the block and get aligned properly. Otherwise they'll end up at a random spot in the BSS section and the alignment doesn't apply there. - ``align_page`` (in a block) will make the assembler align the start address of this block on a page boundary in memory (so, the LSB of the address is 0). Warning: if you use this to align array variables in the block, these have to be initialized with a value to make them stay in the block and get aligned properly. Otherwise they'll end up at a random spot in the BSS section and the alignment doesn't apply there. - - ``merge`` (in a block) will merge this block's contents into an already existing block with the same name. Useful in library scenarios. + - ``merge`` (in a block) will merge this block's contents into an already existing block with the same name. Useful in library scenarios. Can result in a bunch of unused symbol warnings, this depends on the import order. - ``splitarrays`` (block or module) makes all word-arrays in this scope lsb/msb split arrays (as if they all have the @split tag). See Arrays. - ``no_symbol_prefixing`` (block) makes the compiler *not* use symbol-prefixing when translating prog8 code into assembly. Only use this if you know what you're doing because it could result in invalid assembly code being generated. diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 512ba6691..cfa22601c 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -2,8 +2,6 @@ TODO ==== -- improve the working of %option merge: should be able to merge your own stuff into say textio. , and improve the docs about it too. - - [on branch: shortcircuit] investigate McCarthy evaluation again? this may also reduce code size perhaps for things like if a>4 or a<2 .... - [on branch: ir-less-branch-opcodes] IR: reduce the number of branch instructions such as BEQ, BEQR, etc (gradually), replace with CMP(I) + status branch instruction - IR: reduce amount of CMP/CMPI after instructions that set the status bits correctly (LOADs? INC? Bitwise operations, etc), but only after setting the status bits is verified! diff --git a/examples/test.p8 b/examples/test.p8 index 1ae0ece23..adefd15a9 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,15 +1,19 @@ %zeropage basicsafe +%import textio + +txt { + %option merge + sub println(uword string) { + txt.print(string) + txt.nl() + } +} main { sub start() { - str name = "thing" - modify(name) - - sub modify(str arg) { - ubyte n=1 - uword pointervar - arg[n+1] = arg[1] - pointervar[n+1] = pointervar[1] - } + txt.lowercase() + txt.println("Hello, world1") + txt.println("Hello, world2") + txt.println("Hello, world3") } }