diff --git a/benchmark-program/b_textelite.p8 b/benchmark-program/b_textelite.p8 index 0223186a5..a3d2beac2 100644 --- a/benchmark-program/b_textelite.p8 +++ b/benchmark-program/b_textelite.p8 @@ -472,7 +472,7 @@ elite_galaxy { generate_next_planet() textelite.num_commands++ } - elite_planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), elite_planet.name) init_market_for_planet() } @@ -494,7 +494,7 @@ elite_galaxy { ubyte pi for pi in 0 to 255 { generate_next_planet() - elite_planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), elite_planet.name) if elite_util.prefix_matches(nameptr, elite_planet.name) { ubyte distance = elite_planet.distance(x, y) if distance < current_distance { @@ -532,7 +532,7 @@ elite_galaxy { ; else ; txt.chrout('-') ; txt.spc() - elite_planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), elite_planet.name) elite_planet.display(true, distance) } pn++ @@ -548,7 +548,7 @@ elite_galaxy { str current_name = " " ; 8 max ubyte pn = 0 - current_name = elite_planet.name + void strings.copy(elite_planet.name, current_name) init(number) ; txt.clear_screen() ; txt.print("Galaxy #") @@ -569,7 +569,7 @@ elite_galaxy { generate_next_planet() ubyte distance = elite_planet.distance(px, py) if distance <= max_distance { - elite_planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), elite_planet.name) elite_planet.name[0] = strings.upperchar(elite_planet.name[0]) uword tx = elite_planet.x uword ty = elite_planet.y diff --git a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt index c8ad2a428..ca0326bf0 100644 --- a/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt +++ b/codeGenCpu6502/src/prog8/codegen/cpu6502/assignment/AssignmentAsmGen.kt @@ -666,21 +666,8 @@ internal class AssignmentAsmGen( returnDt?.isByteOrBool==true -> assignRegisterByte(target, CpuRegister.A, returnDt.isSigned, false) // function's byte result is in A returnDt?.isWord==true -> assignRegisterpairWord(target, RegisterOrPair.AY) // function's word result is in AY returnDt==BaseDataType.STR -> { - val targetDt = target.datatype - when { - targetDt.isString -> { - asmgen.out(""" - tax - lda #<${target.asmVarname} - sta P8ZP_SCRATCH_W1 - lda #>${target.asmVarname} - sta P8ZP_SCRATCH_W1+1 - txa - jsr prog8_lib.strcpy""") - } - targetDt.isUnsignedWord -> assignRegisterpairWord(target, RegisterOrPair.AY) - else -> throw AssemblyError("str return value type mismatch with target") - } + if (target.datatype.isUnsignedWord) assignRegisterpairWord(target, RegisterOrPair.AY) + else throw AssemblyError("str return value type mismatch with target") } returnDt== BaseDataType.LONG -> { // longs are in R14:R15 (r14=lsw, r15=msw) @@ -3467,31 +3454,16 @@ $endLabel""") } private fun assignVariableString(target: AsmAssignTarget, varName: String) { - when(target.kind) { - TargetStorageKind.VARIABLE -> { - when { - target.datatype.isUnsignedWord -> { - asmgen.out(""" - lda #<$varName - ldy #>$varName - sta ${target.asmVarname} - sty ${target.asmVarname}+1""") - } - target.datatype.isString || target.datatype.isUnsignedByteArray || target.datatype.isByteArray -> { - asmgen.out(""" - lda #<${target.asmVarname} - ldy #>${target.asmVarname} - sta P8ZP_SCRATCH_W1 - sty P8ZP_SCRATCH_W1+1 - lda #<$varName - ldy #>$varName - jsr prog8_lib.strcpy""") - } - else -> throw AssemblyError("assign string to incompatible variable type") - } - } - else -> throw AssemblyError("string-assign to weird target") + if (target.kind == TargetStorageKind.VARIABLE) { + if (target.datatype.isUnsignedWord) { + asmgen.out(""" + lda #<$varName + ldy #>$varName + sta ${target.asmVarname} + sty ${target.asmVarname}+1""") + } else throw AssemblyError("assign string to incompatible variable type ${target.position}") } + else throw AssemblyError("string-assign to weird target ${target.position}") } private fun assignVariableLong(target: AsmAssignTarget, varName: String, sourceDt: DataType) { diff --git a/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt b/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt index b301032fc..aa1a95992 100644 --- a/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt +++ b/codeGenIntermediate/src/prog8/codegen/intermediate/AssignmentGen.kt @@ -123,6 +123,9 @@ internal class AssignmentGen(private val codeGen: IRCodeGen, private val express } internal fun translate(augAssign: PtAugmentedAssign): IRCodeChunks { + if(augAssign.target.type.isString) + throw AssemblyError("cannot assign to str type ${augAssign.position}") + // augmented assignment always has just a single target if (augAssign.target.children.single() is PtIrRegister) throw AssemblyError("assigning to a register should be done by just evaluating the expression into resultregister") @@ -467,7 +470,11 @@ internal class AssignmentGen(private val codeGen: IRCodeGen, private val express } private fun translateRegularAssign(assignment: PtAssignment): IRCodeChunks { - // note: assigning array and string values is done via an explicit memcopy/stringcopy function call. + if(assignment.target.type.isString) { + // assigning array and string values is done via an explicit memcopy/stringcopy function calls. + throw AssemblyError("cannot assign to str type ${assignment.position}") + } + val valueDt = irType(assignment.value.type) val targetDt = irType(assignment.target.type) val result = mutableListOf() diff --git a/compiler/res/prog8lib/cx16/diskio.p8 b/compiler/res/prog8lib/cx16/diskio.p8 index a909700e7..aaccac381 100644 --- a/compiler/res/prog8lib/cx16/diskio.p8 +++ b/compiler/res/prog8lib/cx16/diskio.p8 @@ -566,7 +566,7 @@ return_status: sub internal_f_open_w(str filename, bool open_for_seeks) -> bool { f_close_w() - list_filename = filename + void strings.copy(filename, list_filename) str modifier = ",s,?" modifier[3] = 'w' if open_for_seeks @@ -624,7 +624,7 @@ done: return list_filename io_error: - list_filename = "io error" + void strings.copy("io error", list_filename) goto done } diff --git a/compiler/res/prog8lib/shared_cbm_diskio.p8 b/compiler/res/prog8lib/shared_cbm_diskio.p8 index 6686f7770..7ed52d182 100644 --- a/compiler/res/prog8lib/shared_cbm_diskio.p8 +++ b/compiler/res/prog8lib/shared_cbm_diskio.p8 @@ -550,7 +550,7 @@ done: return list_filename io_error: - list_filename = "io error" + void strings.copy("io error", list_filename) goto done } diff --git a/compiler/res/prog8lib/strings.p8 b/compiler/res/prog8lib/strings.p8 index 6df73a842..445766094 100644 --- a/compiler/res/prog8lib/strings.p8 +++ b/compiler/res/prog8lib/strings.p8 @@ -213,8 +213,6 @@ _found tya asmsub copy(str source @R0, str target @AY) clobbers(A) -> ubyte @Y { ; Copy a string to another, overwriting that one. ; Returns the length of the string that was copied. - ; Often you don’t have to call this explicitly and can just write string1 = string2 - ; but this function is useful if you’re dealing with addresses for instance. %asm {{ sta P8ZP_SCRATCH_W1 sty P8ZP_SCRATCH_W1+1 diff --git a/compiler/res/prog8lib/virtual/diskio.p8 b/compiler/res/prog8lib/virtual/diskio.p8 index 2cd325af3..ca3d3870c 100644 --- a/compiler/res/prog8lib/virtual/diskio.p8 +++ b/compiler/res/prog8lib/virtual/diskio.p8 @@ -284,7 +284,7 @@ diskio { ; get the load adress from a PRG file (usually $0801 but it can be different) if f_open(filename) { uword address - f_read(&address, 2) + void f_read(&address, 2) f_close() return address } diff --git a/compiler/res/prog8lib/virtual/strings.p8 b/compiler/res/prog8lib/virtual/strings.p8 index 5c24858f5..92d238091 100644 --- a/compiler/res/prog8lib/virtual/strings.p8 +++ b/compiler/res/prog8lib/virtual/strings.p8 @@ -106,8 +106,6 @@ strings { sub copy(str source, str target) -> ubyte { ; Copy a string to another, overwriting that one. ; Returns the length of the string that was copied. - ; Often you don’t have to call this explicitly and can just write string1 = string2 - ; but this function is useful if you’re dealing with addresses for instance. %ir {{ loadm.w r99000,strings.copy.source loadm.w r99001,strings.copy.target diff --git a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt index 35d0fb15e..7d56c067f 100644 --- a/compiler/src/prog8/compiler/astprocessing/AstChecker.kt +++ b/compiler/src/prog8/compiler/astprocessing/AstChecker.kt @@ -651,6 +651,14 @@ internal class AstChecker(private val program: Program, } override fun visit(assignment: Assignment) { + if(assignment.target.inferType(program).isString) { + if(assignment.isAugmentable) + errors.err("cannot assign to string by value (use strings.copy or strings.append)", assignment.position) + else + errors.err("cannot assign to string by value (use strings.copy)", assignment.position) + return + } + if(assignment.target.multi==null) { val targetDt = assignment.target.inferType(program) val valueDt = assignment.value.inferType(program) @@ -1500,8 +1508,12 @@ internal class AstChecker(private val program: Program, } if(expr.operator=="+" || expr.operator=="-") { - if(leftDt.isString || rightDt.isString || leftDt.isArray || rightDt.isArray) { - errors.err("missing & (address-of) on the operand", expr.position) + if(leftDt.isString || leftDt.isArray) { + errors.err("missing & (address-of) on the operand", expr.left.position) + return + } + if(rightDt.isString || rightDt.isArray) { + errors.err("missing & (address-of) on the operand", expr.right.position) return } } diff --git a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt index fd24d85f7..4b370194b 100644 --- a/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt +++ b/compiler/src/prog8/compiler/astprocessing/StatementReorderer.kt @@ -298,15 +298,6 @@ internal class StatementReorderer( checkCopyArrayValue(assignment) } - if(!assignment.isAugmentable) { - if (targetType issimpletype BaseDataType.STR) { - if (valueType issimpletype BaseDataType.STR || valueType.getOrUndef() == DataType.pointer(BaseDataType.UBYTE)) { - // replace string assignment by a call to stringcopy - return copyStringValue(assignment) - } - } - } - return noModifications } @@ -372,19 +363,6 @@ internal class StatementReorderer( } } - private fun copyStringValue(assign: Assignment): List { - val identifier = assign.target.identifier!! - val strcopy = FunctionCallStatement(IdentifierReference(listOf("sys", "internal_stringcopy"), assign.position), - mutableListOf( - assign.value as? IdentifierReference ?: assign.value, - identifier - ), - false, - assign.position - ) - return listOf(IAstModification.ReplaceNode(assign, strcopy, assign.parent)) - } - override fun after(deref: ArrayIndexedPtrDereference, parent: Node): Iterable { if(parent is AssignTarget) { val zeroIndexer = deref.chain.firstOrNull { it.second?.constIndex()==0 } diff --git a/compiler/test/TestOptimization.kt b/compiler/test/TestOptimization.kt index d633de60d..576eafe52 100644 --- a/compiler/test/TestOptimization.kt +++ b/compiler/test/TestOptimization.kt @@ -1001,7 +1001,6 @@ main { test("no operand swap on logical expressions with shortcircuit evaluation") { val src=""" -%import diskio %zeropage basicsafe %option no_sysinit @@ -1009,15 +1008,23 @@ main { str scanline_buf = "?"* 20 sub start() { - if diskio.f_open("test.prg") and diskio.f_read(scanline_buf, 2)==2 + if f_open("test.prg") and f_read(scanline_buf, 2)==2 cx16.r0++ - if diskio.f_open("test.prg") or diskio.f_read(scanline_buf, 2)==2 + if f_open("test.prg") or f_read(scanline_buf, 2)==2 cx16.r0++ - if diskio.f_open("test.prg") xor diskio.f_read(scanline_buf, 2)==2 + if f_open("test.prg") xor f_read(scanline_buf, 2)==2 cx16.r0++ } + + sub f_open(str name) -> bool { + return cx16.r0==99 + } + + sub f_read(str buf, uword lengthy) -> uword { + return cx16.r0 + } }""" val result = compileText(Cx16Target(), true, src, outputDir, writeAssembly = false)!! val st = result.compilerAst.entrypoint.statements @@ -1025,15 +1032,15 @@ main { val ifCond1 = (st[0] as IfElse).condition as BinaryExpression val ifCond2 = (st[1] as IfElse).condition as BinaryExpression val ifCond3 = (st[2] as IfElse).condition as BinaryExpression - (ifCond1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_open") - (ifCond2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_open") - (ifCond3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_open") + (ifCond1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_open") + (ifCond2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_open") + (ifCond3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_open") val right1 = ifCond1.right as BinaryExpression val right2 = ifCond2.right as BinaryExpression val right3 = ifCond3.right as BinaryExpression - (right1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_read") - (right2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_read") - (right3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_read") + (right1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read") + (right2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read") + (right3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read") } test("eliminate same target register assignments") { diff --git a/compiler/test/ast/TestAstChecks.kt b/compiler/test/ast/TestAstChecks.kt index ec0980c3b..aadb03715 100644 --- a/compiler/test/ast/TestAstChecks.kt +++ b/compiler/test/ast/TestAstChecks.kt @@ -489,4 +489,37 @@ main { errors.errors[1] shouldContain "string var must be initialized with a string literal" errors.errors[2] shouldContain "string var must be initialized with a string literal" } + + test("cannot assign strings by-value") { + val src=""" +main { + + sub start() { + str n1 = "irmen" + str n2 = "de jong 12345" + + n1 = n2 + n1 = "zsdfzsdf" + n1 = 0 + n2 = 9999 + + void foo(n1) + } + + sub foo(str arg) -> str { + arg = "bar" + arg = 0 + arg = 9999 + return arg + } +}""" + + val errors = ErrorReporterForTests() + compileText(Cx16Target(), optimize=false, src, outputDir, writeAssembly=false, errors = errors) shouldBe null + errors.errors.size shouldBe 4 + errors.errors[0] shouldContain ":8:9: cannot assign to string" + errors.errors[1] shouldContain ":9:9: cannot assign to string" + errors.errors[2] shouldContain ":10:9: cannot assign to string" + errors.errors[3] shouldContain ":11:9: cannot assign to string" + } }) diff --git a/compiler/test/ast/TestVariousCompilerAst.kt b/compiler/test/ast/TestVariousCompilerAst.kt index 919f48c32..ba35bde1b 100644 --- a/compiler/test/ast/TestVariousCompilerAst.kt +++ b/compiler/test/ast/TestVariousCompilerAst.kt @@ -225,20 +225,21 @@ main { sub start() { str @shared name = "part1" + "part2" str @shared rept = "rep"*4 - const ubyte times = 3 - name = "xx1" + "xx2" - rept = "xyz" * (times+1) + test("xx1" + "xx2") + test("xyz" * 4) + } + sub test(str message) { } }""" val result = compileText(C64Target(), optimize=false, src, outputDir, writeAssembly=true)!! val stmts = result.compilerAst.entrypoint.statements - stmts.size shouldBe 6 + stmts.size shouldBe 5 val name1 = stmts[0] as VarDecl val rept1 = stmts[1] as VarDecl (name1.value as StringLiteral).value shouldBe "part1part2" (rept1.value as StringLiteral).value shouldBe "reprepreprep" - val name2strcopy = stmts[3] as IFunctionCall - val rept2strcopy = stmts[4] as IFunctionCall + val name2strcopy = stmts[2] as IFunctionCall + val rept2strcopy = stmts[3] as IFunctionCall val name2 = (name2strcopy.args.first() as AddressOf).identifier!! val rept2 = (rept2strcopy.args.first() as AddressOf).identifier!! (name2.targetVarDecl()!!.value as StringLiteral).value shouldBe "xx1xx2" diff --git a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt index 547c215e8..bddff2c7f 100644 --- a/compilerAst/src/prog8/ast/expressions/AstExpressions.kt +++ b/compilerAst/src/prog8/ast/expressions/AstExpressions.kt @@ -1765,6 +1765,8 @@ class PtrDereference( return resultType(target.datatype) if(target is StructFieldRef) return resultType(target.type) + if(target is Alias) + return target.target.inferType(program) TODO("infertype $chain -> $target") } @@ -1845,7 +1847,9 @@ class ArrayIndexedPtrDereference( InferredTypes.knownFor(fieldDt) } } else { - require(indexPosition==this.chain.size-1) {"when indexing primitive pointer type there cannot be a field after it"} + require(indexPosition==this.chain.size-1) { + "when indexing primitive pointer type there cannot be a field after it $position" + } return if(derefLast) InferredTypes.knownFor(target.datatype.dereference()) else diff --git a/docs/source/libraries.rst b/docs/source/libraries.rst index 0981cc3ee..bcc05cb7a 100644 --- a/docs/source/libraries.rst +++ b/docs/source/libraries.rst @@ -997,8 +997,6 @@ Provides string manipulation routines. ``copy (from, to) -> ubyte length`` Copy a string to another, overwriting that one. Returns the length of the string that was copied. - Often you don't have to call this explicitly and can just write ``string1 = string2`` - but this function is useful if you're dealing with addresses for instance. ``append (string, suffix) -> ubyte length`` Appends the suffix string to the other string (make sure the memory buffer is large enough!) diff --git a/docs/source/todo.rst b/docs/source/todo.rst index cb04fd733..4738673a9 100644 --- a/docs/source/todo.rst +++ b/docs/source/todo.rst @@ -1,19 +1,21 @@ TODO ==== +- add strings.ncopy and nappend (length limited) - fix/check github issues. +- docs: sort the routines in the library chapter alphabetically - redo the benchmark-c tests with final 12.0 release version. Future Things and Ideas ^^^^^^^^^^^^^^^^^^^^^^^ -- struct/ptr: implement the remaining TODO's in PointerAssignmentsGen. +- struct/ptr: implement the remaining TODOs in PointerAssignmentsGen. - struct/ptr: optimize deref in PointerAssignmentsGen: optimize 'forceTemporary' to only use a temporary when the offset is >0 - struct/ptr: optimize the float copying in assignIndexedPointer() (also word and long?) - struct/ptr: optimize augmented assignments to indexed pointer targets like sprptr[2]^^.y++ (these are now not performend in-place but as a regular assignment) - struct/ptr: implement even more struct instance assignments (via memcopy) in CodeDesugarer (see the TODO) (add to documentation as well, paragraph 'Structs') -- struct/ptr: support const pointers (simple and struct types) +- struct/ptr: support const pointers (simple and struct types) (make sure to change codegen properly in all cases, change remark about this limitation in docs too) - struct/ptr: support @nosplit pointer arrays? - struct/ptr: support pointer to pointer? - struct/ptr: support for typed function pointers? (&routine could be typed by default as well then) diff --git a/docs/source/variables.rst b/docs/source/variables.rst index e715493b8..9961e4ebc 100644 --- a/docs/source/variables.rst +++ b/docs/source/variables.rst @@ -434,14 +434,6 @@ You can concatenate two string literals using '+', which can be useful to split long strings over separate lines. But remember that the length of the total string still cannot exceed 255 characters. A string literal can also be repeated a given number of times using '*', where the repeat number must be a constant value. -And a new string value can be assigned to another string, but no bounds check is done! -So be sure the destination string is large enough to contain the new value (it is overwritten in memory):: - - str string1 = "first part" + "second part" - str string2 = "hello!" * 10 - - string1 = string2 - string1 = "new value" There are several escape sequences available to put special characters into your string value: diff --git a/examples/textelite.p8 b/examples/textelite.p8 index 017bca51e..1d02082d2 100644 --- a/examples/textelite.p8 +++ b/examples/textelite.p8 @@ -421,7 +421,7 @@ galaxy { repeat system { generate_next_planet() } - planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), planet.name) init_market_for_planet() } @@ -441,7 +441,7 @@ galaxy { ubyte pi for pi in 0 to 255 { generate_next_planet() - planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), planet.name) if util.prefix_matches(nameptr, planet.name) { ubyte distance = planet.distance(x, y) if distance < current_distance { @@ -479,7 +479,7 @@ galaxy { else txt.chrout('-') txt.spc() - planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), planet.name) planet.display(true, distance) } pn++ @@ -495,7 +495,7 @@ galaxy { str current_name = " " ; 8 max ubyte pn = 0 - current_name = planet.name + void strings.copy(planet.name, current_name) init(number) txt.clear_screen() txt.print("Galaxy #") @@ -516,7 +516,7 @@ galaxy { generate_next_planet() ubyte distance = planet.distance(px, py) if distance <= max_distance { - planet.name = make_current_planet_name() + void strings.copy(make_current_planet_name(), planet.name) planet.name[0] = strings.upperchar(planet.name[0]) uword tx = planet.x uword ty = planet.y