strings can no longer be assigned by-value. Use strings.copy() instead. Fixes #189

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

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
+                    if (target.datatype.isUnsignedWord) assignRegisterpairWord(target, RegisterOrPair.AY)
-                    when {
+                    else throw AssemblyError("str return value type mismatch with target")
-                        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")
-                    }
                 }
                 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) {
+        if (target.kind == TargetStorageKind.VARIABLE) {
-            TargetStorageKind.VARIABLE -> {
+            if (target.datatype.isUnsignedWord) {
-                when {
+                asmgen.out("""
-                    target.datatype.isUnsignedWord -> {
+                    lda  #<$varName
-                        asmgen.out("""
+                    ldy  #>$varName
-                            lda  #<$varName
+                    sta  ${target.asmVarname}
-                            ldy  #>$varName
+                    sty  ${target.asmVarname}+1""")
-                            sta  ${target.asmVarname}
+            } else throw AssemblyError("assign string to incompatible variable type ${target.position}")
-                            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")
         }
+        else throw AssemblyError("string-assign to weird target ${target.position}")
     }
 
     private fun assignVariableLong(target: AsmAssignTarget, varName: String, sourceDt: DataType) {

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<IRCodeChunkBase>()

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
     }
 

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
     }
 

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

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
         }

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

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) {
+            if(leftDt.isString || leftDt.isArray) {
-                errors.err("missing & (address-of) on the operand", expr.position)
+                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
             }
         }

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<IAstModification> {
-        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<IAstModification> {
         if(parent is AssignTarget) {
             val zeroIndexer = deref.chain.firstOrNull { it.second?.constIndex()==0 }

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")
+        (ifCond1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_open")
-        (ifCond2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_open")
+        (ifCond2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_open")
-        (ifCond3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "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")
+        (right1.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read")
-        (right2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_read")
+        (right2.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read")
-        (right3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("diskio", "f_read")
+        (right3.left as FunctionCallExpression).target.nameInSource shouldBe listOf("f_read")
     }
 
     test("eliminate same target register assignments") {

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"
+    }
 })

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
+                test("xx1" + "xx2")
-                name = "xx1" + "xx2"
+                test("xyz" * 4)
-                rept = "xyz" * (times+1)
+            }
+            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 name2strcopy = stmts[2] as IFunctionCall
-            val rept2strcopy = stmts[4] 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"

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

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!)

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)

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:
 

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