From 7dd2517f67d395fdd26b902280f600766f2abbb9 Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sat, 15 Jan 2022 12:29:29 +0100 Subject: [PATCH] fix Zp allocation issues --- .../codegen/target/cpu6502/codegen/AsmGen.kt | 19 +---- .../compiler/astprocessing/AstChecker.kt | 19 +++-- compiler/test/ZeropageTests.kt | 77 ++++++++----------- .../src/prog8/compilerinterface/Zeropage.kt | 12 +-- docs/source/todo.rst | 1 + 5 files changed, 58 insertions(+), 70 deletions(-) diff --git a/codeGeneration/src/prog8/codegen/target/cpu6502/codegen/AsmGen.kt b/codeGeneration/src/prog8/codegen/target/cpu6502/codegen/AsmGen.kt index 6ae1271e1..45770f5e0 100644 --- a/codeGeneration/src/prog8/codegen/target/cpu6502/codegen/AsmGen.kt +++ b/codeGeneration/src/prog8/codegen/target/cpu6502/codegen/AsmGen.kt @@ -1465,22 +1465,11 @@ $repeatLabel lda $counterVar when(dt) { DataType.UBYTE, DataType.UWORD -> { val result = zeropage.allocate(counterVar, dt, null, stmt.position, errors) - return result.fold( - success = { zpvar -> - asmInfo.extraVars.add(Triple(dt, counterVar, zpvar.first)) - counterVar - }, - failure = { zpError -> - if(mustBeInZeropage) { - errors.err(zpError.message!!, stmt.position) - null - } else { - // allocate normally - asmInfo.extraVars.add(Triple(dt, counterVar, null)) - counterVar - } - } + result.fold( + success = { zpvar -> asmInfo.extraVars.add(Triple(dt, counterVar, zpvar.first)) }, + failure = { asmInfo.extraVars.add(Triple(dt, counterVar, null)) } // allocate normally ) + return counterVar } else -> throw AssemblyError("invalidt dt") } diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 53538a671..a40802880 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -1311,12 +1311,21 @@ internal class AstChecker(private val program: Program, private fun checkValueTypeAndRangeString(targetDt: DataType, value: StringLiteralValue) : Boolean { return if (targetDt == DataType.STR) { - if (value.value.length > 255) { - errors.err("string length must be 0-255", value.position) - false + when { + value.value.length > 255 -> { + errors.err("string length must be 0-255", value.position) + false + } + value.value.isEmpty() -> { + val decl = value.parent as? VarDecl + if(decl!=null && (decl.zeropage==ZeropageWish.REQUIRE_ZEROPAGE || decl.zeropage==ZeropageWish.PREFER_ZEROPAGE)) { + errors.err("string in Zeropage must be non-empty", value.position) + false + } + else true + } + else -> true } - else - true } else false } diff --git a/compiler/test/ZeropageTests.kt b/compiler/test/ZeropageTests.kt index 39de96964..6b1553793 100644 --- a/compiler/test/ZeropageTests.kt +++ b/compiler/test/ZeropageTests.kt @@ -1,7 +1,6 @@ package prog8tests -import com.github.michaelbull.result.expect -import com.github.michaelbull.result.get +import com.github.michaelbull.result.expectError import com.github.michaelbull.result.getOrElse import com.github.michaelbull.result.onFailure import io.kotest.assertions.fail @@ -22,6 +21,7 @@ import prog8.codegen.target.c64.C64Zeropage import prog8.codegen.target.cx16.CX16Zeropage import prog8.compilerinterface.* import prog8tests.helpers.ErrorReporterForTests +import java.lang.IllegalArgumentException class TestAbstractZeropage: FunSpec({ @@ -100,21 +100,18 @@ class TestC64Zeropage: FunSpec({ result.onFailure { fail(it.toString()) } result = zp.allocate("varname", DataType.UBYTE, null, null, errors) result.onFailure { fail(it.toString()) } - result = zp.allocate("varname", DataType.UBYTE, null, null, errors) - result.onFailure { fail(it.toString()) } // TODO SHOULD ACTUALLY BE ERROR + shouldThrow { zp.allocate("varname", DataType.UBYTE, null, null, errors) } result = zp.allocate("varname2", DataType.UBYTE, null, null, errors) result.onFailure { fail(it.toString()) } } test("testZpFloatEnable") { val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FULL, emptyList(), false, false, C64Target)) - shouldThrow { - zp.allocate("", DataType.FLOAT, null, null, errors) - } + var result = zp.allocate("", DataType.FLOAT, null, null, errors) + result.expectError { "should be allocation error due to disabled floats" } val zp2 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), true, false, C64Target)) - shouldThrow { - zp2.allocate("", DataType.FLOAT, null, null, errors) - } + result = zp2.allocate("", DataType.FLOAT, null, null, errors) + result.expectError { "should be allocation error due to disabled ZP use" } val zp3 = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.FLOATSAFE, emptyList(), true, false, C64Target)) zp3.allocate("", DataType.FLOAT, null, null, errors) } @@ -138,9 +135,8 @@ class TestC64Zeropage: FunSpec({ val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.DONTUSE, emptyList(), false, false, C64Target)) println(zp.free) zp.availableBytes() shouldBe 0 - shouldThrow { - zp.allocate("", DataType.BYTE, null, null, errors) - } + val result = zp.allocate("", DataType.BYTE, null, null, errors) + result.expectError { "expected error due to disabled ZP use" } } test("testFreeSpacesBytes") { @@ -185,10 +181,8 @@ class TestC64Zeropage: FunSpec({ zp.hasByteAvailable() shouldBe true zp.hasWordAvailable() shouldBe true - shouldThrow { - // in regular zp there aren't 5 sequential bytes free - zp.allocate("", DataType.FLOAT, null, null, errors) - } + var result = zp.allocate("", DataType.FLOAT, null, null, errors) + result.expectError { "expect allocation error: in regular zp there aren't 5 sequential bytes free" } for (i in 0 until zp.availableBytes()) { val result = zp.allocate("", DataType.UBYTE, null, null, errors) @@ -197,12 +191,10 @@ class TestC64Zeropage: FunSpec({ zp.availableBytes() shouldBe 0 zp.hasByteAvailable() shouldBe false zp.hasWordAvailable() shouldBe false - shouldThrow { - zp.allocate("", DataType.UBYTE, null, null, errors) - } - shouldThrow { - zp.allocate("", DataType.UWORD, null, null, errors) - } + result = zp.allocate("", DataType.UBYTE, null, null, errors) + result.expectError { "expected allocation error" } + result = zp.allocate("", DataType.UWORD, null, null, errors) + result.expectError { "expected allocation error" } } test("testFullAllocation") { @@ -210,7 +202,7 @@ class TestC64Zeropage: FunSpec({ zp.availableBytes() shouldBe 239 zp.hasByteAvailable() shouldBe true zp.hasWordAvailable() shouldBe true - val result = zp.allocate("", DataType.UWORD, null, null, errors) + var result = zp.allocate("", DataType.UWORD, null, null, errors) val loc = result.getOrElse { throw it } .first loc shouldBeGreaterThan 3u loc shouldNotBeIn zp.free @@ -221,10 +213,9 @@ class TestC64Zeropage: FunSpec({ } zp.availableBytes() shouldBe 5 - shouldThrow { - // can't allocate because no more sequential bytes, only fragmented - zp.allocate("", DataType.UWORD, null, null, errors) - } + // can't allocate because no more sequential bytes, only fragmented + result = zp.allocate("", DataType.UWORD, null, null, errors) + result.expectError { "should give allocation error" } for(i in 0..4) { zp.allocate("", DataType.UBYTE, null, null, errors) @@ -233,27 +224,25 @@ class TestC64Zeropage: FunSpec({ zp.availableBytes() shouldBe 0 zp.hasByteAvailable() shouldBe false zp.hasWordAvailable() shouldBe false - shouldThrow { - // no more space - zp.allocate("", DataType.UBYTE, null, null, errors) - } + result = zp.allocate("", DataType.UBYTE, null, null, errors) + result.expectError { "should give allocation error" } } test("testEfficientAllocation") { val zp = C64Zeropage(CompilationOptions(OutputType.RAW, LauncherType.NONE, ZeropageType.BASICSAFE, emptyList(), true, false, C64Target)) zp.availableBytes() shouldBe 18 - zp.allocate("", DataType.WORD, null, null, errors) shouldBe 0x04u - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0x06u - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0x0au - zp.allocate("", DataType.UWORD, null, null, errors) shouldBe 0x9bu - zp.allocate("", DataType.UWORD, null, null, errors) shouldBe 0x9eu - zp.allocate("", DataType.UWORD, null, null, errors) shouldBe 0xa5u - zp.allocate("", DataType.UWORD, null, null, errors) shouldBe 0xb0u - zp.allocate("", DataType.UWORD, null, null, errors) shouldBe 0xbeu - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0x0eu - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0x92u - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0x96u - zp.allocate("", DataType.UBYTE, null, null, errors) shouldBe 0xf9u + zp.allocate("", DataType.WORD, null, null, errors ).getOrElse{throw it}.first shouldBe 0x04u + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0x06u + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0x0au + zp.allocate("", DataType.UWORD, null, null, errors).getOrElse{throw it}.first shouldBe 0x9bu + zp.allocate("", DataType.UWORD, null, null, errors).getOrElse{throw it}.first shouldBe 0x9eu + zp.allocate("", DataType.UWORD, null, null, errors).getOrElse{throw it}.first shouldBe 0xa5u + zp.allocate("", DataType.UWORD, null, null, errors).getOrElse{throw it}.first shouldBe 0xb0u + zp.allocate("", DataType.UWORD, null, null, errors).getOrElse{throw it}.first shouldBe 0xbeu + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0x0eu + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0x92u + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0x96u + zp.allocate("", DataType.UBYTE, null, null, errors).getOrElse{throw it}.first shouldBe 0xf9u zp.availableBytes() shouldBe 0 } diff --git a/compilerInterfaces/src/prog8/compilerinterface/Zeropage.kt b/compilerInterfaces/src/prog8/compilerinterface/Zeropage.kt index dce73479e..cd761dbd1 100644 --- a/compilerInterfaces/src/prog8/compilerinterface/Zeropage.kt +++ b/compilerInterfaces/src/prog8/compilerinterface/Zeropage.kt @@ -6,7 +6,7 @@ import com.github.michaelbull.result.Result import prog8.ast.base.* -class ZeropageDepletedError(message: String) : Exception(message) +class ZeropageAllocationError(message: String) : Exception(message) abstract class Zeropage(protected val options: CompilationOptions) { @@ -36,11 +36,11 @@ abstract class Zeropage(protected val options: CompilationOptions) { return free.windowed(2).any { it[0] == it[1] - 1u } } - fun allocate(scopedname: String, datatype: DataType, arraySize: Int?, position: Position?, errors: IErrorReporter): Result, ZeropageDepletedError> { + fun allocate(scopedname: String, datatype: DataType, arraySize: Int?, position: Position?, errors: IErrorReporter): Result, ZeropageAllocationError> { require(scopedname.isEmpty() || !allocations.values.any { it.first==scopedname } ) {"scopedname can't be allocated twice"} if(options.zeropage== ZeropageType.DONTUSE) - throw InternalCompilerException("zero page usage has been disabled") + return Err(ZeropageAllocationError("zero page usage has been disabled")) val size: Int = when (datatype) { @@ -61,9 +61,9 @@ abstract class Zeropage(protected val options: CompilationOptions) { else errors.warn("$scopedname: allocating a large value in zeropage; float $memsize bytes", Position.DUMMY) memsize - } else throw InternalCompilerException("floating point option not enabled") + } else return Err(ZeropageAllocationError("floating point option not enabled")) } - else -> throw InternalCompilerException("cannot put datatype $datatype in zeropage") + else -> return Err(ZeropageAllocationError("cannot put datatype $datatype in zeropage")) } synchronized(this) { @@ -82,7 +82,7 @@ abstract class Zeropage(protected val options: CompilationOptions) { } } - return Err(ZeropageDepletedError("no more free space in ZP to allocate $size sequential bytes")) + return Err(ZeropageAllocationError("no more free space in ZP to allocate $size sequential bytes")) } private fun reserve(range: UIntRange) = free.removeAll(range) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 6f3a4df08..146c17122 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -6,6 +6,7 @@ For next compiler release (7.7) - fix array and string initialization in zeropage - fix cx16 zeropage preallocated vars (virtual regs) - fix ForloopAsmGen zp allocation handling +- check all examples if they still run correctly (c64 + cx16) - document check: arrays and strings can also be placed in zeropage (but almost never should, due to size!) - document @requirezp and add to syntax def files and IDEA