From ba96a637be66f9fde407300b011b9baa65801c00 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Tue, 18 May 2021 23:52:43 +0200 Subject: [PATCH] remove strdedup compiler argument again (string deduplication is the default again but only for known-const strings, i.e. string literals) --- compiler/src/prog8/CompilerMain.kt | 5 +- compiler/src/prog8/compiler/Compiler.kt | 11 ++-- .../compiler/astprocessing/AstExtensions.kt | 2 +- .../astprocessing/LiteralsToAutoVars.kt | 5 +- compiler/test/UnitTests.kt | 56 +++++++++---------- compilerAst/src/prog8/ast/AstToplevel.kt | 19 +++---- docs/source/todo.rst | 1 - .../src/prog8/http/TestHttp.kt | 2 +- 8 files changed, 45 insertions(+), 56 deletions(-) diff --git a/compiler/src/prog8/CompilerMain.kt b/compiler/src/prog8/CompilerMain.kt index 59cbdecdb..4f3751838 100644 --- a/compiler/src/prog8/CompilerMain.kt +++ b/compiler/src/prog8/CompilerMain.kt @@ -42,7 +42,6 @@ private fun compileMain(args: Array) { val compilationTarget by cli.option(ArgType.String, fullName = "target", description = "target output of the compiler, currently '${C64Target.name}' and '${Cx16Target.name}' available").default(C64Target.name) val moduleFiles by cli.argument(ArgType.String, fullName = "modules", description = "main module file(s) to compile").multiple(999) val libDirs by cli.option(ArgType.String, fullName="libdirs", description = "list of extra paths to search in for imported modules").multiple().delimiter(File.pathSeparator) - val stringDedup by cli.option(ArgType.Boolean, fullName="strdedup", description = "deduplicate identical string literals").default(false) try { cli.parse(args) @@ -70,7 +69,7 @@ private fun compileMain(args: Array) { val results = mutableListOf() for(filepathRaw in moduleFiles) { val filepath = pathFrom(filepathRaw).normalize() - val compilationResult = compileProgram(filepath, dontOptimize!=true, dontWriteAssembly!=true, slowCodegenWarnings==true, stringDedup, compilationTarget, libdirs, outputPath) + val compilationResult = compileProgram(filepath, dontOptimize!=true, dontWriteAssembly!=true, slowCodegenWarnings==true, compilationTarget, libdirs, outputPath) results.add(compilationResult) } @@ -107,7 +106,7 @@ private fun compileMain(args: Array) { val filepath = pathFrom(filepathRaw).normalize() val compilationResult: CompilationResult try { - compilationResult = compileProgram(filepath, dontOptimize!=true, dontWriteAssembly!=true, slowCodegenWarnings==true, stringDedup, compilationTarget, libdirs, outputPath) + compilationResult = compileProgram(filepath, dontOptimize!=true, dontWriteAssembly!=true, slowCodegenWarnings==true, compilationTarget, libdirs, outputPath) if(!compilationResult.success) exitProcess(1) } catch (x: ParsingFailedError) { diff --git a/compiler/src/prog8/compiler/Compiler.kt b/compiler/src/prog8/compiler/Compiler.kt index 997d544c8..cc80c2756 100644 --- a/compiler/src/prog8/compiler/Compiler.kt +++ b/compiler/src/prog8/compiler/Compiler.kt @@ -50,7 +50,6 @@ data class CompilationOptions(val output: OutputType, val zpReserved: List, val floats: Boolean, val noSysInit: Boolean, - val stringDedup: Boolean, val compTarget: ICompilationTarget) { var slowCodegenWarnings = false var optimize = false @@ -70,7 +69,6 @@ fun compileProgram(filepath: Path, optimize: Boolean, writeAssembly: Boolean, slowCodegenWarnings: Boolean, - stringDedup: Boolean, compilationTarget: String, libdirs: List, outputDir: Path): CompilationResult { @@ -92,7 +90,7 @@ fun compileProgram(filepath: Path, try { val totalTime = measureTimeMillis { // import main module and everything it needs - val (ast, compilationOptions, imported) = parseImports(filepath, errors, compTarget, stringDedup, libdirs) + val (ast, compilationOptions, imported) = parseImports(filepath, errors, compTarget, libdirs) compilationOptions.slowCodegenWarnings = slowCodegenWarnings compilationOptions.optimize = optimize programAst = ast @@ -171,7 +169,6 @@ private class BuiltinFunctionsFacade(functions: Map): IBuilt private fun parseImports(filepath: Path, errors: IErrorReporter, compTarget: ICompilationTarget, - stringDedup: Boolean, libdirs: List): Triple> { val compilationTargetName = compTarget.name println("Compiler target: $compilationTargetName. Parsing...") @@ -184,7 +181,7 @@ private fun parseImports(filepath: Path, errors.report() val importedFiles = programAst.modules.filter { !it.source.startsWith("@embedded@") }.map { it.source } - val compilerOptions = determineCompilationOptions(programAst, stringDedup, compTarget) + val compilerOptions = determineCompilationOptions(programAst, compTarget) if (compilerOptions.launcher == LauncherType.BASIC && compilerOptions.output != OutputType.PRG) throw ParsingFailedError("${programAst.modules.first().position} BASIC launcher requires output type PRG.") @@ -199,7 +196,7 @@ private fun parseImports(filepath: Path, return Triple(programAst, compilerOptions, importedFiles) } -private fun determineCompilationOptions(program: Program, stringDedup: Boolean, compTarget: ICompilationTarget): CompilationOptions { +private fun determineCompilationOptions(program: Program, compTarget: ICompilationTarget): CompilationOptions { val mainModule = program.mainModule val outputType = (mainModule.statements.singleOrNull { it is Directive && it.directive == "%output" } as? Directive)?.args?.single()?.name?.uppercase() @@ -246,7 +243,7 @@ private fun determineCompilationOptions(program: Program, stringDedup: Boolean, return CompilationOptions( if (outputType == null) OutputType.PRG else OutputType.valueOf(outputType), if (launcherType == null) LauncherType.BASIC else LauncherType.valueOf(launcherType), - zpType, zpReserved, floatsEnabled, noSysInit, stringDedup, + zpType, zpReserved, floatsEnabled, noSysInit, compTarget ) } diff --git a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt index fc4469098..2497e5d69 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstExtensions.kt @@ -53,7 +53,7 @@ internal fun Program.checkIdentifiers(errors: IErrorReporter, options: Compilati val transforms = AstVariousTransforms(this) transforms.visit(this) transforms.applyModifications() - val lit2decl = LiteralsToAutoVars(this, options) + val lit2decl = LiteralsToAutoVars(this) lit2decl.visit(this) lit2decl.applyModifications() } diff --git a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt index 9d8788719..705939bcf 100644 --- a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt +++ b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt @@ -10,15 +10,14 @@ import prog8.ast.statements.VarDecl import prog8.ast.statements.WhenChoice import prog8.ast.walk.AstWalker import prog8.ast.walk.IAstModification -import prog8.compiler.CompilationOptions -internal class LiteralsToAutoVars(private val program: Program, private val options: CompilationOptions) : AstWalker() { +internal class LiteralsToAutoVars(private val program: Program) : AstWalker() { override fun after(string: StringLiteralValue, parent: Node): Iterable { if(string.parent !is VarDecl && string.parent !is WhenChoice) { // replace the literal string by a identifier reference to the interned string - val scopedName = program.internString(string, options.stringDedup) + val scopedName = program.internString(string) val identifier = IdentifierReference(scopedName, string.position) return listOf(IAstModification.ReplaceNode(string, identifier, parent)) } diff --git a/compiler/test/UnitTests.kt b/compiler/test/UnitTests.kt index 685839910..735842dbd 100644 --- a/compiler/test/UnitTests.kt +++ b/compiler/test/UnitTests.kt @@ -134,7 +134,7 @@ class TestC64Zeropage { @Test fun testNames() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, C64Target)) zp.allocate("", DataType.UBYTE, null, errors) zp.allocate("", DataType.UBYTE, null, errors) @@ -147,37 +147,37 @@ class TestC64Zeropage { @Test fun testZpFloatEnable() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) assertFailsWith { zp.allocate("", DataType.FLOAT, null, errors) } - val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), true, false, true, C64Target)) + val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), true, false, C64Target)) assertFailsWith { zp2.allocate("", DataType.FLOAT, null, errors) } - val zp3 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), true, false, true, C64Target)) + val zp3 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), true, false, C64Target)) zp3.allocate("", DataType.FLOAT, null, errors) } @Test fun testZpModesWithFloats() { - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, C64Target)) - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, true, C64Target)) - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, true, C64Target)) - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), false, false, true, C64Target)) - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, true, C64Target)) - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), true, false, true, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), false, false, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), true, false, C64Target)) assertFailsWith { - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), true, false, true, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), true, false, C64Target)) } assertFailsWith { - C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), true, false, true, C64Target)) + C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), true, false, C64Target)) } } @Test fun testZpDontuse() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), false, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), false, false, C64Target)) println(zp.free) assertEquals(0, zp.available()) assertFailsWith { @@ -187,19 +187,19 @@ class TestC64Zeropage { @Test fun testFreeSpaces() { - val zp1 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, true, C64Target)) + val zp1 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, C64Target)) assertEquals(18, zp1.available()) - val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), false, false, true, C64Target)) + val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), false, false, C64Target)) assertEquals(85, zp2.available()) - val zp3 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, true, C64Target)) + val zp3 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, C64Target)) assertEquals(125, zp3.available()) - val zp4 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, C64Target)) + val zp4 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) assertEquals(238, zp4.available()) } @Test fun testReservedSpace() { - val zp1 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, C64Target)) + val zp1 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) assertEquals(238, zp1.available()) assertTrue(50 in zp1.free) assertTrue(100 in zp1.free) @@ -208,7 +208,7 @@ class TestC64Zeropage { assertTrue(200 in zp1.free) assertTrue(255 in zp1.free) assertTrue(199 in zp1.free) - val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, listOf(50 .. 100, 200..255), false, false, true, C64Target)) + val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, listOf(50 .. 100, 200..255), false, false, C64Target)) assertEquals(139, zp2.available()) assertFalse(50 in zp2.free) assertFalse(100 in zp2.free) @@ -221,7 +221,7 @@ class TestC64Zeropage { @Test fun testBasicsafeAllocation() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, C64Target)) assertEquals(18, zp.available()) assertFailsWith { @@ -244,7 +244,7 @@ class TestC64Zeropage { @Test fun testFullAllocation() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) assertEquals(238, zp.available()) val loc = zp.allocate("", DataType.UWORD, null, errors) assertTrue(loc > 3) @@ -274,7 +274,7 @@ class TestC64Zeropage { @Test fun testEfficientAllocation() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, C64Target)) assertEquals(18, zp.available()) assertEquals(0x04, zp.allocate("", DataType.WORD, null, errors)) assertEquals(0x06, zp.allocate("", DataType.UBYTE, null, errors)) @@ -293,7 +293,7 @@ class TestC64Zeropage { @Test fun testReservedLocations() { - val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, true, C64Target)) + val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, C64Target)) assertEquals(zp.SCRATCH_REG, zp.SCRATCH_B1+1, "zp _B1 and _REG must be next to each other to create a word") } } @@ -303,23 +303,23 @@ class TestC64Zeropage { class TestCx16Zeropage { @Test fun testReservedLocations() { - val zp = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, true, Cx16Target)) + val zp = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), false, false, Cx16Target)) assertEquals(zp.SCRATCH_REG, zp.SCRATCH_B1+1, "zp _B1 and _REG must be next to each other to create a word") } @Test fun testFreeSpaces() { - val zp1 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, true, Cx16Target)) + val zp1 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, Cx16Target)) assertEquals(88, zp1.available()) - val zp3 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, true, Cx16Target)) + val zp3 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.KERNALSAFE, emptyList(), false, false, Cx16Target)) assertEquals(175, zp3.available()) - val zp4 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, Cx16Target)) + val zp4 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, Cx16Target)) assertEquals(216, zp4.available()) } @Test fun testReservedSpace() { - val zp1 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, true, Cx16Target)) + val zp1 = CX16Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, Cx16Target)) assertEquals(216, zp1.available()) assertTrue(0x22 in zp1.free) assertTrue(0x80 in zp1.free) diff --git a/compilerAst/src/prog8/ast/AstToplevel.kt b/compilerAst/src/prog8/ast/AstToplevel.kt index 77a41c01a..913c43d69 100644 --- a/compilerAst/src/prog8/ast/AstToplevel.kt +++ b/compilerAst/src/prog8/ast/AstToplevel.kt @@ -281,7 +281,7 @@ class Program(val name: String, return mainBlocks[0].subScope("start") as Subroutine } - fun internString(string: StringLiteralValue, dedup: Boolean): List { + fun internString(string: StringLiteralValue): List { // Move a string literal into the internal, deduplicated, string pool // replace it with a variable declaration that points to the entry in the pool. @@ -305,18 +305,13 @@ class Program(val name: String, } val key = Pair(string.value, string.altEncoding) - if(dedup) { - val existing = internedStringsUnique[key] - if (existing != null) - return existing + val existing = internedStringsUnique[key] + if (existing != null) + return existing - val scopedName = getScopedName(string) - internedStringsUnique[key] = scopedName - return scopedName - } else { - // don't deduplicate string literals - return getScopedName(string) - } + val scopedName = getScopedName(string) + internedStringsUnique[key] = scopedName + return scopedName } diff --git a/docs/source/todo.rst b/docs/source/todo.rst index ed030ff2d..15a15853e 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -6,7 +6,6 @@ TODO However: who even needs variables declared in prog8 code that are only used by assembly??? - github issue about strings and their immutability: - Can we make deduplication the default again? (only string literals are considered...) remove cli option for it again? IMPROVE DOCUMENTATION ABOUT STRINGS AND DEDUP and (NON)IMMUTABILITY. - test all examples (including imgviewer, assembler and petaxian) before release of the new version diff --git a/httpCompilerService/src/prog8/http/TestHttp.kt b/httpCompilerService/src/prog8/http/TestHttp.kt index 6736978cf..8820fde7f 100644 --- a/httpCompilerService/src/prog8/http/TestHttp.kt +++ b/httpCompilerService/src/prog8/http/TestHttp.kt @@ -29,7 +29,7 @@ class RequestParser : Take { val form = RqFormBase(request) val names = form.names() val a = form.param("a").single() - val compilationResult = compileProgram(Path.of(a), true, true, true, false, "c64", emptyList(), Path.of(".")) + val compilationResult = compileProgram(Path.of(a), true, true, true, "c64", emptyList(), Path.of(".")) return RsJson(Jsonding()) } }