String concatenation copies with the lengths it measured, not strcpy/strcat

tslang/include/TypeScript/Config.h

@@ -19,13 +19,6 @@
 #define ENABLE_ASYNC 1
 #define ENABLE_EXCEPTIONS 1
 
-#define USE_SPRINTF 1
-#ifndef WIN32
-#ifndef USE_SPRINTF
-#define USE_SPRINTF 1
-#endif
-#endif
-
 #define NUMBER_F64 1
 #define ANY_AS_DEFAULT 1
 // somehow it will error if set to true

tslang/include/TypeScript/LowerToLLVM/ConvertLogic.h

@@ -35,58 +35,6 @@ class ConvertLogic
     {
     }
 
-    mlir::Value itoa(mlir::Value value)
-    {
-        auto i8PtrTy = th.getPtrType();
-
-        auto _itoaFuncOp = ch.getOrInsertFunction(
-            "_itoa", th.getFunctionType(th.getPtrType(),
-                                        ArrayRef<mlir::Type>{rewriter.getI32Type(), th.getPtrType(), rewriter.getI32Type()}, true));
-
-        auto bufferSizeValue = clh.createI32ConstantOf(50);
-        // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true);
-        auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic);
-        auto base = clh.createI32ConstantOf(10);
-
-        return rewriter.create<LLVM::CallOp>(loc, _itoaFuncOp, ValueRange{value, newStringValue, base}).getResult();
-    }
-
-    mlir::Value i64toa(mlir::Value value)
-    {
-        auto i8PtrTy = th.getPtrType();
-
-        // 64-bit whatever the target: _i64toa's first parameter is `long long`
-        // (char *_i64toa(long long value, char *str, int radix)) - a fixed 64-bit integer on every
-        // target this compiler emits for, not a size_t.
-        auto _i64toaFuncOp = ch.getOrInsertFunction(
-            "_i64toa", th.getFunctionType(th.getPtrType(),
-                                          ArrayRef<mlir::Type>{rewriter.getI64Type(), th.getPtrType(), rewriter.getI32Type()}, true));
-
-        auto bufferSizeValue = clh.createI32ConstantOf(50);
-        // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true);
-        auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic);
-        auto base = clh.createI32ConstantOf(10);
-
-        return rewriter.create<LLVM::CallOp>(loc, _i64toaFuncOp, ValueRange{value, newStringValue, base}).getResult();
-    }
-
-    mlir::Value gcvt(mlir::Value in)
-    {
-        auto i8PtrTy = th.getPtrType();
-
-        auto _gcvtFuncOp = ch.getOrInsertFunction(
-            "_gcvt", th.getFunctionType(th.getPtrType(),
-                                        ArrayRef<mlir::Type>{rewriter.getF64Type(), rewriter.getI32Type(), th.getPtrType()}, true));
-
-        auto bufferSizeValue = clh.createI32ConstantOf(50);
-        // auto newStringValue = ch.Alloca(i8PtrTy, bufferSizeValue, true);
-        auto newStringValue = ch.MemoryAlloc(bufferSizeValue, MemoryAllocSet::Atomic);
-        auto doubleValue = rewriter.create<LLVM::FPExtOp>(loc, rewriter.getF64Type(), in);
-        auto precision = clh.createI32ConstantOf(16);
-
-        return rewriter.create<LLVM::CallOp>(loc, _gcvtFuncOp, ValueRange{doubleValue, precision, newStringValue}).getResult();
-    }
-
     mlir::Value sprintf(int buffSize, std::string format, mlir::Value value)
     {
         auto i8PtrTy = th.getPtrType();
@@ -167,20 +115,12 @@ class ConvertLogic
 
     mlir::Value intToString(mlir::Value value, int width, bool isSigned)
     {
-#ifndef USE_SPRINTF
-        return itoa(value);
-#else
         return sprintfOfInt(value, width, isSigned);
-#endif
     }
 
     mlir::Value f64ToString(mlir::Value value)
     {
-#ifndef USE_SPRINTF
-        return gcvt(value);
-#else
         return sprintfOfF64(value);
-#endif
     }
 };
 } // namespace typescript

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -837,21 +837,21 @@ class StringConcatOpLowering : public TsLlvmPattern<mlir_ts::StringConcatOp>
         auto llvmIndexType = tch.convertType(th.getIndexType());
 
         auto strlenFuncOp = ch.getOrInsertFunction("strlen", th.getFunctionType(llvmIndexType, {i8PtrTy}));
-        auto strcpyFuncOp = ch.getOrInsertFunction("strcpy", th.getFunctionType(i8PtrTy, {i8PtrTy, i8PtrTy}));
-        auto strcatFuncOp = ch.getOrInsertFunction("strcat", th.getFunctionType(i8PtrTy, {i8PtrTy, i8PtrTy}));
 
         SmallVector<mlir::Value> opers;
         for (auto oper : transformed.getOps())
         {
             opers.push_back(nullStringAsText(loc, oper, rewriter, tch));
         }
 
+        // each operand is measured once: the lengths size the buffer and bound every copy into it
+        SmallVector<mlir::Value> lengths;
         mlir::Value size = clh.createIndexConstantOf(llvmIndexType, 1);
-        // calc size
         for (auto oper : opers)
         {
-            auto size1 = rewriter.create<LLVM::CallOp>(loc, strlenFuncOp, oper);
-            size = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{size, size1.getResult()});
+            auto length = rewriter.create<LLVM::CallOp>(loc, strlenFuncOp, oper).getResult();
+            lengths.push_back(length);
+            size = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{size, length});
         }
 
         auto allocInStack = op.getAllocInStack().has_value() && op.getAllocInStack().value()
@@ -860,25 +860,18 @@ class StringConcatOpLowering : public TsLlvmPattern<mlir_ts::StringConcatOp>
         mlir::Value newStringValue = allocInStack ? ch.Alloca(th.getI8Type(), size, true)
                                                   : ch.MemoryAlloc(size);
 
-        // copy
-        auto concat = false;
-        auto result = newStringValue;
-        for (auto oper : opers)
+        // copy: each operand at its offset, without its terminator, then one terminator at the end
+        mlir::Value offset = clh.createIndexConstantOf(llvmIndexType, 0);
+        for (auto [oper, length] : llvm::zip(opers, lengths))
         {
-            if (concat)
-            {
-                auto callResult = rewriter.create<LLVM::CallOp>(loc, strcatFuncOp, ValueRange{result, oper});
-                result = callResult.getResult();
-            }
-            else
-            {
-                auto callResult = rewriter.create<LLVM::CallOp>(loc, strcpyFuncOp, ValueRange{result, oper});
-                result = callResult.getResult();
-            }
-
-            concat = true;
+            auto dest = rewriter.create<LLVM::GEPOp>(loc, i8PtrTy, th.getI8Type(), newStringValue, ValueRange{offset});
+            rewriter.create<LLVM::MemcpyOp>(loc, dest, oper, length, /*isVolatile=*/false);
+            offset = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{offset, length});
         }
 
+        auto end = rewriter.create<LLVM::GEPOp>(loc, i8PtrTy, th.getI8Type(), newStringValue, ValueRange{offset});
+        rewriter.create<LLVM::StoreOp>(loc, clh.createI8ConstantOf(0), end);
+
         rewriter.replaceOp(op, ValueRange{newStringValue});
 
         return success();

tslang/lib/TypeScript/TypeScriptOps.cpp

@@ -76,7 +76,7 @@ bool mlir_ts::isEmpty(mlir::Region &condtion)
     return false;
 }
 
-// Only the printing conversions do: `ConvertLogic`'s itoa and f64ToString, and `ts.CharToString`,
+// Only the printing conversions do: `ConvertLogic`'s intToString and f64ToString, and `ts.CharToString`,
 // each allocate a buffer and write into it. Everything else that reaches the plain cast - a
 // boolean, `undefined`, a string literal - hands back a global, which is immortal and owns
 // nothing. A literal is asked about by its element type, since that is what the lowering unwraps

tslang/test/tester/CMakeLists.txt

@@ -359,6 +359,7 @@ tslang_add_test(NAME test-compile-00-for-optional-class-condition COMMAND test-r
 tslang_add_test(NAME test-compile-00-for-condition-narrowing COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_condition_narrowing.ts")
 tslang_add_test(NAME test-compile-00-param-assigned-owned COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00param_assigned_owned.ts")
 tslang_add_test(NAME test-compile-00-string-empty-falsy COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts")
+tslang_add_test(NAME test-compile-00-string-concat-bounds COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts")
 tslang_add_test(NAME test-compile-00-narrowed-assign-other-member COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts")
 tslang_add_test(NAME test-compile-00-const-record-owned-fields COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-compile-00-array-length-valid COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
@@ -876,6 +877,7 @@ tslang_add_test(NAME test-jit-00-for-optional-class-condition COMMAND test-runne
 tslang_add_test(NAME test-jit-00-for-condition-narrowing COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_condition_narrowing.ts")
 tslang_add_test(NAME test-jit-00-param-assigned-owned COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00param_assigned_owned.ts")
 tslang_add_test(NAME test-jit-00-string-empty-falsy COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_empty_falsy.ts")
+tslang_add_test(NAME test-jit-00-string-concat-bounds COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00string_concat_bounds.ts")
 tslang_add_test(NAME test-jit-00-narrowed-assign-other-member COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00narrowed_assign_other_member.ts")
 tslang_add_test(NAME test-jit-00-const-record-owned-fields COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_record_owned_fields.ts")
 tslang_add_test(NAME test-jit-00-array-length-valid COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_length_valid.ts")
@@ -1722,6 +1724,7 @@ set(TSLANG_CORPUS
     00for_condition_narrowing.ts
     00param_assigned_owned.ts
     00string_empty_falsy.ts
+    00string_concat_bounds.ts
     00narrowed_assign_other_member.ts
     00const_record_owned_fields.ts
     00array_length_valid.ts

tslang/test/tester/tests/00string_concat_bounds.ts

@@ -0,0 +1,45 @@
+// `+` on strings copies each operand at its own offset with the length it measured, then ends the
+// result with one terminator: empty and null operands, many operands, a result built in a loop,
+// and results kept on the stack or the heap all hold exactly the bytes of their operands
+function concat3(a: string, b: string, c: string) {
+    return a + b + c;
+}
+
+function build(count: number) {
+    let s = "";
+    for (let i = 0; i < count; i++) {
+        s = s + "ab" + "";
+    }
+
+    return s;
+}
+
+function main() {
+    const empty = "";
+    const a = "a";
+    const bc = "bc";
+
+    assert(concat3(a, bc, "def") == "abcdef", "three operands");
+    assert(concat3(a, bc, "def").length == 6, "three operands: length");
+    assert(concat3(empty, empty, empty) == "", "all empty");
+    assert(concat3(empty, empty, empty).length == 0, "all empty: length");
+    assert(concat3(empty, bc, empty) == "bc", "empty around a string");
+    assert(concat3(a, empty, a) == "aa", "empty between strings");
+
+    const many = a + bc + a + bc + a + bc + a + bc + a + bc;
+    assert(many == "abcabcabcabcabc", "ten operands");
+    assert(many.length == 15, "ten operands: length");
+
+    const n: string | null = null;
+    assert(a + n == "anull", "null operand prints as null");
+    assert((n + empty).length == 4, "null then empty: length");
+
+    const built = build(100);
+    assert(built.length == 200, "built in a loop: length");
+    assert(build(3) == "ababab", "built in a loop: content");
+
+    const withNumber = a + 12 + bc;
+    assert(withNumber == "a12bc", "number operand");
+
+    print("done.");
+}