String.append is now by reference, StringCopy.append is by copy/linear

Adding memory leak tests to String.append and StringCopy.append

Issue #94

core/Bench.carp

@@ -18,10 +18,10 @@
   (hidden get-unit)
   (defn get-unit [n]
     (cond
-      (< n 1000.0) (String.append (Double.str n) @"ns")
-      (< n 1000000.0) (String.append (Double.str (/ n 1000.0)) @"µs")
-      (< n 1000000000.0) (String.append (Double.str (/ n 1000000.0)) @"ms")
-      (String.append (Double.str (/ n 1000000000.0)) @"s")))
+      (< n 1000.0) (StringCopy.append (Double.str n) @"ns")
+      (< n 1000000.0) (StringCopy.append (Double.str (/ n 1000.0)) @"µs")
+      (< n 1000000000.0) (StringCopy.append (Double.str (/ n 1000000.0)) @"ms")
+      (StringCopy.append (Double.str (/ n 1000000000.0)) @"s")))
 
   (private print)
   (hidden print)

core/Format.carp

@@ -6,7 +6,7 @@
     (if (= idx -1)
       (list 'copy s) ; no more splits found, just return string
       (if (= \% (String.char-at s (inc idx))) ; this is an escaped %
-        (list 'String.append
+        (list 'StringCopy.append
               (list 'copy "%")
               (fmt-internal (String.substring s (+ idx 2) len) args))
         (if (= 0 (count args)) ; we need to insert something, but have nothing
@@ -19,9 +19,9 @@
                 (macro-error "error in format string: too many arguments to format string")
                 (list 'format s (car args)))
               (let [slice (String.substring s 0 (+ (inc idx) next))]
-                (list 'String.append (list 'format slice (car args))
-                                     (fmt-internal (String.substring s (+ (inc idx) next) len)
-                                                   (cdr args)))))))))))
+                (list 'StringCopy.append (list 'format slice (car args))
+                                         (fmt-internal (String.substring s (+ (inc idx) next) len)
+                                                       (cdr args)))))))))))
 
 (doc fmt "fmt formats a string. It supports all of the string interpolations defined in format of the type that should be interpolated (e.g. %d and %x on integers).")
 (defmacro fmt [s :rest args]

core/Macros.carp

@@ -260,7 +260,7 @@
     (list "")
     (if (= (count forms) 1)
       (list 'str (car forms))
-      (list 'String.append (list 'str (car forms)) (build-str* (cdr forms))))))
+      (list 'StringCopy.append (list 'str (car forms)) (build-str* (cdr forms))))))
 
 (defmacro str* [:rest forms]
   (build-str* forms))

core/String.carp

@@ -3,7 +3,7 @@
 (defmodule String
 
   (register =          (Fn [&String &String] Bool))
-  (register append     (Fn [String String] String)) ;; TODO: should take &String:s
+  (register append     (Fn [&String &String] String))
   (register delete     (Fn [String] ()))
   (register copy       (Fn [&String] String))
   (register count      (Fn [&String] Int))
@@ -31,16 +31,16 @@
     (let [str @""]
       (do
         (for [i 0 n]
-          (set! str (append str @inpt)))
+          (set! str (append &str inpt)))
         str)))
 
   (defn pad-left [len pad s]
     (let [x (Int.max 0 (- len (count s)))]
-      (append (from-chars &(Array.replicate x &pad)) @s)))
+      (append &(from-chars &(Array.replicate x &pad)) s)))
 
   (defn pad-right [len pad s]
     (let [x (Int.max 0 (- len (count s)))]
-      (append @s (from-chars &(Array.replicate x &pad)))))
+      (append s &(from-chars &(Array.replicate x &pad)))))
 
   (doc count-char "Returns the number of occurrences of `c` in the string `s`.")
   (defn count-char [s c]
@@ -86,7 +86,7 @@
     (let-do [result @""
              len (Array.count strings)]
       (for [i 0 len]
-        (set! result (String.append result @(Array.nth strings i))))
+        (set! result (String.append &result (Array.nth strings i))))
       result))
 
   ;; TODO: Should use some kind of "StringBuilder" instead of generating intermediate strings.
@@ -97,12 +97,14 @@
       (for [i 0 len]
         (do
           (when (> i 0)
-            (set! result (String.append result @sep)))
-          (set! result (String.append result @(Array.nth strings i)))))
+            (set! result (String.append &result sep)))
+          (set! result (String.append &result (Array.nth strings i)))))
       result))
 )
 
 (defmodule StringCopy
+  (register append     (Fn [String String] String))
+
   (defn = [a b]
     (String.= &a &b))
 
@@ -113,7 +115,7 @@
 (defdynamic string-join- [strs]
   (if (= (count strs) 0)
     '(String.copy "")
-    (list 'String.append (car strs) (string-join- (cdr strs)))))
+    (list 'StringCopy.append (car strs) (string-join- (cdr strs)))))
 
 ;; TODO: Remove this function and replace uses with 'str*'
 (defmacro string-join [:rest strs]

core/Test.carp

@@ -33,7 +33,7 @@
           (if (String.= cname (Array.nth (Array.nth &color-table i) 0))
              (set! res @(Array.nth (Array.nth &color-table i) 1))
              ()))
-        (String.append @"\x1b[" (String.append res @"m")))))
+        (StringCopy.append @"\x1b[" (StringCopy.append res @"m")))))
 
 
   (defn color [cname]
@@ -85,11 +85,11 @@
         (if (Int.> (Int.+ passed failed) 0)
           (do
             (IO.color "green")
-            (if (Int.> passed 0) (IO.print &(String.append @"\t|" (String.repeat passed "="))) ())
+            (if (Int.> passed 0) (IO.print &(StringCopy.append @"\t|" (String.repeat passed "="))) ())
             (if (Int.= failed 0) (IO.print "|") ())
             (IO.color "red")
             (if (Int.= passed 0) (IO.print "\t|") ())
-            (if (Int.> failed 0) (IO.print &(String.append (String.repeat failed "=") @"|")) ())
+            (if (Int.> failed 0) (IO.print &(StringCopy.append (String.repeat failed "=") @"|")) ())
             (IO.println ""))
           ())
         (IO.color "green")

core/carp_pattern.h

@@ -584,11 +584,11 @@ String Pattern_internal_add_value(PatternMatchState *ms, String res, String src,
         }
         res = Pattern_internal_add_char(res, tr[i]);
       }
-      else if (tr[i] == '0') res = String_append(res, src);
+      else if (tr[i] == '0') res = StringCopy_append(res, src);
       else {
         Array a = {.len = 0, .capacity = 0, .data = NULL};
         Pattern_internal_push_onecapture(ms, tr[i] - '1', src, e, a);
-        res = String_append(res, ((String*)a.data)[0]);  /* add capture to accumulated result */
+        res = StringCopy_append(res, ((String*)a.data)[0]);  /* add capture to accumulated result */
       }
     }
   }

core/carp_string.h

@@ -75,12 +75,17 @@ bool String__EQ_(String *a, String *b) {
     return strcmp(*a, *b) == 0;
 }
 
-String String_append(String a, String b) {
-    int la = strlen(a);
-    int lb = strlen(b);
+String String_append(String *a, String *b) {
+    int la = strlen(*a);
+    int lb = strlen(*b);
     int total = la + lb + 1;
     String buffer = CARP_MALLOC(total);
-    snprintf(buffer, total, "%s%s", a, b);
+    snprintf(buffer, total, "%s%s", *a, *b);
+    return buffer;
+}
+
+String StringCopy_append(String a, String b) {
+    String buffer = String_append(&a, &b);
     CARP_FREE(a);
     CARP_FREE(b);
     return buffer;

docs/LanguageGuide.md

@@ -89,7 +89,7 @@ be used at a specific point in your program. In such cases the concept of 'holes
 add a hole in your source code and reload (":r") to let the Carp compiler figure out what type goes there.
 
 ```
-(String.append ?w00t @"!") ;; Will generate a type error telling you that the type of '?w00t' is String
+(StringCopy.append ?w00t @"!") ;; Will generate a type error telling you that the type of '?w00t' is String
 ```
 
 ### Special forms during evaluation of dynamic code

examples/basics.carp

@@ -12,7 +12,7 @@
 
 (defmodule Things
   (defn inside [s]
-    (let [msg (String.append s (String.copy "!"))]
+    (let [msg (StringCopy.append s (String.copy "!"))]
       (println (ref msg))))
   (defn call []
     (inside (String.copy "Hello"))))
@@ -131,7 +131,7 @@
 (defn print-last-string []
   (println &(get-last-string [(String.copy "NO") (String.copy "NO") (String.copy "YES")])))
 
-(defn exclaim [x] (String.append x @"!"))
+(defn exclaim [x] (StringCopy.append x @"!"))
 
 (deftype Simple [])
 (deftype Complex [x Int f Float d Double s String c Char])

test/array.carp

@@ -8,8 +8,8 @@
    [4 5 6]
    [7 8 9]])
 
-(defn excl [x] (String.append x @"!"))
-(defn excl-ref [x] (String.append @x @"!"))
+(defn excl [x] (StringCopy.append x @"!"))
+(defn excl-ref [x] (StringCopy.append @x @"!"))
 
 (defn inc-ref [x] (+ @x 1))
 

test/memory.carp

@@ -191,7 +191,7 @@
         (assert (= &[@"q" @"b" @"c"] &xs)))))
 
 (defn append-ref [a b]
-  (String.append @a @b))
+  (StringCopy.append @a @b))
 
 (defn array-reduce []
   (let [xs [@"a" @"b" @"c"]
@@ -317,6 +317,18 @@
         ys (Array.copy-map str-ref &xs)]
     (assert (= &[@"1" @"2" @"3" @"4"] &ys))))
 
+(defn string-append-leak-test []
+  (let [a "abcdef"
+        b "ghijklmnopqrstuvwxyz"]
+       (let [result (String.append a b)]
+            (assert (StringCopy.= result @"abcdefghijklmnopqrstuvwxyz")))))
+
+(defn stringcopy-append-leak-test []
+  (let [a "abcdef"
+        b "ghijklmnopqrstuvwxyz"]
+       (let [result (StringCopy.append @a @b)]
+            (assert (StringCopy.= result @"abcdefghijklmnopqrstuvwxyz")))))
+
 (defn main []
   (let []
     (with-test test
@@ -370,4 +382,6 @@
       (assert-no-leak test array-replicate "array-replicate does not leak")
       (assert-no-leak test array-copy-map-1 "array-copy-map-1 does not leak")
       (assert-no-leak test array-copy-map-2 "array-copy-map-2 does not leak")
+      (assert-no-leak test string-append-leak-test "String.append does not leak")
+      (assert-no-leak test stringcopy-append-leak-test "StringCopy.append does not leak")
       (print-test-results test))))

test/string.carp

@@ -221,7 +221,7 @@
     )
     (assert-equal test
                   "hello world"
-                  &(append @"hello " @"world")
+                  &(append "hello " "world")
                   "append works correctly"
     )
     (assert-equal test