From b4e94ae4dd1c29a966bb5ea0454ab31f1e9863cb Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Wed, 5 Jul 2023 23:14:51 +0200
Subject: [PATCH] optimizer: avoid symbol name clash when inlining subroutine

---
 .../src/prog8/optimizer/Extensions.kt         |  4 +--
 codeOptimizers/src/prog8/optimizer/Inliner.kt | 20 +++++++++++---
 compiler/src/prog8/compiler/Compiler.kt       |  2 +-
 .../test/codegeneration/TestVariousCodeGen.kt | 27 +++++++++++++++++++
 docs/source/todo.rst                          |  3 ---
 5 files changed, 47 insertions(+), 9 deletions(-)

diff --git a/codeOptimizers/src/prog8/optimizer/Extensions.kt b/codeOptimizers/src/prog8/optimizer/Extensions.kt
index b79394d01..66c8ac89f 100644
--- a/codeOptimizers/src/prog8/optimizer/Extensions.kt
+++ b/codeOptimizers/src/prog8/optimizer/Extensions.kt
@@ -54,8 +54,8 @@ fun Program.optimizeStatements(errors: IErrorReporter,
     return optimizationCount
 }
 
-fun Program.inlineSubroutines(): Int {
-    val inliner = Inliner(this)
+fun Program.inlineSubroutines(options: CompilationOptions): Int {
+    val inliner = Inliner(this, options)
     inliner.visit(this)
     return inliner.applyModifications()
 }
diff --git a/codeOptimizers/src/prog8/optimizer/Inliner.kt b/codeOptimizers/src/prog8/optimizer/Inliner.kt
index 2580e9eee..7936a5caa 100644
--- a/codeOptimizers/src/prog8/optimizer/Inliner.kt
+++ b/codeOptimizers/src/prog8/optimizer/Inliner.kt
@@ -8,7 +8,9 @@ import prog8.ast.statements.*
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstModification
 import prog8.ast.walk.IAstVisitor
+import prog8.code.core.CompilationOptions
 import prog8.code.core.InternalCompilerException
+import prog8.code.target.VMTarget
 
 
 private  fun isEmptyReturn(stmt: Statement): Boolean = stmt is Return && stmt.value==null
@@ -16,7 +18,7 @@ private  fun isEmptyReturn(stmt: Statement): Boolean = stmt is Return && stmt.va
 
 // inliner potentially enables *ONE LINED* subroutines, wihtout to be inlined.
 
-class Inliner(val program: Program): AstWalker() {
+class Inliner(private val program: Program, private val options: CompilationOptions): AstWalker() {
 
     class DetermineInlineSubs(val program: Program): IAstVisitor {
         private val modifications = mutableListOf<IAstModification>()
@@ -211,7 +213,7 @@ class Inliner(val program: Program): AstWalker() {
 
     override fun after(functionCallStatement: FunctionCallStatement, parent: Node): Iterable<IAstModification>  {
         val sub = functionCallStatement.target.targetStatement(program) as? Subroutine
-        return if(sub==null)
+        return if(sub==null || !canInline(sub, functionCallStatement))
             noModifications
         else
             possibleInlineFcallStmt(sub, functionCallStatement, parent)
@@ -219,7 +221,7 @@ class Inliner(val program: Program): AstWalker() {
 
     override fun before(functionCallExpr: FunctionCallExpression, parent: Node): Iterable<IAstModification> {
         val sub = functionCallExpr.target.targetStatement(program) as? Subroutine
-        if(sub!=null && sub.inline && sub.parameters.isEmpty()) {
+        if(sub!=null && sub.inline && sub.parameters.isEmpty() && canInline(sub, functionCallExpr)) {
             require(sub.statements.size == 1 || (sub.statements.size == 2 && isEmptyReturn(sub.statements[1]))) {
                 "invalid inline sub at ${sub.position}"
             }
@@ -246,5 +248,17 @@ class Inliner(val program: Program): AstWalker() {
         return noModifications
     }
 
+    private fun canInline(sub: Subroutine, fcall: IFunctionCall): Boolean {
+        if(!sub.inline)
+            return false
+        if(options.compTarget.name!=VMTarget.NAME) {
+            val stmt = sub.statements.single()
+            if (stmt is IFunctionCall) {
+                val existing = (fcall as Node).definingScope.lookup(stmt.target.nameInSource.take(1))
+                return existing !is VarDecl
+            }
+        }
+        return true
+    }
 }
 
diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt
index a8c1d84f2..58b1c5659 100644
--- a/compiler/src/prog8/compiler/Compiler.kt
+++ b/compiler/src/prog8/compiler/Compiler.kt
@@ -369,7 +369,7 @@ private fun optimizeAst(program: Program, compilerOptions: CompilationOptions, e
         val optsDone1 = program.simplifyExpressions(errors, compTarget)
         val optsDone2 = program.splitBinaryExpressions(compilerOptions)
         val optsDone3 = program.optimizeStatements(errors, functions, compilerOptions)
-        val optsDone4 = program.inlineSubroutines()
+        val optsDone4 = program.inlineSubroutines(compilerOptions)
         program.constantFold(errors, compTarget) // because simplified statements and expressions can result in more constants that can be folded away
         errors.report()
         if (optsDone1 + optsDone2 + optsDone3 + optsDone4 == 0)
diff --git a/compiler/test/codegeneration/TestVariousCodeGen.kt b/compiler/test/codegeneration/TestVariousCodeGen.kt
index 2d6a3b3b0..bc7556faa 100644
--- a/compiler/test/codegeneration/TestVariousCodeGen.kt
+++ b/compiler/test/codegeneration/TestVariousCodeGen.kt
@@ -257,4 +257,31 @@ mylabel:
 """
         compileText(Cx16Target(), true, src, writeAssembly = true) shouldNotBe null
     }
+
+    test("duplicate symbols okay other block and variable") {
+        val src = """
+main {
+    ubyte derp
+    sub start() {
+        derp++
+        foo.bar()
+    }
+}
+
+foo {
+    sub bar() {
+        derp.print()
+    }
+}
+
+derp {
+    sub print() {
+        cx16.r0++
+        cx16.r1++
+    }
+}"""
+
+        compileText(VMTarget(), true, src, writeAssembly = true) shouldNotBe null
+        compileText(Cx16Target(), true, src, writeAssembly = true) shouldNotBe null
+    }
 })
\ No newline at end of file
diff --git a/docs/source/todo.rst b/docs/source/todo.rst
index 657ec66ef..d28f97d31 100644
--- a/docs/source/todo.rst
+++ b/docs/source/todo.rst
@@ -1,9 +1,6 @@
 TODO
 ====
 
-- check for name clash: variable vs block name
-
-
 ...