From 69b9dfa46890f771a0909dbcdd67f180976f2b46 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 1 Feb 2022 23:09:52 +0100 Subject: [PATCH] fix invalid recursion warning for code referencing subroutine but not via a call --- compiler/test/TestCallgraph.kt | 62 ++++++++++++++++--- .../src/prog8/compilerinterface/CallGraph.kt | 11 +--- docs/source/todo.rst | 2 +- examples/test.p8 | 32 ++-------- 4 files changed, 63 insertions(+), 44 deletions(-) diff --git a/compiler/test/TestCallgraph.kt b/compiler/test/TestCallgraph.kt index aa4789590..f6c8ba393 100644 --- a/compiler/test/TestCallgraph.kt +++ b/compiler/test/TestCallgraph.kt @@ -3,9 +3,9 @@ package prog8tests import io.kotest.assertions.withClue import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.ints.shouldBeGreaterThanOrEqual -import io.kotest.matchers.maps.shouldContainKey import io.kotest.matchers.maps.shouldNotContainKey import io.kotest.matchers.shouldBe +import io.kotest.matchers.string.shouldContain import prog8.ast.Program import prog8.ast.statements.Block import prog8.ast.statements.Subroutine @@ -55,7 +55,7 @@ class TestCallgraph: FunSpec({ } } - test("testGraphForEmptyButReferencedSub") { + test("reference to empty sub") { val sourcecode = """ %import string main { @@ -85,15 +85,11 @@ class TestCallgraph: FunSpec({ val startSub = mainBlock.statements.filterIsInstance().single{it.name=="start"} val emptySub = mainBlock.statements.filterIsInstance().single{it.name=="empty"} - withClue("start 'calls' (references) empty") { - graph.calls shouldContainKey startSub - } + graph.calls shouldNotContainKey startSub + graph.calledBy shouldNotContainKey emptySub withClue("empty doesn't call anything") { graph.calls shouldNotContainKey emptySub } - withClue("empty gets 'called'") { - graph.calledBy shouldContainKey emptySub - } withClue( "start doesn't get called (except as entrypoint ofc.)") { graph.calledBy shouldNotContainKey startSub } @@ -179,4 +175,54 @@ class TestCallgraph: FunSpec({ callgraph.unused(subRout) shouldBe true callgraph.unused(subOrrectlab) shouldBe true } + + test("recursion detection") { + val source=""" + main { + sub start() { + recurse1() + } + sub recurse1() { + recurse2() + } + sub recurse2() { + start() + } + }""" + val module = parseModule(SourceCode.Text(source)) + val program = Program("test", DummyFunctions, DummyMemsizer, DummyStringEncoder) + program.addModule(module) + val callgraph = CallGraph(program) + val errors = ErrorReporterForTests() + callgraph.checkRecursiveCalls(errors) + errors.errors.size shouldBe 0 + errors.warnings.size shouldBe 4 + errors.warnings[0] shouldContain "contains recursive subroutine calls" + errors.warnings[1] shouldContain "start at" + errors.warnings[2] shouldContain "recurse1 at" + errors.warnings[3] shouldContain "recurse2 at" + } + + test("no recursion warning if reference isn't a call") { + val source=""" + main { + sub start() { + recurse1() + } + sub recurse1() { + recurse2() + } + sub recurse2() { + uword @shared address = &start + } + }""" + val module = parseModule(SourceCode.Text(source)) + val program = Program("test", DummyFunctions, DummyMemsizer, DummyStringEncoder) + program.addModule(module) + val callgraph = CallGraph(program) + val errors = ErrorReporterForTests() + callgraph.checkRecursiveCalls(errors) + errors.errors.size shouldBe 0 + errors.warnings.size shouldBe 0 + } }) diff --git a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt index 2469fb161..9cc31be28 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt +++ b/compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt @@ -15,6 +15,7 @@ class CallGraph(private val program: Program) : IAstVisitor { val importedBy = mutableMapOf>().withDefault { setOf() } val calls = mutableMapOf>().withDefault { setOf() } val calledBy = mutableMapOf>().withDefault { setOf() } + val notCalledButReferenced = mutableSetOf() private val allIdentifiersAndTargets = mutableMapOf() private val allAssemblyNodes = mutableListOf() @@ -25,7 +26,7 @@ class CallGraph(private val program: Program) : IAstVisitor { val allIdentifiers: Map = allIdentifiersAndTargets private val usedSubroutines: Set by lazy { - calledBy.keys + program.entrypoint + calledBy.keys + program.entrypoint + notCalledButReferenced } private val usedBlocks: Set by lazy { @@ -81,13 +82,7 @@ class CallGraph(private val program: Program) : IAstVisitor { } override fun visit(addressOf: AddressOf) { - val otherSub = addressOf.identifier.targetSubroutine(program) - if(otherSub!=null) { - addressOf.definingSubroutine?.let { thisSub -> - calls[thisSub] = calls.getValue(thisSub) + otherSub - calledBy[otherSub] = calledBy.getValue(otherSub) + thisSub - } - } + addressOf.identifier.targetSubroutine(program)?.let { notCalledButReferenced.add(it) } super.visit(addressOf) } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 6346102a5..dbb1ebb80 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -3,7 +3,7 @@ TODO For next release ^^^^^^^^^^^^^^^^ -- Fix: don't report as recursion if code assigns address of its own subroutine to something, rather than calling it +... Need help with ^^^^^^^^^^^^^^ diff --git a/examples/test.p8 b/examples/test.p8 index 81feae368..5f4e49871 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,33 +1,11 @@ main { sub start() { - %asm {{ - lda #