values: Split lists into read-only lists and editable lists

This is analogous to dictionaries and ensures that no circular references
can be created when using the exported API in apfl.h.

This also changes apfl_value_copy into apfl_value_incref to better reflect
what it does and to reflect that it is no longer an operation that can
fail.

src/apfl.h

@@ -495,18 +495,6 @@ struct apfl_expr apfl_parser_get_expr(apfl_parser_ptr);
 struct apfl_ctx_data;
 typedef struct apfl_ctx_data *apfl_ctx;
 
-typedef unsigned apfl_refcount;
-
-struct apfl_list {
-    // TODO: Not sure if it's a good idea to expose this struct?
-
-    apfl_refcount refcount;
-    struct apfl_value *items;
-    size_t len;
-    size_t cap;
-};
-
-
 enum apfl_value_type {
     APFL_VALUE_NIL,
     APFL_VALUE_BOOLEAN,
@@ -517,9 +505,16 @@ enum apfl_value_type {
     // TODO: functions/closures
 };
 
+struct apfl_list_data;
+typedef struct apfl_list_data *apfl_list;
+
+apfl_list apfl_list_incref(apfl_list);
+void apfl_list_unref(apfl_list);
+
 struct apfl_dict_data;
 typedef struct apfl_dict_data *apfl_dict;
 
+apfl_dict apfl_dict_incref(apfl_dict);
 void apfl_dict_unref(apfl_dict);
 
 struct apfl_value {
@@ -528,17 +523,32 @@ struct apfl_value {
         bool boolean;
         apfl_number number;
         apfl_refcounted_string string;
-        struct apfl_list *list;
+        apfl_list list;
         apfl_dict dict;
     };
 };
 
 bool apfl_value_eq(const struct apfl_value, const struct apfl_value);
 struct apfl_value apfl_value_move(struct apfl_value *src);
-bool apfl_value_copy(struct apfl_value *dst, struct apfl_value src);
+struct apfl_value apfl_value_incref(struct apfl_value);
 void apfl_value_print(struct apfl_value, FILE *);
 void apfl_value_deinit(struct apfl_value *);
 
+struct apfl_editable_list_data;
+typedef struct apfl_editable_list_data *apfl_editable_list;
+
+apfl_editable_list apfl_editable_list_new(void);
+apfl_editable_list apfl_editable_list_new_from_list(apfl_list);
+bool apfl_editable_list_append(apfl_editable_list, struct apfl_value);
+bool apfl_editable_list_append_list(apfl_editable_list, apfl_list);
+void apfl_editable_list_destroy(apfl_editable_list);
+
+/* Finalize the list and return a non-editable list.
+ * This also destroys the editable list object, so it's no longer safe to use it.
+ * Returns NULL on failure
+ */
+apfl_list apfl_editable_list_finalize(apfl_editable_list);
+
 struct apfl_editable_dict_data;
 typedef struct apfl_editable_dict_data *apfl_editable_dict;
 

src/eval.c

@@ -81,7 +81,7 @@ evaluate_constant(struct apfl_expr_const *constant)
 }
 
 static struct apfl_result
-evaluate_list_expr_to_list(apfl_ctx ctx, struct apfl_expr *expr, struct apfl_list *out)
+evaluate_list_expr_to_list(apfl_ctx ctx, struct apfl_expr *expr, apfl_editable_list elist)
 {
     struct apfl_result result = evaluate(ctx, expr);
     if (result.type != APFL_RESULT_OK) {
@@ -93,39 +93,24 @@ evaluate_list_expr_to_list(apfl_ctx ctx, struct apfl_expr *expr, struct apfl_lis
         return (struct apfl_result) { .type = APFL_RESULT_ERR };
     }
 
-    struct apfl_list *list = result.value.list;
+    apfl_list list = apfl_list_incref(result.value.list);
+    apfl_value_deinit(&result.value);
 
-    if (!apfl_resizable_ensure_cap_for_more_elements(
-        sizeof(struct apfl_value),
-        (void **)&out->items,
-        out->len,
-        &out->cap,
-        list->len
-    )) {
-        apfl_value_deinit(&result.value);
+    if (!apfl_editable_list_append_list(elist, list)) {
         return fatal();
     }
 
-    for (size_t i = 0; i < list->len; i++) {
-        if (!apfl_value_copy(&out->items[out->len], list->items[i])) {
-            apfl_value_deinit(&result.value);
-            return fatal();
-        }
-        out->len++;
-    }
-
-    apfl_value_deinit(&result.value);
     return (struct apfl_result) { .type = APFL_RESULT_OK, .value.type = APFL_VALUE_NIL };
 }
 
 static struct apfl_result
-evaluate_expr_list_into_list(apfl_ctx ctx, struct apfl_expr_list *list, struct apfl_list *out)
+evaluate_expr_list_into_list(apfl_ctx ctx, struct apfl_expr_list *list, apfl_editable_list elist)
 {
     for (size_t i = 0; i < list->len; i++) {
         struct apfl_expr_list_item *item = &list->items[i];
 
         if (item->expand) {
-            struct apfl_result result = evaluate_list_expr_to_list(ctx, item->expr, out);
+            struct apfl_result result = evaluate_list_expr_to_list(ctx, item->expr, elist);
             if (result.type != APFL_RESULT_OK) {
                 return result;
             }
@@ -135,15 +120,7 @@ evaluate_expr_list_into_list(apfl_ctx ctx, struct apfl_expr_list *list, struct a
                 return result;
             }
 
-            if (!apfl_resizable_append(
-                sizeof(struct apfl_value),
-                (void **)&out->items,
-                &out->len,
-                &out->cap,
-                &result.value,
-                1
-            )) {
-                apfl_value_deinit(&result.value);
+            if (!apfl_editable_list_append(elist, apfl_value_move(&result.value))) {
                 return fatal();
             }
         }
@@ -155,17 +132,22 @@ evaluate_expr_list_into_list(apfl_ctx ctx, struct apfl_expr_list *list, struct a
 static struct apfl_result
 evaluate_list(apfl_ctx ctx, struct apfl_expr_list *list)
 {
-    struct apfl_list *out = apfl_list_new();
-    if (out == NULL) {
+    apfl_editable_list elist = apfl_editable_list_new();
+    if (elist == NULL) {
         return fatal();
     }
 
-    struct apfl_result result = evaluate_expr_list_into_list(ctx, list, out);
+    struct apfl_result result = evaluate_expr_list_into_list(ctx, list, elist);
     if (result.type != APFL_RESULT_OK) {
-        DESTROY(out, apfl_list_deinit);
+        apfl_editable_list_destroy(elist);
         return result;
     }
 
+    apfl_list out = apfl_editable_list_finalize(elist);
+    if (out == NULL) {
+        return fatal();
+    }
+
     return (struct apfl_result) {
         .type = APFL_RESULT_OK,
         .value = {

src/hashmap.c

@@ -85,7 +85,7 @@ destroy_value(apfl_hashmap map, void *value)
 }
 
 static bool
-copy_key(apfl_hashmap map, void *dest, const void *src)
+copy_key(apfl_hashmap map, void *dest, void *src)
 {
     if (HAS_CALLBACK(map, copy_key)) {
         return INVOKE_CALLBACK(map, copy_key, dest, src);
@@ -96,7 +96,7 @@ copy_key(apfl_hashmap map, void *dest, const void *src)
 }
 
 static bool
-copy_value(apfl_hashmap map, void *dest, const void *src)
+copy_value(apfl_hashmap map, void *dest, void *src)
 {
     if (HAS_CALLBACK(map, copy_value)) {
         return INVOKE_CALLBACK(map, copy_value, dest, src);
@@ -133,7 +133,7 @@ find_key_in_bucket(const apfl_hashmap map, struct bucket *bucket, const void *ke
 }
 
 static bool
-set_in_bucket(apfl_hashmap map, struct bucket *bucket, const void *key, const void *value)
+set_in_bucket(apfl_hashmap map, struct bucket *bucket, void *key, void *value)
 {
     size_t keysize = map->keysize;
     size_t valsize = map->valsize;
@@ -289,7 +289,7 @@ apfl_hashmap_get(const apfl_hashmap map, const void *key, void *value)
 }
 
 bool
-apfl_hashmap_set(apfl_hashmap map, const void *key, const void *value)
+apfl_hashmap_set(apfl_hashmap map, void *key, void *value)
 {
     return set_in_bucket(map, bucket_by_key(map, key), key, value);
 }

src/hashmap.h

@@ -36,11 +36,11 @@ struct apfl_hashmap_callbacks {
 
     // Copies a key. Returns true on success, false on failure.
     // If not provided, the bytes will be copied with memcpy.
-    bool      (*copy_key)     (void *opaque, void *dest, const void *src);
+    bool      (*copy_key)     (void *opaque, void *dest, void *src);
 
     // Copies a value. Returns true on success, false on failure.
     // If not provided, the bytes will be copied with memcpy.
-    bool      (*copy_value)   (void *opaque, void *dest, const void *src);
+    bool      (*copy_value)   (void *opaque, void *dest, void *src);
 };
 
 apfl_hash apfl_hash_fnv1a_add(const void *, size_t len, apfl_hash);
@@ -49,7 +49,7 @@ apfl_hash apfl_hash_fnv1a(const void *, size_t len);
 apfl_hashmap apfl_hashmap_new(struct apfl_hashmap_callbacks, size_t keysize, size_t valsize);
 void apfl_hashmap_delete(apfl_hashmap, const void *key);
 bool apfl_hashmap_get(const apfl_hashmap, const void *key, void *value);
-bool apfl_hashmap_set(apfl_hashmap, const void *key, const void *value);
+bool apfl_hashmap_set(apfl_hashmap, void *key, void *value);
 size_t apfl_hashmap_count(const apfl_hashmap);
 void apfl_hashmap_destroy(apfl_hashmap);
 

src/internal.h

@@ -45,8 +45,6 @@ extern "C" {
 // Internal use only functions
 
 void apfl_print_indented(unsigned indent, FILE *, const char* fmt, ...);
-struct apfl_list *apfl_list_new(void);
-void apfl_list_deinit(struct apfl_list *);
 
 /* Decrease a refererence count. Returns true, if the object should be freed.
  */

src/value.c

@@ -2,8 +2,15 @@
 
 #include "apfl.h"
 #include "internal.h"
+#include "resizable.h"
 #include "hashmap.h"
 
+struct apfl_list_data {
+    unsigned refcount;
+    struct apfl_value *items;
+    size_t len;
+};
+
 struct apfl_dict_data {
     unsigned refcount;
     apfl_hashmap map;
@@ -11,6 +18,31 @@ struct apfl_dict_data {
 
 static apfl_hash value_hash(const struct apfl_value);
 
+apfl_list
+apfl_list_incref(apfl_list list)
+{
+    if (list == NULL) {
+        return NULL;
+    }
+    list->refcount++;
+    return list;
+}
+
+void
+apfl_list_unref(apfl_list list)
+{
+    if (list == NULL || !apfl_refcount_dec(&list->refcount)) {
+        return;
+    }
+
+    for (size_t i = 0; i < list->len; i++) {
+        apfl_value_deinit(&list->items[i]);
+    }
+
+    free(list->items);
+    free(list);
+}
+
 static bool
 dict_keys_eq(void *opaque, const void *a, const void *b)
 {
@@ -33,10 +65,16 @@ dict_destroy_key_or_value(void *opaque, void *kv)
 }
 
 static bool
-dict_copy_key_or_value(void *opaque, void *dest, const void *src)
+dict_copy_key_or_value(void *opaque, void *_dest, void *_src)
 {
     (void)opaque;
-    return apfl_value_copy(dest, *(struct apfl_value *)src);
+
+    struct apfl_value *dest = _dest;
+    struct apfl_value *src = _src;
+
+    *dest = apfl_value_incref(*src);
+
+    return true;
 }
 
 static const struct apfl_hashmap_callbacks dict_hashmap_callbacks = {
@@ -59,6 +97,16 @@ new_hashmap_for_dict(void)
     );
 }
 
+apfl_dict
+apfl_dict_incref(apfl_dict dict)
+{
+    if (dict == NULL) {
+        return NULL;
+    }
+    dict->refcount++;
+    return dict;
+}
+
 void
 apfl_dict_unref(apfl_dict dict)
 {
@@ -149,36 +197,29 @@ apfl_value_move(struct apfl_value *src)
     return out;
 }
 
-bool
-apfl_value_copy(struct apfl_value *dst, struct apfl_value src)
+struct apfl_value
+apfl_value_incref(struct apfl_value value)
 {
-    *dst = src;
-
-    switch (src.type) {
+    switch (value.type) {
     case APFL_VALUE_NIL:
     case APFL_VALUE_BOOLEAN:
     case APFL_VALUE_NUMBER:
         // Nothing to do
-        goto ok;
+        return value;
     case APFL_VALUE_STRING:
-        dst->string = apfl_refcounted_string_incref(src.string);
-        goto ok;
+        value.string = apfl_refcounted_string_incref(value.string);
+        return value;
     case APFL_VALUE_LIST:
-        assert(src.list != NULL);
-        assert(dst->list == src.list);
-        src.list->refcount++;
-        goto ok;
+        value.list = apfl_list_incref(value.list);
+        return value;
     case APFL_VALUE_DICT:
-        assert(src.dict != NULL);
-        assert(dst->dict == src.dict);
-        src.dict->refcount++;
-        goto ok;
+        value.dict = apfl_dict_incref(value.dict);
+        return value;
     }
 
     assert(false);
 
-ok:
-    return true;
+    return value;
 }
 
 void
@@ -189,7 +230,7 @@ apfl_value_print(struct apfl_value value, FILE *out)
 }
 
 static bool
-list_eq(const struct apfl_list *a, const struct apfl_list *b)
+list_eq(apfl_list a, apfl_list b)
 {
     if (a == b) {
         return true;
@@ -293,31 +334,138 @@ apfl_value_eq(const struct apfl_value a, const struct apfl_value b)
     return false;
 }
 
-struct apfl_list *
-apfl_list_new(void)
+struct apfl_editable_list_data {
+    struct apfl_value *items;
+    size_t len;
+    size_t cap;
+};
+
+apfl_editable_list
+apfl_editable_list_new(void)
+{
+    apfl_editable_list elist = ALLOC(struct apfl_editable_list_data);
+    if (elist == NULL) {
+        return NULL;
+    }
+
+    elist->items = NULL;
+    elist->len = 0;
+    elist->cap = 0;
+
+    return elist;
+}
+
+apfl_editable_list
+apfl_editable_list_new_from_list(apfl_list list)
 {
-    struct apfl_list *list = ALLOC(struct apfl_list);
     if (list == NULL) {
         return NULL;
     }
 
-    *list = (struct apfl_list) {
-        .refcount = 1,
-        .items = NULL,
-        .len = 0,
-        .cap = 0,
-    };
+    apfl_editable_list elist = ALLOC(struct apfl_editable_list_data);
+    if (elist == NULL) {
+        apfl_list_unref(list);
+        return NULL;
+    }
+
+    if (list->refcount == 1) {
+        // We're the only one with a reference to the list, so we can directly use the items of the list!
+        elist->items = list->items;
+        elist->len = list->len;
+        elist->cap = list->len;
+    } else {
+        // We fisrt need to create a shallow copy of the elements
+        if ((elist->items = ALLOC_LIST(struct apfl_value, list->len)) == NULL) {
+            apfl_list_unref(list);
+            free(elist);
+            return NULL;
+        }
+
+        for (size_t i = 0; i < list->len; i++) {
+            elist->items[i] = apfl_value_incref(list->items[i]);
+        }
+
+        elist->len = list->len;
+        elist->cap = list->len;
+    }
+
+    apfl_list_unref(list);
+
+    return elist;
+}
+
+static bool
+editable_list_append_values(apfl_editable_list elist, struct apfl_value *values, size_t len)
+{
+    if (!apfl_resizable_ensure_cap_for_more_elements(
+        sizeof(struct apfl_value),
+        (void **)&elist->items,
+        elist->len,
+        &elist->cap,
+        len
+    )) {
+        return false;
+    }
+
+    for (size_t i = 0; i < len; i++) {
+        elist->items[elist->len] = apfl_value_incref(values[i]);
+        elist->len++;
+    }
+
+    return true;
+}
+
+bool
+apfl_editable_list_append(apfl_editable_list elist, struct apfl_value value)
+{
+    bool ok = editable_list_append_values(elist, &value, 1);
+    apfl_value_deinit(&value);
+    return ok;
+}
+
+bool
+apfl_editable_list_append_list(apfl_editable_list elist, apfl_list list)
+{
+    bool ok = editable_list_append_values(elist, list->items, list->len);
+    apfl_list_unref(list);
+    return ok;
+}
+
+void
+apfl_editable_list_destroy(apfl_editable_list elist)
+{
+    if (elist == NULL) {
+        return;
+    }
+
+    if (elist->items != NULL) {
+        for (size_t i = 0; i < elist->len; i++) {
+            apfl_value_deinit(&elist->items[i]);
+        }
+        free(elist->items);
+    }
+
+    free(elist);
+}
+
+apfl_list
+apfl_editable_list_finalize(apfl_editable_list elist)
+{
+    apfl_list list = ALLOC(struct apfl_list_data);
+    if (list == NULL) {
+        apfl_editable_list_destroy(elist);
+        return NULL;
+    }
+
+    list->refcount = 1;
+    list->items = elist->items; // TODO: Maybe shrink memory with realloc?
+    list->len = elist->len;
+
+    free(elist);
 
     return list;
 }
 
-void
-apfl_list_deinit(struct apfl_list *list)
-{
-    DEINIT_LIST(list->items, list->len, apfl_value_deinit);
-    list->refcount = 0;
-    list->cap = 0;
-}
 
 struct apfl_editable_dict_data {
     apfl_hashmap map;
@@ -348,6 +496,7 @@ apfl_editable_dict_new_from_dict(apfl_dict dict)
 
     apfl_editable_dict ed = ALLOC(struct apfl_editable_dict_data);
     if (ed == NULL) {
+        apfl_dict_unref(dict);
         return NULL;
     }
 
@@ -358,6 +507,7 @@ apfl_editable_dict_new_from_dict(apfl_dict dict)
         // There are other places referencing the dict, we need to create a shallow copy first.
         if ((ed->map = apfl_hashmap_copy(dict->map)) == NULL) {
             free(ed);
+            apfl_dict_unref(dict);
             return NULL;
         }
     }
@@ -436,9 +586,8 @@ apfl_value_deinit(struct apfl_value *value)
         value->string = NULL;
         goto ok;
     case APFL_VALUE_LIST:
-        if (apfl_refcount_dec(&value->list->refcount)) {
-            DESTROY(value->list, apfl_list_deinit);
-        }
+        apfl_list_unref(value->list);
+        value->list = NULL;
         goto ok;
     case APFL_VALUE_DICT:
         apfl_dict_unref(value->dict);