fix unused subroutine removal not removing all unused subs

codeOptimizers/src/prog8/optimizer/UnusedCodeRemover.kt

@@ -20,7 +20,12 @@ class UnusedCodeRemover(private val program: Program,
                         private val compTarget: ICompilationTarget
 ): AstWalker() {
 
-    private val callgraph = CallGraph(program)
+    private lateinit var callgraph: CallGraph
+
+    override fun before(program: Program): Iterable<IAstModification> {
+        callgraph = CallGraph(program)
+        return noModifications
+    }
 
     override fun before(module: Module, parent: Node): Iterable<IAstModification> {
         return if (!module.isLibrary && (module.containsNoCodeNorVars || callgraph.unused(module)))

compiler/src/prog8/compiler/Compiler.kt

@@ -419,9 +419,15 @@ private fun processAst(program: Program, errors: IErrorReporter, compilerOptions
 }
 
 private fun optimizeAst(program: Program, compilerOptions: CompilationOptions, errors: IErrorReporter, functions: IBuiltinFunctions) {
-    val remover = UnusedCodeRemover(program, errors, compilerOptions.compTarget)
-    remover.visit(program)
-    remover.applyModifications()
+    fun removeUnusedCode(program: Program, errors: IErrorReporter, compilerOptions: CompilationOptions) {
+        val remover = UnusedCodeRemover(program, errors, compilerOptions.compTarget)
+        remover.visit(program)
+        while (errors.noErrors() && remover.applyModifications() > 0) {
+            remover.visit(program)
+        }
+    }
+
+    removeUnusedCode(program, errors,compilerOptions)
     while (true) {
         // keep optimizing expressions and statements until no more steps remain
         val optsDone1 = program.simplifyExpressions(errors, compilerOptions.compTarget)
@@ -435,9 +441,7 @@ private fun optimizeAst(program: Program, compilerOptions: CompilationOptions, e
         if (optsDone1 + optsDone2 + optsDone3 == 0)
             break
     }
-    val remover2 = UnusedCodeRemover(program, errors, compilerOptions.compTarget)
-    remover2.visit(program)
-    remover2.applyModifications()
+    removeUnusedCode(program, errors, compilerOptions)
     if(errors.noErrors())
         program.constantFold(errors, compilerOptions) // because simplified statements and expressions can result in more constants that can be folded away
     errors.report()

compiler/test/TestOptimization.kt

@@ -239,6 +239,37 @@ class TestOptimization: FunSpec({
         (func2.statements[0] as Assignment).target.identifier!!.nameInSource shouldBe listOf("cx16", "r0")
     }
 
+    test("unused subroutine removal") {
+        val src="""
+main {
+    sub strip() {
+        lstrip()
+    }
+
+    sub lstrip() {
+        lstripped()
+    }
+
+    sub lstripped() {
+        cx16.r0++
+        evenmore()
+        cx16.r1++
+    }
+
+    sub evenmore() {
+        cx16.r1++
+        cx16.r2++
+    }
+
+    sub start() {
+        ; nothing
+    }
+}"""
+        val result = compileText(C64Target(), optimize=true, src, writeAssembly=false)!!
+        result.compilerAst.entrypoint.statements.size shouldBe 0
+        result.compilerAst.entrypoint.definingScope.statements.size shouldBe 1
+    }
+
     test("test simple augmented assignment optimization correctly initializes all variables") {
         val src="""
             main {

compilerAst/src/prog8/ast/AstToplevel.kt

@@ -330,7 +330,7 @@ open class Module(final override var statements: MutableList<Statement>,
 }
 
 
-class GlobalNamespace(val modules: Iterable<Module>): Node, INameScope {
+class GlobalNamespace(val modules: MutableList<Module>): Node, INameScope {
     override val name = "<<<global>>>"
     override val position = Position("<<<global>>>", 0, 0, 0)
     override val statements = mutableListOf<Statement>()        // not used

compilerAst/src/prog8/ast/Program.kt

@@ -17,7 +17,7 @@ class Program(val name: String,
     private val _modules = mutableListOf<Module>()
 
     val modules: List<Module> = _modules
-    val namespace: GlobalNamespace = GlobalNamespace(modules)
+    val namespace: GlobalNamespace = GlobalNamespace(_modules)
 
     init {
         // insert a container module for all interned strings later

compilerAst/src/prog8/ast/walk/AstWalker.kt

@@ -12,8 +12,11 @@ interface IAstModification {
 
     class Remove(val node: Node, val parent: IStatementContainer) : IAstModification {
         override fun perform() {
-            if (!parent.statements.remove(node) && parent !is GlobalNamespace)
-                throw FatalAstException("attempt to remove non-existing node $node")
+            if (!parent.statements.remove(node)) {
+                val glob = parent as? GlobalNamespace
+                if(glob!=null && !glob.modules.remove(node))
+                    throw FatalAstException("attempt to remove non-existing node $node")
+            }
         }
     }
 

docs/source/todo.rst

@@ -1,10 +1,7 @@
 TODO
 ====
 
-chess is larger than on 10.1  (other variables ended up in zeropage)
-rockrunner is larger than on 10.1
-medemo is quite a bit larger than on 10.1
-assembler, imageviewer is bigger than before  (since commit "added string.lstripped() and string.ltrimmed()" )
+rockrunner cave load/decode is now broken (even with -noopt)
 
 (after merge in boolean): move all "OperatorXinplace" from expressionGen to AssignmentGen, see if we can get rid of the Result return type.