From ab544ee96516ea3b827868cc46e531aec8d6f196 Mon Sep 17 00:00:00 2001
From: Irmen de Jong <irmen@razorvine.net>
Date: Tue, 16 Feb 2021 23:43:38 +0100
Subject: [PATCH] improved string constant interning; no longer output
 duplicate strings in the Ast

---
 compiler/res/prog8lib/c64/syslib.p8           |  2 +-
 compiler/src/prog8/compiler/Compiler.kt       |  3 +-
 .../astprocessing/LiteralsToAutoVars.kt       | 11 ++----
 compilerAst/src/prog8/ast/AstToplevel.kt      | 38 ++++++++++++++++---
 .../src/prog8/ast/statements/AstStatements.kt |  6 ---
 docs/source/todo.rst                          |  1 -
 6 files changed, 39 insertions(+), 22 deletions(-)

diff --git a/compiler/res/prog8lib/c64/syslib.p8 b/compiler/res/prog8lib/c64/syslib.p8
index 9ed05e340..9da60a794 100644
--- a/compiler/res/prog8lib/c64/syslib.p8
+++ b/compiler/res/prog8lib/c64/syslib.p8
@@ -178,7 +178,7 @@ c64 {
 
 ; ---- C64 ROM kernal routines ----
 
-romsub $AB1E = STROUT(uword strptr @ AY) clobbers(A, X, Y)      ; print null-terminated string (use c64scr.print instead)
+romsub $AB1E = STROUT(uword strptr @ AY) clobbers(A, X, Y)      ; print null-terminated string (use txt.print instead)
 romsub $E544 = CLEARSCR() clobbers(A,X,Y)                       ; clear the screen
 romsub $E566 = HOMECRSR() clobbers(A,X,Y)                       ; cursor to top left of screen
 romsub $EA31 = IRQDFRT() clobbers(A,X,Y)                        ; default IRQ routine
diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt
index 92dcc92db..a973b5722 100644
--- a/compiler/src/prog8/compiler/Compiler.kt
+++ b/compiler/src/prog8/compiler/Compiler.kt
@@ -172,7 +172,6 @@ private fun parseImports(filepath: Path, errors: ErrorReporter): Triple<Program,
     errors.handle()
 
     val importedFiles = programAst.modules.filter { !it.source.startsWith("@embedded@") }.map { it.source }
-
     val compilerOptions = determineCompilationOptions(programAst)
     if (compilerOptions.launcher == LauncherType.BASIC && compilerOptions.output != OutputType.PRG)
         throw ParsingFailedError("${programAst.modules.first().position} BASIC launcher requires output type PRG.")
@@ -188,7 +187,7 @@ private fun parseImports(filepath: Path, errors: ErrorReporter): Triple<Program,
 }
 
 private fun determineCompilationOptions(program: Program): CompilationOptions {
-    val mainModule = program.modules.first()
+    val mainModule = program.mainModule
     val outputType = (mainModule.statements.singleOrNull { it is Directive && it.directive == "%output" }
             as? Directive)?.args?.single()?.name?.toUpperCase()
     val launcherType = (mainModule.statements.singleOrNull { it is Directive && it.directive == "%launcher" }
diff --git a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt
index 2247f1377..e698610d8 100644
--- a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt
+++ b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt
@@ -17,13 +17,10 @@ internal class LiteralsToAutoVars(private val program: Program) : AstWalker() {
 
     override fun after(string: StringLiteralValue, parent: Node): Iterable<IAstModification> {
         if(string.parent !is VarDecl && string.parent !is WhenChoice) {
-            // replace the literal string by a identifier reference to a new local vardecl
-            val vardecl = VarDecl.createAuto(string)
-            val identifier = IdentifierReference(listOf(vardecl.name), vardecl.position)
-            return listOf(
-                    IAstModification.ReplaceNode(string, identifier, parent),
-                    IAstModification.InsertFirst(vardecl, string.definingScope())
-            )
+            // replace the literal string by a identifier reference to the interned string
+            val scopedName = program.internString(string)
+            val identifier = IdentifierReference(scopedName, string.position)
+            return listOf(IAstModification.ReplaceNode(string, identifier, parent))
         }
         return noModifications
     }
diff --git a/compilerAst/src/prog8/ast/AstToplevel.kt b/compilerAst/src/prog8/ast/AstToplevel.kt
index 5c1132834..6b3b49c1c 100644
--- a/compilerAst/src/prog8/ast/AstToplevel.kt
+++ b/compilerAst/src/prog8/ast/AstToplevel.kt
@@ -1,10 +1,7 @@
 package prog8.ast
 
 import prog8.ast.base.*
-import prog8.ast.expressions.Expression
-import prog8.ast.expressions.IdentifierReference
-import prog8.ast.expressions.InferredTypes
-import prog8.ast.expressions.NumericLiteralValue
+import prog8.ast.expressions.*
 import prog8.ast.statements.*
 import prog8.ast.walk.AstWalker
 import prog8.ast.walk.IAstVisitor
@@ -258,10 +255,26 @@ interface IBuiltinFunctions {
 class Program(val name: String, val modules: MutableList<Module>, val builtinFunctions: IBuiltinFunctions): Node {
     val namespace = GlobalNamespace(modules, builtinFunctions.names)
 
+    val mainModule: Module
+        get() = modules.first { it.name!=internedStringsModuleName }
     val definedLoadAddress: Int
-        get() = modules.first().loadAddress
+        get() = mainModule.loadAddress
 
     var actualLoadAddress: Int = 0
+    private val internedStrings = mutableMapOf<String, List<String>>()
+    val internedStringsModuleName = "prog8_interned_strings"
+
+    init {
+        // insert a container module for all interned strings later
+        if(modules.firstOrNull()?.name != internedStringsModuleName) {
+            val internedStringsModule = Module(internedStringsModuleName, mutableListOf(), Position.DUMMY, false, Path.of(""))
+            modules.add(0, internedStringsModule)
+            val block = Block(internedStringsModuleName, null, mutableListOf(), false, Position.DUMMY)
+            internedStringsModule.statements.add(block)
+            internedStringsModule.linkParents(this)
+            internedStringsModule.program = this
+        }
+    }
 
     fun entrypoint(): Subroutine? {
         val mainBlocks = allBlocks().filter { it.name=="main" }
@@ -274,6 +287,21 @@ class Program(val name: String, val modules: MutableList<Module>, val builtinFun
         }
     }
 
+    fun internString(string: StringLiteralValue): List<String> {
+        val existing = internedStrings[string.value]
+        if(existing!=null)
+            return existing
+
+        val decl = VarDecl(VarDeclType.VAR, DataType.STR, ZeropageWish.NOT_IN_ZEROPAGE, null, "string_${internedStrings.size}", null, string,
+            isArray = false, autogeneratedDontRemove = true, position = string.position)
+        val internedStringsBlock = modules.first { it.name==internedStringsModuleName }.statements.first { it is Block && it.name == internedStringsModuleName}
+        (internedStringsBlock as Block).statements.add(decl)
+        decl.linkParents(internedStringsBlock)
+        val scopedName = listOf(internedStringsModuleName, decl.name)
+        internedStrings[string.value] = scopedName
+        return scopedName
+    }
+
     fun allBlocks(): List<Block> = modules.flatMap { it.statements.filterIsInstance<Block>() }
 
     override val position: Position = Position.DUMMY
diff --git a/compilerAst/src/prog8/ast/statements/AstStatements.kt b/compilerAst/src/prog8/ast/statements/AstStatements.kt
index 8a7d6aeb3..38101d90b 100644
--- a/compilerAst/src/prog8/ast/statements/AstStatements.kt
+++ b/compilerAst/src/prog8/ast/statements/AstStatements.kt
@@ -176,12 +176,6 @@ open class VarDecl(val type: VarDeclType,
     companion object {
         private var autoHeapValueSequenceNumber = 0
 
-        fun createAuto(string: StringLiteralValue): VarDecl {
-            val autoVarName = "auto_heap_value_${++autoHeapValueSequenceNumber}"
-            return VarDecl(VarDeclType.VAR, DataType.STR, ZeropageWish.NOT_IN_ZEROPAGE, null, autoVarName, null, string,
-                        isArray = false, autogeneratedDontRemove = true, position = string.position)
-        }
-
         fun createAuto(array: ArrayLiteralValue): VarDecl {
             val autoVarName = "auto_heap_value_${++autoHeapValueSequenceNumber}"
             val arrayDt =
diff --git a/docs/source/todo.rst b/docs/source/todo.rst
index 461bb6503..867e7299e 100644
--- a/docs/source/todo.rst
+++ b/docs/source/todo.rst
@@ -2,7 +2,6 @@
 TODO
 ====
 
-- auto_heap_value for strings: avoid creating multiple values for identical strings (or fix it at the end)
 - add sound to the cx16 tehtriz
 - refactor the asmgen into their own submodule?
 - refactor the compiler optimizers into their own submodule?