From aef211e5f3ab7fa6d8aa5bd419f6e68e5e15a55c Mon Sep 17 00:00:00 2001 From: Irmen de Jong Date: Sun, 13 Oct 2024 17:46:41 +0200 Subject: [PATCH] stricter array literal element type handling (number,bool,address-of). More consistent implicit address-of handling if array literals contain by-ref identifiers (such as subroutine names) --- codeCore/src/prog8/code/SymbolTableMaker.kt | 1 - codeCore/src/prog8/code/ast/AstExpressions.kt | 1 + codeCore/src/prog8/code/ast/AstPrinter.kt | 1 + .../src/prog8/codegen/cpu6502/AsmGen.kt | 1 - .../optimizer/ConstantFoldingOptimizer.kt | 2 +- .../compiler/astprocessing/AstChecker.kt | 14 +--- .../astprocessing/IntermediateAstMaker.kt | 7 +- .../astprocessing/LiteralsToAutoVars.kt | 2 + .../compiler/astprocessing/TypecastsAdder.kt | 20 +++++ .../test/codegeneration/TestArrayThings.kt | 41 +++++++++ docs/source/todo.rst | 2 +- examples/test.p8 | 83 ++----------------- 12 files changed, 81 insertions(+), 94 deletions(-) diff --git a/codeCore/src/prog8/code/SymbolTableMaker.kt b/codeCore/src/prog8/code/SymbolTableMaker.kt index 6462276ff..1b3b44a7a 100644 --- a/codeCore/src/prog8/code/SymbolTableMaker.kt +++ b/codeCore/src/prog8/code/SymbolTableMaker.kt @@ -131,7 +131,6 @@ class SymbolTableMaker(private val program: PtProgram, private val options: Comp TODO("address-of array element $it in initial array value") StArrayElement(null, it.identifier.name, null) } - is PtIdentifier -> StArrayElement(null, it.name, null) is PtNumber -> StArrayElement(it.number, null, null) is PtBool -> StArrayElement(null, null, it.value) else -> throw AssemblyError("invalid array element $it") diff --git a/codeCore/src/prog8/code/ast/AstExpressions.kt b/codeCore/src/prog8/code/ast/AstExpressions.kt index f5ee093a5..b07c59d66 100644 --- a/codeCore/src/prog8/code/ast/AstExpressions.kt +++ b/codeCore/src/prog8/code/ast/AstExpressions.kt @@ -163,6 +163,7 @@ class PtArrayIndexer(elementType: DataType, position: Position): PtExpression(el class PtArray(type: DataType, position: Position): PtExpression(type, position) { + // children are always one of 3 types: PtBool, PtNumber or PtAddressOf. override fun hashCode(): Int = Objects.hash(children, type) override fun equals(other: Any?): Boolean { if(other==null || other !is PtArray) diff --git a/codeCore/src/prog8/code/ast/AstPrinter.kt b/codeCore/src/prog8/code/ast/AstPrinter.kt index 47d47eff3..aee95db21 100644 --- a/codeCore/src/prog8/code/ast/AstPrinter.kt +++ b/codeCore/src/prog8/code/ast/AstPrinter.kt @@ -24,6 +24,7 @@ fun printAst(root: PtNode, skipLibraries: Boolean, output: (text: String) -> Uni is PtArray -> { val valuelist = node.children.map { when (it) { + is PtBool -> it.toString() is PtNumber -> it.number.toString() is PtIdentifier -> it.name else -> "?" diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt index a099bf80c..461530c40 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/AsmGen.kt @@ -145,7 +145,6 @@ private fun PtVariable.prefix(parent: PtNode, st: SymbolTable): PtVariable { val newValue = PtArray(arrayValue.type, arrayValue.position) arrayValue.children.forEach { elt -> when(elt) { - is PtIdentifier -> newValue.add(elt.prefix(arrayValue, st)) is PtBool -> newValue.add(elt) is PtNumber -> newValue.add(elt) is PtAddressOf -> { diff --git a/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt b/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt index 0b9a68164..a2aa74c6c 100644 --- a/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt +++ b/codeOptimizers/src/prog8/optimizer/ConstantFoldingOptimizer.kt @@ -122,7 +122,7 @@ class ConstantFoldingOptimizer(private val program: Program, private val errors: errors.warn("resulting array has length zero", part.position) val tmp = mutableListOf() repeat(rightconst.number.toInt()) { - tmp += part.value + part.value.forEach { tmp += it.copy() } } val newArray = ArrayLiteral(part.type, tmp.toTypedArray(), part.position) return listOf(IAstModification.ReplaceNode(expr, newArray, parent)) diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 463b2b465..fbe4695b7 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -1010,19 +1010,8 @@ internal class AstChecker(private val program: Program, checkValueTypeAndRangeArray(array.type.getOr(DataType.UNDEFINED), arrayspec, array) } - fun isPassByReferenceElement(e: Expression): Boolean { - if(e is IdentifierReference) { - val decl = e.targetVarDecl(program) - return if(decl!=null) - decl.datatype in PassByReferenceDatatypes - else - true // is probably a symbol that needs addr-of - } - return e is StringLiteral - } - if(array.parent is VarDecl) { - if (!array.value.all { it is NumericLiteral || it is AddressOf || isPassByReferenceElement(it) }) + if (!array.value.all { it is NumericLiteral || it is AddressOf }) errors.err("array literal for variable initialization contains non-constant elements", array.position) } else if(array.parent is ForLoop) { if (!array.value.all { it.constValue(program) != null }) @@ -1036,6 +1025,7 @@ internal class AstChecker(private val program: Program, if(arraydt!=targetDt) errors.err("value has incompatible type ($arraydt) for the variable ($targetDt)", array.position) } + super.visit(array) } diff --git a/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt b/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt index 2d023ef09..61e7ceb52 100644 --- a/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt +++ b/compiler/src/prog8/compiler/astprocessing/IntermediateAstMaker.kt @@ -566,8 +566,11 @@ class IntermediateAstMaker(private val program: Program, private val errors: IEr private fun transform(srcArr: ArrayLiteral): PtArray { val arr = PtArray(srcArr.inferType(program).getOrElse { throw FatalAstException("array must know its type") }, srcArr.position) - for (elt in srcArr.value) - arr.add(transformExpression(elt)) + for (elt in srcArr.value) { + val child = transformExpression(elt) + require(child is PtAddressOf || child is PtBool || child is PtNumber) { "array element invalid type $child" } + arr.add(child) + } return arr } diff --git a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt index 26de5ee99..fb5d469db 100644 --- a/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt +++ b/compiler/src/prog8/compiler/astprocessing/LiteralsToAutoVars.kt @@ -49,6 +49,8 @@ internal class LiteralsToAutoVars(private val program: Program, private val erro } } else { val arrayDt = array.guessDatatype(program) + if(arrayDt.isUnknown) + return noModifications val elementDt = ArrayToElementTypes.getValue(arrayDt.getOr(DataType.UNDEFINED)) val maxSize = when(elementDt) { in ByteDatatypesWithBoolean -> PtContainmentCheck.MAX_SIZE_FOR_INLINE_CHECKS_BYTE diff --git a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt index 9788e6f6a..f51e8f605 100644 --- a/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt +++ b/compiler/src/prog8/compiler/astprocessing/TypecastsAdder.kt @@ -1,6 +1,7 @@ package prog8.compiler.astprocessing import prog8.ast.IFunctionCall +import prog8.ast.INameScope import prog8.ast.Node import prog8.ast.Program import prog8.ast.base.FatalAstException @@ -363,6 +364,25 @@ class TypecastsAdder(val program: Program, val options: CompilationOptions, val return adjustRangeDts(range, fromConst, fromDt, toConst, toDt, varDt.getOr(DataType.UNDEFINED), parent) } + override fun after(array: ArrayLiteral, parent: Node): Iterable { + // Arrays can contain booleans, numbers, or address-ofs. + // if there is an identifier here (that is of a pass-by-reference type), take its address explicitly. + + for((index, elt) in array.value.withIndex()) { + if (elt is IdentifierReference) { + val eltType = elt.inferType(program) + val tgt = elt.targetStatement(program) + if(eltType.isPassByReference || tgt is Subroutine || tgt is Label || tgt is Block) { + val addressof = AddressOf(elt, null, elt.position) + addressof.linkParents(array) + array.value[index] = addressof + } + } + } + + return noModifications + } + private fun adjustRangeDts( range: RangeExpression, fromConst: NumericLiteral?, diff --git a/compiler/test/codegeneration/TestArrayThings.kt b/compiler/test/codegeneration/TestArrayThings.kt index ae1500f06..f9fd7ba8a 100644 --- a/compiler/test/codegeneration/TestArrayThings.kt +++ b/compiler/test/codegeneration/TestArrayThings.kt @@ -4,6 +4,7 @@ import io.kotest.core.spec.style.FunSpec import io.kotest.matchers.shouldBe import io.kotest.matchers.shouldNotBe import io.kotest.matchers.string.shouldContain +import io.kotest.matchers.types.instanceOf import prog8.code.StStaticVariable import prog8.code.SymbolTableMaker import prog8.code.ast.* @@ -456,6 +457,46 @@ main { array3.children.map { (it as PtNumber).number } shouldBe listOf(100, 101, 102) } + test("identifiers in array literals getting implicit address-of") { + val src=""" +main { + sub start() { +label: + str @shared name = "name" + uword[] @shared array1 = [name, label, start, main] + uword[] @shared array2 = [&name, &label, &start, &main] + } +}""" + val result = compileText(C64Target(), false, src, writeAssembly = true)!! + val x = result.codegenAst!!.entrypoint()!! + x.children.size shouldBe 5 + val array1 = (x.children[1] as PtVariable).value as PtArray + val array2 = (x.children[2] as PtVariable).value as PtArray + array1.children.forEach { + it shouldBe instanceOf() + } + array2.children.forEach { + it shouldBe instanceOf() + } + } + + test("variable identifiers in array literals not getting implicit address-of") { + val src=""" +main { + sub start() { +label: + str @shared name = "name" + ubyte @shared bytevar + uword[] @shared array1 = [cx16.r0] ; error, is variables + uword[] @shared array2 = [bytevar] ; error, is variables + } +}""" + val errors = ErrorReporterForTests() + compileText(C64Target(), false, src, writeAssembly = true, errors=errors) shouldBe null + errors.errors.size shouldBe 2 + errors.errors[0] shouldContain "contains non-constant" + errors.errors[1] shouldContain "contains non-constant" + } fun getTestOptions(): CompilationOptions { val target = VMTarget() diff --git a/docs/source/todo.rst b/docs/source/todo.rst index 7b274be0f..3febf0695 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,7 +1,6 @@ TODO ==== -- word arrays (after ast processing) should no longer contain identifiers, these should have been replaced by &identifier. - should the array-to-array assignment support be removed and instead require an explicit copy function call? What prog8_lib_arraycopy() now does. Or just use memcopy. - should we add a cleararray builtin function that can efficiently set every element in the array to the given value @@ -14,6 +13,7 @@ Maybe this routine can be made more intelligent. See usesOtherRegistersWhileEva Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ +- keep boolean array intact in IR so that it might be represented as a bitmask in the resulting code (8 times storage improvement) - improve detection that a variable is not read before being written so that initializing it to zero can be omitted (only happens now if a vardecl is immediately followed by a for loop for instance) - Improve the SublimeText syntax file for prog8, you can also install this for 'bat': https://github.com/sharkdp/bat?tab=readme-ov-file#adding-new-syntaxes--language-definitions - Can we support signed % (remainder) somehow? diff --git a/examples/test.p8 b/examples/test.p8 index d64713dbc..d4bd6a495 100644 --- a/examples/test.p8 +++ b/examples/test.p8 @@ -1,80 +1,11 @@ -%import floats -%import textio - -%option no_sysinit -%zeropage basicsafe - main { - - sub arrayinit_with_multiplier() { - str name = "xyz" * 3 - bool[3] boolarray = [true] * 3 - ubyte[3] bytearray = [42] * 3 - uword[3] wordarray = [5555] * 3 - float[3] floatarray = [123.45] * 3 - - txt.print(name) - txt.nl() - for cx16.r1L in 0 to 2 { - txt.print_bool(boolarray[cx16.r1L]) - txt.spc() - txt.print_ub(bytearray[cx16.r1L]) - txt.spc() - txt.print_uw(wordarray[cx16.r1L]) - txt.spc() - floats.print(floatarray[cx16.r1L]) - txt.nl() - } - txt.nl() - txt.nl() - } - - sub arrayinit_with_range() { - ubyte[3] bytearray2 = 10 to 12 - uword[3] wordarray2 = 5000 to 5002 - float[3] floatarray2 = 100 to 102 - - for cx16.r1L in 0 to 2 { - txt.print_ub(bytearray2[cx16.r1L]) - txt.spc() - txt.print_uw(wordarray2[cx16.r1L]) - txt.spc() - floats.print(floatarray2[cx16.r1L]) - txt.nl() - } - txt.nl() - txt.nl() - } - - sub arrayassign() { - bool[4] boolarray3 - ubyte[4] bytearray3 - uword[4] wordarray3 - float[4] floatarray3 - - boolarray3 = [true] *4 - bytearray3 = [42]*4 - wordarray3 = [999]*4 - wordarray3 = [&bytearray3]*4 - wordarray3 = [bytearray3]*4 - floatarray3 = [99.77]*4 - - for cx16.r1L in 0 to 2 { - txt.print_bool(boolarray3[cx16.r1L]) - txt.spc() - txt.print_ub(bytearray3[cx16.r1L]) - txt.spc() - txt.print_uw(wordarray3[cx16.r1L]) - txt.spc() - floats.print(floatarray3[cx16.r1L]) - txt.nl() - } - txt.nl() - } - sub start() { - arrayinit_with_multiplier() - arrayinit_with_range() - arrayassign() +label: + str @shared name = "name" + ubyte @shared bytevar + uword[] @shared array = [name, label, start, main, 9999] + uword[] @shared array2 = [&name, &label, &start, &main, 9999] + uword[] @shared array3 = [cx16.r0] ; error, is variables + uword[] @shared array4 = [bytevar] ; error, is variables } }