fix invalid recursion warning for code referencing subroutine but not via a call

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<Subroutine>().single{it.name=="start"}
         val emptySub = mainBlock.statements.filterIsInstance<Subroutine>().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
+    }
 })

compilerInterfaces/src/prog8/compilerinterface/CallGraph.kt

@@ -15,6 +15,7 @@ class CallGraph(private val program: Program) : IAstVisitor {
     val importedBy = mutableMapOf<Module, Set<Module>>().withDefault { setOf() }
     val calls = mutableMapOf<Subroutine, Set<Subroutine>>().withDefault { setOf() }
     val calledBy = mutableMapOf<Subroutine, Set<Node>>().withDefault { setOf() }
+    val notCalledButReferenced = mutableSetOf<Subroutine>()
     private val allIdentifiersAndTargets = mutableMapOf<IdentifierReference, Statement>()
     private val allAssemblyNodes = mutableListOf<InlineAssembly>()
 
@@ -25,7 +26,7 @@ class CallGraph(private val program: Program) : IAstVisitor {
     val allIdentifiers: Map<IdentifierReference, Statement> = allIdentifiersAndTargets
 
     private val usedSubroutines: Set<Subroutine> by lazy {
-        calledBy.keys + program.entrypoint
+        calledBy.keys + program.entrypoint + notCalledButReferenced
     }
 
     private val usedBlocks: Set<Block> 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)
     }
 

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
 ^^^^^^^^^^^^^^

examples/test.p8

@@ -1,33 +1,11 @@
 main {
     sub start() {
-        %asm {{
-            lda  #<blockname
-            lda  #<blockname.subroutine
-correctlabel:
-            nop     ; rout orrectlab locknam
-        }}
+        recurse1()
     }
-
-}
-
-blockname {
-    sub subroutine() {
-        @($c000) = 0
+    sub recurse1() {
+        recurse2()
     }
-
-    sub correctlabel() {
-        @($c000) = 0
+    sub recurse2() {
+        uword @shared address = &start
     }
 }
-
-; all block and subroutines below should NOT be found in asm because they're only substrings of the names in there, or in a comment
-locknam {
-    sub rout() {
-        @($c000) = 0
-    }
-
-    sub orrectlab() {
-        @($c000) = 0
-    }
-}
-