From bbba4b3d60432d5b77b26952d2df108992810739 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 29 Oct 2024 22:57:54 +0100 Subject: [PATCH] new block merge semantics and implementation --- compiler/src/prog8/compiler/ModuleImporter.kt | 27 +------- .../compiler/astprocessing/AstExtensions.kt | 10 ++- .../astprocessing/AstIdentifiersChecker.kt | 11 ++- .../compiler/astprocessing/BlockMerger.kt | 39 +++++++++++ .../astprocessing/StatementReorderer.kt | 3 +- compiler/test/ast/TestProgram.kt | 42 +++++++++++ compilerAst/src/prog8/ast/AstToplevel.kt | 4 +- .../src/prog8/ast/statements/AstStatements.kt | 6 +- docs/source/syntaxreference.rst | 2 +- docs/source/todo.rst | 3 +- examples/test.p8 | 69 +++++++------------ 11 files changed, 130 insertions(+), 86 deletions(-) create mode 100644 compiler/src/prog8/compiler/astprocessing/BlockMerger.kt diff --git a/compiler/src/prog8/compiler/ModuleImporter.kt b/compiler/src/prog8/compiler/ModuleImporter.kt index aaf7119a8..da37a3c74 100644 --- a/compiler/src/prog8/compiler/ModuleImporter.kt +++ b/compiler/src/prog8/compiler/ModuleImporter.kt @@ -1,6 +1,7 @@ package prog8.compiler import com.github.michaelbull.result.* +import prog8.ast.IStatementContainer import prog8.ast.Module import prog8.ast.Program import prog8.ast.base.SyntaxError @@ -60,7 +61,8 @@ class ModuleImporter(private val program: Program, .mapIndexed { i, it -> i to it } .filter { (it.second as? Directive)?.directive == "%import" } .forEach { executeImportDirective(it.second as Directive, moduleAst) } - moduleAst.statements = lines + moduleAst.statements.clear() + moduleAst.statements.addAll(lines) return moduleAst } catch (x: Exception) { // in case of error, make sure the module we're importing is no longer in the Ast @@ -105,29 +107,6 @@ class ModuleImporter(private val program: Program, ) removeDirectivesFromImportedModule(importedModule) - - // modules can contain blocks with "merge" option. - // their content has to be merged into already existing other block with the same name. - val blocks = importedModule.statements.filterIsInstance() - for(block in blocks) { - 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) - - if(blockHasMergeOption && !existingBlockHasMergeOption) { - val directive = Directive("%option", listOf(DirectiveArg(null, "merge", null, block.position)), block.position) - existingBlock.statements.add(0, directive) - directive.linkParents(existingBlock) - } - } - } - } - return importedModule } diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index f45770688..7782033f6 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -105,10 +105,14 @@ internal fun Program.verifyFunctionArgTypes(errors: IErrorReporter, options: Com } internal fun Program.preprocessAst(errors: IErrorReporter, options: CompilationOptions) { - val transforms = AstPreprocessor(this, errors, options) - transforms.visit(this) - while(errors.noErrors() && transforms.applyModifications()>0) + val mergeBlocks = BlockMerger() + mergeBlocks.visit(this) + if(errors.noErrors()) { + val transforms = AstPreprocessor(this, errors, options) transforms.visit(this) + while (errors.noErrors() && transforms.applyModifications() > 0) + transforms.visit(this) + } } internal fun Program.checkIdentifiers(errors: IErrorReporter, options: CompilationOptions) { diff --git a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt index 80f59f6cd..d86ce4511 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstIdentifiersChecker.kt @@ -44,13 +44,10 @@ internal class AstIdentifiersChecker(private val errors: IErrorReporter, override fun visit(block: Block) { val existing = blocks[block.name] if(existing!=null) { - // 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) - } + 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/src/prog8/compiler/astprocessing/BlockMerger.kt b/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt new file mode 100644 index 000000000..1ab4de5bb --- /dev/null +++ b/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt @@ -0,0 +1,39 @@ +package prog8.compiler.astprocessing + +import prog8.ast.Program +import prog8.ast.statements.Block +import prog8.ast.statements.Directive + +class BlockMerger { + // All blocks having a 'merge' option, + // will be joined into a block with the same name, coming from a library. + // (or a normal block if no library block with that name was found) + + fun visit(program: Program) { + val allBlocks = program.allBlocks + for(block in allBlocks) { + if("merge" in block.options()) { + // move everything into the first other block with the same name + // and remove the source block altogether + val libraryBlock = allBlocks.firstOrNull { it !== block && it.isInLibrary && it.name==block.name } + if(libraryBlock!=null) { + merge(block, libraryBlock) + } else { + val regularBlock = allBlocks.firstOrNull { it !== block && it.name==block.name } + if(regularBlock!=null) { + merge(block, regularBlock) + } + } + } + } + } + + private fun merge(block: Block, target: Block) { + for(stmt in block.statements.filter { it !is Directive }) { + target.statements.add(stmt) + stmt.linkParents(target) + } + block.statements.clear() + block.definingScope.remove(block) + } +} diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index 8129e300b..c5c3fe032 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -26,7 +26,8 @@ internal class StatementReorderer( override fun after(module: Module, parent: Node): Iterable { val (blocks, other) = module.statements.partition { it is Block } - module.statements = other.asSequence().plus(blocks.sortedBy { (it as Block).address ?: UInt.MAX_VALUE }).toMutableList() + module.statements.clear() + module.statements.addAll(other.asSequence().plus(blocks.sortedBy { (it as Block).address ?: UInt.MAX_VALUE })) val mainBlock = module.statements.asSequence().filterIsInstance().firstOrNull { it.name=="main" } if(mainBlock!=null && mainBlock.address==null) { diff --git a/compiler/test/ast/TestProgram.kt b/compiler/test/ast/TestProgram.kt index 8c28f9b1d..1d07605a0 100644 --- a/compiler/test/ast/TestProgram.kt +++ b/compiler/test/ast/TestProgram.kt @@ -5,6 +5,7 @@ import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.collections.shouldBeIn import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain import io.kotest.matchers.types.shouldBeSameInstanceAs import prog8.ast.Module @@ -12,9 +13,11 @@ import prog8.ast.Program import prog8.code.core.Position import prog8.code.core.SourceCode import prog8.code.core.internedStringsModuleName +import prog8.code.target.C64Target import prog8tests.helpers.DummyFunctions import prog8tests.helpers.DummyMemsizer import prog8tests.helpers.DummyStringEncoder +import prog8tests.helpers.compileText class TestProgram: FunSpec({ @@ -106,4 +109,43 @@ class TestProgram: FunSpec({ ms2 shouldBeSameInstanceAs ms1 } } + + context("block merge") { + test("merge works") { + val src = """ +%import textio + +main { + + sub start() { + blah.test() + } +} + +txt { + ; merges this block into the txt block coming from the textio library + %option merge + + sub schrijf(str arg) { + print(arg) + } +} + +blah { + ; merges this block into the other 'blah' one + %option merge + + sub test() { + printit("test merge") + } +} + +blah { + sub printit(str arg) { + txt.schrijf(arg) + } +}""" + compileText(C64Target(), optimize=false, src, writeAssembly=false) shouldNotBe null + } + } }) diff --git a/compilerAst/src/prog8/ast/AstToplevel.kt b/compilerAst/src/prog8/ast/AstToplevel.kt index 6a2fb9cfd..07f6f7644 100644 --- a/compilerAst/src/prog8/ast/AstToplevel.kt +++ b/compilerAst/src/prog8/ast/AstToplevel.kt @@ -282,7 +282,7 @@ inline fun findParentNode(node: Node): T? { } -open class Module(final override var statements: MutableList, +open class Module(final override val statements: MutableList, final override val position: Position, val source: SourceCode ) : Node, INameScope { @@ -364,7 +364,7 @@ class GlobalNamespace(val modules: MutableList): Node, INameScope { internal object BuiltinFunctionScopePlaceholder : INameScope { override val name = "<>" - override var statements = mutableListOf() + override val statements = mutableListOf() } diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 67f3a5b42..f15d8bf42 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -68,7 +68,7 @@ class BuiltinFunctionPlaceholder(override val name: String, override val positio class Block(override val name: String, val address: UInt?, - override var statements: MutableList, + override val statements: MutableList, val isInLibrary: Boolean, override val position: Position) : Statement(), INameScope { override lateinit var parent: Node @@ -767,7 +767,7 @@ class Defer(val scope: AnonymousScope, override val position: Position): Stateme override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) } -class AnonymousScope(override var statements: MutableList, +class AnonymousScope(override val statements: MutableList, override val position: Position) : IStatementContainer, Statement() { override lateinit var parent: Node @@ -802,7 +802,7 @@ class Subroutine(override val name: String, val isAsmSubroutine: Boolean, var inline: Boolean, var hasBeenInlined: Boolean=false, - override var statements: MutableList, + override val statements: MutableList, override val position: Position) : Statement(), INameScope { override lateinit var parent: Node diff --git a/docs/source/syntaxreference.rst b/docs/source/syntaxreference.rst index 48b129ced..9717db8d7 100644 --- a/docs/source/syntaxreference.rst +++ b/docs/source/syntaxreference.rst @@ -199,7 +199,7 @@ Directives program was launched. - ``force_output`` (in a block) will force the block to be outputted in the final program. Can be useful to make sure some data is generated that would otherwise be discarded because the compiler thinks it's not referenced (such as sprite data) - - ``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. + - ``merge`` (in a block) will merge this block's contents into an already existing block with the same name. Can be used to add subroutines to an existing library block, for instance. - ``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 or module) 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 25266e70c..e2cf79b7e 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,8 +1,7 @@ TODO ==== -- writing a 'txt' block in user program suddenly spews out all textio unused reference warnings because of the %option merge promotion going on. Merging should probably be handled differently - +Block node is not a Statement Improve register load order in subroutine call args assignments: in certain situations, the "wrong" order of evaluation of function call arguments is done which results diff --git a/examples/test.p8 b/examples/test.p8 index 76690711a..e640a8c2d 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,51 +1,34 @@ %import textio -%import diskio %option no_sysinit %zeropage basicsafe main { + sub start() { - txt.lowercase() - - uword buffer = memory("buffer", 16000, 0) - uword line = 0 - - if diskio.f_open("gradlew.bat") { - uword size = diskio.f_read(buffer, 1000) - txt.print_uw(size) - txt.nl() - diskio.f_close() - } - - if diskio.f_open("gradlew.bat") { - size = diskio.f_read_all(buffer) - txt.print_uw(size) - txt.nl() - - diskio.f_close() - } - - if diskio.f_open("gradlew.bat") { - ubyte linesize, status - repeat { - linesize = diskio.f_readline(buffer) - if_cs { - line++ - } else break - } - diskio.f_close() - - txt.print_uw(line) - txt.nl() - } - - if diskio.f_open_w("result.txt") { - void diskio.f_write("line 1\n", 7) - void diskio.f_write("line 2\n", 7) - void diskio.f_write("line 3\n", 7) - diskio.f_close_w() - } + blah.test() + } +} + +txt { + ; merges this block into the txt block coming from the textio library + %option merge + + sub schrijf(str arg) { + print(arg) + } +} + +blah { + ; merges this block into the other 'blah' one + %option merge + + sub test() { + printit("test merge") + } +} + +blah { + sub printit(str arg) { + txt.schrijf(arg) } - - }