reused existing CallGraph to check for recursion, which is now fixed. It's a warning too now.

compiler/res/version.txt

@@ -1 +1 @@
-4.5
+4.6-SNAPSHOT

compiler/src/prog8/ast/base/Extensions.kt

@@ -42,12 +42,6 @@ internal fun Module.checkImportedValid() {
     imr.applyModifications()
 }
 
-internal fun Program.checkRecursion(errors: ErrorReporter) {
-    val checker = AstRecursionChecker(namespace, errors)
-    checker.visit(this)
-    checker.processMessages(name)
-}
-
 internal fun Program.checkIdentifiers(errors: ErrorReporter) {
 
     val checker2 = AstIdentifiersChecker(this, errors)

compiler/src/prog8/ast/processing/AstRecursionChecker.kt

@@ -1,118 +0,0 @@
-package prog8.ast.processing
-
-import prog8.ast.INameScope
-import prog8.ast.base.ErrorReporter
-import prog8.ast.base.Position
-import prog8.ast.expressions.FunctionCall
-import prog8.ast.statements.FunctionCallStatement
-import prog8.ast.statements.Subroutine
-
-
-internal class AstRecursionChecker(private val namespace: INameScope,
-                                   private val errors: ErrorReporter) : IAstVisitor {
-    private val callGraph = DirectedGraph<INameScope>()
-
-    fun processMessages(modulename: String) {
-        val cycle = callGraph.checkForCycle()
-        if(cycle.isEmpty())
-            return
-        val chain = cycle.joinToString(" <-- ") { "${it.name} at ${it.position}" }
-        errors.err("Program contains recursive subroutine calls, this is not supported. Recursive chain:\n (a subroutine call in) $chain", Position.DUMMY)
-    }
-
-    override fun visit(functionCallStatement: FunctionCallStatement) {
-        val scope = functionCallStatement.definingScope()
-        val targetStatement = functionCallStatement.target.targetStatement(namespace)
-        if(targetStatement!=null) {
-            val targetScope = when (targetStatement) {
-                is Subroutine -> targetStatement
-                else -> targetStatement.definingScope()
-            }
-            callGraph.add(scope, targetScope)
-        }
-        super.visit(functionCallStatement)
-    }
-
-    override fun visit(functionCall: FunctionCall) {
-        val scope = functionCall.definingScope()
-        val targetStatement = functionCall.target.targetStatement(namespace)
-        if(targetStatement!=null) {
-            val targetScope = when (targetStatement) {
-                is Subroutine -> targetStatement
-                else -> targetStatement.definingScope()
-            }
-            callGraph.add(scope, targetScope)
-        }
-        super.visit(functionCall)
-    }
-
-    private class DirectedGraph<VT> {
-        private val graph = mutableMapOf<VT, MutableSet<VT>>()
-        private var uniqueVertices = mutableSetOf<VT>()
-        val numVertices : Int
-            get() = uniqueVertices.size
-
-        fun add(from: VT, to: VT) {
-            var targets = graph[from]
-            if(targets==null) {
-                targets = mutableSetOf()
-                graph[from] = targets
-            }
-            targets.add(to)
-            uniqueVertices.add(from)
-            uniqueVertices.add(to)
-        }
-
-        fun print() {
-            println("#vertices: $numVertices")
-            graph.forEach { (from, to) ->
-                println("$from   CALLS:")
-                to.forEach { println("   $it") }
-            }
-            val cycle = checkForCycle()
-            if(cycle.isNotEmpty()) {
-                println("CYCLIC!  $cycle")
-            }
-        }
-
-        fun checkForCycle(): MutableList<VT> {
-            val visited = uniqueVertices.associateWith { false }.toMutableMap()
-            val recStack = uniqueVertices.associateWith { false }.toMutableMap()
-            val cycle = mutableListOf<VT>()
-            for(node in uniqueVertices) {
-                if(isCyclicUntil(node, visited, recStack, cycle))
-                    return cycle
-            }
-            return mutableListOf()
-        }
-
-        private fun isCyclicUntil(node: VT,
-                                  visited: MutableMap<VT, Boolean>,
-                                  recStack: MutableMap<VT, Boolean>,
-                                  cycleNodes: MutableList<VT>): Boolean {
-
-            if(recStack[node]==true) return true
-            if(visited[node]==true) return false
-
-            // mark current node as visited and add to recursion stack
-            visited[node] = true
-            recStack[node] = true
-
-            // recurse for all neighbours
-            val neighbors = graph[node]
-            if(neighbors!=null) {
-                for (neighbour in neighbors) {
-                    if (isCyclicUntil(neighbour, visited, recStack, cycleNodes)) {
-                        cycleNodes.add(node)
-                        return true
-                    }
-                }
-            }
-
-            // pop node from recursion stack
-            recStack[node] = false
-            return false
-        }
-    }
-
-}

compiler/src/prog8/compiler/Main.kt

@@ -208,7 +208,8 @@ private fun postprocessAst(programAst: Program, errors: ErrorReporter, compilerO
     programAst.variousCleanups()
     programAst.checkValid(compilerOptions, errors)          // check if final tree is still valid
     errors.handle()
-    programAst.checkRecursion(errors)         // check if there are recursive subroutine calls
+    val callGraph = CallGraph(programAst)
+    callGraph.checkRecursiveCalls(errors)
     errors.handle()
     programAst.verifyFunctionArgTypes()
     programAst.moveMainAndStartToFirst()

compiler/src/prog8/optimizer/CallGraph.kt

@@ -1,11 +1,10 @@
 package prog8.optimizer
 
-import prog8.ast.INameScope
-import prog8.ast.Module
-import prog8.ast.Node
-import prog8.ast.Program
+import prog8.ast.*
 import prog8.ast.base.DataType
+import prog8.ast.base.ErrorReporter
 import prog8.ast.base.ParentSentinel
+import prog8.ast.base.Position
 import prog8.ast.expressions.FunctionCall
 import prog8.ast.expressions.IdentifierReference
 import prog8.ast.processing.IAstVisitor
@@ -25,7 +24,7 @@ class CallGraph(private val program: Program) : IAstVisitor {
 
     val imports = mutableMapOf<Module, List<Module>>().withDefault { mutableListOf() }
     val importedBy = mutableMapOf<Module, List<Module>>().withDefault { mutableListOf() }
-    val calls = mutableMapOf<INameScope, List<Subroutine>>().withDefault { mutableListOf() }
+    val calls = mutableMapOf<Subroutine, List<Subroutine>>().withDefault { mutableListOf() }
     val calledBy = mutableMapOf<Subroutine, List<Node>>().withDefault { mutableListOf() }
 
     // TODO  add dataflow graph: what statements use what variables - can be used to eliminate unused vars
@@ -79,8 +78,10 @@ class CallGraph(private val program: Program) : IAstVisitor {
             importedBy[importedModule] = importedBy.getValue(importedModule).plus(thisModule)
         } else if (directive.directive == "%asminclude") {
             val asm = loadAsmIncludeFile(directive.args[0].str!!, thisModule.source)
-            val scope = directive.definingScope()
-            scanAssemblyCode(asm, directive, scope)
+            val scope = directive.definingSubroutine()
+            if(scope!=null) {
+                scanAssemblyCode(asm, directive, scope)
+            }
         }
 
         super.visit(directive)
@@ -167,12 +168,12 @@ class CallGraph(private val program: Program) : IAstVisitor {
 
     override fun visit(inlineAssembly: InlineAssembly) {
         // parse inline asm for subroutine calls (jmp, jsr)
-        val scope = inlineAssembly.definingScope()
+        val scope = inlineAssembly.definingSubroutine()!!
         scanAssemblyCode(inlineAssembly.assembly, inlineAssembly, scope)
         super.visit(inlineAssembly)
     }
 
-    private fun scanAssemblyCode(asm: String, context: Statement, scope: INameScope) {
+    private fun scanAssemblyCode(asm: String, context: Statement, scope: Subroutine) {
         asm.lines().forEach { line ->
             val matches = asmJumpRx.matchEntire(line)
             if (matches != null) {
@@ -208,4 +209,55 @@ class CallGraph(private val program: Program) : IAstVisitor {
             }
         }
     }
+
+    fun checkRecursiveCalls(errors: ErrorReporter) {
+        val cycles = recursionCycles()
+        if(cycles.any()) {
+            errors.warn("Program contains recursive subroutine calls. These only works in very specific limited scenarios!", Position.DUMMY)
+            val printed = mutableSetOf<Subroutine>()
+            for(chain in cycles) {
+                if(chain[0] !in printed) {
+                    val chainStr = chain.joinToString(" <-- ") { "${it.name} at ${it.position}" }
+                    errors.warn("Cycle in (a subroutine call in) $chainStr", Position.DUMMY)
+                    printed.add(chain[0])
+                }
+            }
+        }
+    }
+
+    private fun recursionCycles(): List<List<Subroutine>> {
+        val chains = mutableListOf<MutableList<Subroutine>>()
+        for(caller in calls.keys) {
+            val visited = calls.keys.associateWith { false }.toMutableMap()
+            val recStack = calls.keys.associateWith { false }.toMutableMap()
+            val chain = mutableListOf<Subroutine>()
+            if(hasCycle(caller, visited, recStack, chain))
+                chains.add(chain)
+        }
+        return chains
+    }
+
+    private fun hasCycle(sub: Subroutine, visited: MutableMap<Subroutine, Boolean>, recStack: MutableMap<Subroutine, Boolean>, chain: MutableList<Subroutine>): Boolean {
+        // mark current node as visited and add to recursion stack
+        if(recStack[sub]==true)
+            return true
+        if(visited[sub]==true)
+            return false
+
+        // mark visited and add to recursion stack
+        visited[sub] = true
+        recStack[sub] = true
+
+        // recurse for all neighbours
+        for(called in calls.getValue(sub)) {
+            if(hasCycle(called, visited, recStack, chain)) {
+                chain.add(called)
+                return true
+            }
+        }
+
+        // pop from recursion stack
+        recStack[sub] = false
+        return false
+    }
 }

examples/test.p8

@@ -7,7 +7,39 @@
 main {
 
     sub start() {
+        derp()
+        rec2()
+    }
 
+    sub rec2() {
+        rec3()
+    }
+
+    sub rec3() {
+        rec4()
+    }
+
+    sub rec4() {
+        rec2()
+    }
+
+    sub derp() {
+        repeat {
+            derp()
+        }
+        if true {
+            derp()
+        } else {
+            derp()
+        }
+
+        do {
+            derp()
+        } until true
+
+        while true {
+            derp()
+        }
     }
 
     asmsub testX() {

examples/textelite.p8

@@ -720,8 +720,7 @@ planet {
                         source_stack[stack_ptr] = source_ptr
                         stack_ptr++
                         source_ptr = getword(c, wordNr)
-                        ; TODO recursive call... should give error message... but hey since it's not doing that here now, lets exploit it
-                        recursive_soup()         ; RECURSIVE CALL
+                        recursive_soup()    ; RECURSIVE CALL - ignore the warning message from the compiler; we don't use local variables or parameters so we're safe in this case
                         stack_ptr--
                         source_ptr = source_stack[stack_ptr]
                     } else {