Skip to content

Commit

Permalink
fix syntax error check for missing return statement
Browse files Browse the repository at this point in the history
  • Loading branch information
irmen committed Sep 25, 2024
1 parent 1326498 commit 4c84357
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 12 deletions.
15 changes: 11 additions & 4 deletions compiler/src/prog8/compiler/astprocessing/AstChecker.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand Down Expand Up @@ -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)")
}
}

Expand Down
48 changes: 48 additions & 0 deletions compiler/test/ast/TestVariousCompilerAst.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
})

1 change: 0 additions & 1 deletion compilerAst/src/prog8/ast/AstToSourceTextConverter.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
2 changes: 1 addition & 1 deletion compilerAst/src/prog8/ast/Program.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
2 changes: 1 addition & 1 deletion compilerAst/src/prog8/ast/antlr/Antlr2Kotlin.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
5 changes: 2 additions & 3 deletions compilerAst/src/prog8/ast/statements/AstStatements.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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<String>): 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,
Expand Down
2 changes: 0 additions & 2 deletions docs/source/todo.rst
Original file line number Diff line number Diff line change
@@ -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:
Expand Down

0 comments on commit 4c84357

Please sign in to comment.