From eeeb8d81f4d289612b60863c9d6f722b0684def3 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Wed, 30 Oct 2024 00:37:45 +0100 Subject: [PATCH] merge now also allows monkeypatching if signature is 100% identical --- .../compiler/astprocessing/AstExtensions.kt | 2 +- .../compiler/astprocessing/BlockMerger.kt | 18 +++++- compiler/test/ast/TestProgram.kt | 55 +++++++++++++++++-- .../src/prog8/ast/statements/AstStatements.kt | 9 +++ docs/source/syntaxreference.rst | 4 +- docs/source/todo.rst | 2 + examples/test.p8 | 28 ++++------ 7 files changed, 92 insertions(+), 26 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index 7782033f6..00d22de3a 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -105,7 +105,7 @@ internal fun Program.verifyFunctionArgTypes(errors: IErrorReporter, options: Com } internal fun Program.preprocessAst(errors: IErrorReporter, options: CompilationOptions) { - val mergeBlocks = BlockMerger() + val mergeBlocks = BlockMerger(errors) mergeBlocks.visit(this) if(errors.noErrors()) { val transforms = AstPreprocessor(this, errors, options) diff --git a/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt b/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt index 1ab4de5bb..5757471c0 100644 --- a/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt +++ b/compiler/src/prog8/compiler/astprocessing/BlockMerger.kt @@ -3,8 +3,10 @@ package prog8.compiler.astprocessing import prog8.ast.Program import prog8.ast.statements.Block import prog8.ast.statements.Directive +import prog8.ast.statements.Subroutine +import prog8.code.core.IErrorReporter -class BlockMerger { +class BlockMerger(val errors: IErrorReporter) { // 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) @@ -29,9 +31,21 @@ class BlockMerger { } private fun merge(block: Block, target: Block) { + val named = target.statements.filterIsInstance().associateBy { it.name } + for(stmt in block.statements.filter { it !is Directive }) { + if(stmt is Subroutine && stmt.name in named) { + val existing = named.getValue(stmt.name) + if(stmt.returntypes==existing.returntypes) { + if(stmt.parameters == existing.parameters) { + // overwrite the target subroutine, everything is identical! + existing.definingScope.remove(existing) + errors.info("monkeypatched subroutine '${existing.scopedName.joinToString(".")}' in ${existing.definingModule.name}", stmt.position) + } + } + } target.statements.add(stmt) - stmt.linkParents(target) + stmt.parent = target } block.statements.clear() block.definingScope.remove(block) diff --git a/compiler/test/ast/TestProgram.kt b/compiler/test/ast/TestProgram.kt index 1d07605a0..5723f2cb4 100644 --- a/compiler/test/ast/TestProgram.kt +++ b/compiler/test/ast/TestProgram.kt @@ -14,10 +14,8 @@ 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 +import prog8.code.target.VMTarget +import prog8tests.helpers.* class TestProgram: FunSpec({ @@ -147,5 +145,54 @@ blah { }""" compileText(C64Target(), optimize=false, src, writeAssembly=false) shouldNotBe null } + + test("merge override existing subroutine") { + val src=""" +%import textio + +main { + + sub start() { + txt.print("sdfdsf") + } +} + +txt { + %option merge + + sub print(str text) { + cx16.r0++ + ; just some dummy implementation to replace existing print + } +}""" + + val result = compileText(VMTarget(), optimize=false, src, writeAssembly=false) + result shouldNotBe null + } + + test("merge doesn't override existing subroutine if signature differs") { + val src=""" +%import textio + +main { + + sub start() { + txt.print("sdfdsf") + } +} + +txt { + %option merge + + sub print(str anotherparamname) { + cx16.r0++ + ; just some dummy implementation to replace existing print + } +}""" + val errors = ErrorReporterForTests() + compileText(VMTarget(), optimize=false, src, writeAssembly=false, errors = errors) shouldBe null + errors.errors.size shouldBe 1 + errors.errors[0] shouldContain "name conflict" + } } }) diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index b377f666f..fe3c5af06 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -6,6 +6,7 @@ import prog8.ast.expressions.* import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstVisitor import prog8.code.core.* +import java.util.* interface INamedStatement { @@ -861,6 +862,14 @@ open class SubroutineParameter(val name: String, override fun copy() = SubroutineParameter(name, type, zp, position) override fun toString() = "Param($type:$name)" override fun referencesIdentifier(nameInSource: List): Boolean = nameInSource.size==1 && name==nameInSource[0] + + override fun equals(other: Any?): Boolean { + if (this === other) return true + if (other !is SubroutineParameter) return false + return name == other.name && type == other.type && zp == other.zp + } + + override fun hashCode(): Int = Objects.hash(name, type, zp) } class IfElse(var condition: Expression, diff --git a/docs/source/syntaxreference.rst b/docs/source/syntaxreference.rst index 9717db8d7..b8c6650e5 100644 --- a/docs/source/syntaxreference.rst +++ b/docs/source/syntaxreference.rst @@ -199,7 +199,9 @@ 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. Can be used to add subroutines to an existing library block, for instance. + - ``merge`` (in a block) will merge this block's contents into an already existing block with the same name. + Can be used to add or override subroutines to an existing library block, for instance. + Overriding (monkeypatching) happens only if the signature of the subroutine exactly matches the original subroutine, including the exact names and types of the parameters. - ``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 12b3f1002..1f426691d 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,6 +1,8 @@ TODO ==== +Allow %merge to overwrite existing subs if the signature is identical? + 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 in overwriting registers that already got their value, which requires a lot of stack juggling (especially on plain 6502 cpu!) diff --git a/examples/test.p8 b/examples/test.p8 index e640a8c2d..97d0c3e98 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -5,7 +5,7 @@ main { sub start() { - blah.test() + txt.print("sdfdsf") } } @@ -13,22 +13,14 @@ 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) + sub print(str text) { + repeat 4 chrout('@') + repeat { + cx16.r0L = @(text) + if_z break + chrout(cx16.r0L) + text++ + } + repeat 4 chrout('@') } }