new block merge semantics and implementation

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<Block>()
-        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
     }
 

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) {

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

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)
+    }
+}

compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt

@@ -26,7 +26,8 @@ internal class StatementReorderer(
 
     override fun after(module: Module, parent: Node): Iterable<IAstModification> {
         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<Block>().firstOrNull { it.name=="main" }
         if(mainBlock!=null && mainBlock.address==null) {

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
+        }
+    }
 })

compilerAst/src/prog8/ast/AstToplevel.kt

@@ -282,7 +282,7 @@ inline fun <reified T> findParentNode(node: Node): T? {
 }
 
 
-open class Module(final override var statements: MutableList<Statement>,
+open class Module(final override val statements: MutableList<Statement>,
                   final override val position: Position,
                   val source: SourceCode
 ) : Node, INameScope {
@@ -364,7 +364,7 @@ class GlobalNamespace(val modules: MutableList<Module>): Node, INameScope {
 
 internal object BuiltinFunctionScopePlaceholder : INameScope {
     override val name = "<<builtin-functions-scope-placeholder>>"
-    override var statements = mutableListOf<Statement>()
+    override val statements = mutableListOf<Statement>()
 }
 
 

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<Statement>,
+            override val statements: MutableList<Statement>,
             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<Statement>,
+class AnonymousScope(override val statements: MutableList<Statement>,
                      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<Statement>,
+                 override val statements: MutableList<Statement>,
                  override val position: Position) : Statement(), INameScope {
 
     override lateinit var parent: Node

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.

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

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)
     }
-
-
 }