From 4c843571ea121a0cfffbf38c6cf4b4263ea6dae1 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Thu, 26 Sep 2024 01:52:33 +0200 Subject: [PATCH] fix syntax error check for missing return statement --- .../compiler/astprocessing/AstChecker.kt | 15 ++++-- compiler/test/ast/TestVariousCompilerAst.kt | 48 +++++++++++++++++++ .../src/prog8/ast/AstToSourceTextConverter.kt | 1 - compilerAst/src/prog8/ast/Program.kt | 2 +- .../src/prog8/ast/antlr/Antlr2Kotlin.kt | 2 +- .../src/prog8/ast/statements/AstStatements.kt | 5 +- docs/source/todo.rst | 2 - 7 files changed, 63 insertions(+), 12 deletions(-) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 7c617945c..acd232596 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -322,7 +322,7 @@ internal class AstChecker(private val program: Program, checkLongType(numLiteral) } - private fun hasReturnOrJumpOrRts(scope: IStatementContainer): Boolean { + private fun hasReturnOrExternalJumpOrRts(scope: IStatementContainer): Boolean { class Searcher: IAstVisitor { var count=0 @@ -331,7 +331,14 @@ internal class AstChecker(private val program: Program, count++ } override fun visit(jump: Jump) { - count++ + val jumpTarget = jump.identifier?.targetStatement(program) + if(jumpTarget!=null) { + val sub = jump.definingSubroutine + val targetSub = if(jumpTarget is Subroutine) jumpTarget else jumpTarget.definingSubroutine + if(sub !== targetSub) + count++ + } + else count++ } override fun visit(inlineAssembly: InlineAssembly) { @@ -374,11 +381,11 @@ internal class AstChecker(private val program: Program, // subroutine must contain at least one 'return' or 'goto' // (or if it has an asm block, that must contain a 'rts' or 'jmp' or 'bra') - if(!hasReturnOrJumpOrRts(subroutine)) { + if(!hasReturnOrExternalJumpOrRts(subroutine)) { if (subroutine.returntypes.isNotEmpty()) { // for asm subroutines with an address, no statement check is possible. if (subroutine.asmAddress == null && !subroutine.inline) - err("non-inline subroutine has result value(s) and thus must have at least one 'return' or 'goto' in it (or the assembler equivalent in case of %asm)") + err("non-inline subroutine has result value(s) and thus must have at least one 'return' or external 'goto' in it (or the assembler equivalent in case of %asm)") } } diff --git a/compiler/test/ast/TestVariousCompilerAst.kt b/compiler/test/ast/TestVariousCompilerAst.kt index 94d791cb3..1d9f97225 100644 --- a/compiler/test/ast/TestVariousCompilerAst.kt +++ b/compiler/test/ast/TestVariousCompilerAst.kt @@ -638,5 +638,53 @@ main { errors.errors[1] shouldEndWith "cannot assign to 'void', perhaps a void function call was intended" errors.errors[2] shouldEndWith "cannot assign to 'void', perhaps a void function call was intended" } + + test("missing return value is a syntax error") { + val src=""" +main { + sub start() { + cx16.r0 = runit1() + cx16.r1 = runit2() + } + + sub runit1() -> uword { + repeat { + cx16.r0++ + goto runit1 + } + } + + sub runit2() -> uword { + cx16.r0++ + } +}""" + val errors = ErrorReporterForTests() + compileText(C64Target(), optimize=false, src, writeAssembly=false, errors = errors) shouldBe null + errors.errors.size shouldBe 2 + errors.errors[0] shouldContain "has result value" + errors.errors[1] shouldContain "has result value" + } + + test("missing return value is not a syntax error if there's an external goto") { + val src=""" +main { + sub start() { + cx16.r0 = runit1() + runit2() + } + + sub runit1() -> uword { + repeat { + cx16.r0++ + goto runit2 + } + } + + sub runit2() { + cx16.r0++ + } +}""" + compileText(C64Target(), optimize=false, src, writeAssembly=false) shouldNotBe null + } }) diff --git a/compilerAst/src/prog8/ast/AstToSourceTextConverter.kt b/compilerAst/src/prog8/ast/AstToSourceTextConverter.kt index 265cb3ead..03c64a586 100644 --- a/compilerAst/src/prog8/ast/AstToSourceTextConverter.kt +++ b/compilerAst/src/prog8/ast/AstToSourceTextConverter.kt @@ -254,7 +254,6 @@ class AstToSourceTextConverter(val output: (text: String) -> Unit, val program: output("goto ") when { jump.address!=null -> output(jump.address!!.toHex()) - jump.generatedLabel!=null -> output(jump.generatedLabel) jump.identifier!=null -> jump.identifier.accept(this) } } diff --git a/compilerAst/src/prog8/ast/Program.kt b/compilerAst/src/prog8/ast/Program.kt index d470aea33..66e7c1b04 100644 --- a/compilerAst/src/prog8/ast/Program.kt +++ b/compilerAst/src/prog8/ast/Program.kt @@ -158,6 +158,6 @@ class Program(val name: String, fun jumpLabel(label: Label): Jump { val ident = IdentifierReference(listOf(label.name), label.position) - return Jump(null, ident, null, label.position) + return Jump(null, ident, label.position) } } diff --git a/compilerAst/src/prog8/ast/antlr/Antlr2Kotlin.kt b/compilerAst/src/prog8/ast/antlr/Antlr2Kotlin.kt index 65d8febe1..1081e7fd7 100644 --- a/compilerAst/src/prog8/ast/antlr/Antlr2Kotlin.kt +++ b/compilerAst/src/prog8/ast/antlr/Antlr2Kotlin.kt @@ -284,7 +284,7 @@ private fun ReturnstmtContext.toAst() : Return { private fun UnconditionaljumpContext.toAst(): Jump { val address = integerliteral()?.toAst()?.number?.toUInt() val identifier = scoped_identifier()?.toAst() - return Jump(address, identifier, null, toPosition()) + return Jump(address, identifier, toPosition()) } private fun LabeldefContext.toAst(): Statement = diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt index 390d0aa9b..85c0b6920 100644 --- a/compilerAst/src/prog8/ast/statements/AstStatements.kt +++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt @@ -642,7 +642,6 @@ data class AssignTarget(var identifier: IdentifierReference?, class Jump(var address: UInt?, val identifier: IdentifierReference?, - val generatedLabel: String?, // can be used in code generation scenarios override val position: Position) : Statement() { override lateinit var parent: Node @@ -658,13 +657,13 @@ class Jump(var address: UInt?, else throw FatalAstException("can't replace $node") } - override fun copy() = Jump(address, identifier?.copy(), generatedLabel, position) + override fun copy() = Jump(address, identifier?.copy(), position) override fun accept(visitor: IAstVisitor) = visitor.visit(this) override fun accept(visitor: AstWalker, parent: Node) = visitor.visit(this, parent) override fun referencesIdentifier(nameInSource: List): Boolean = identifier?.referencesIdentifier(nameInSource)==true override fun toString() = - "Jump(addr: $address, identifier: $identifier, label: $generatedLabel; pos=$position)" + "Jump(addr: $address, identifier: $identifier, pos=$position)" } class FunctionCallStatement(override var target: IdentifierReference, diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 07f608b63..ac5824616 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,8 +1,6 @@ TODO ==== -Textelite main.start() -> no error when a return value is added but no return statement - Can we move the asm init code that is injected into the start() subroutine, to init_system_phase2 instead? Doc improvements: some short overview for people coming from other programming languages like C: