From 6add5e47b911b8c82d11172befa5d26ca596e2d0 Mon Sep 17 00:00:00 2001 From: Laria Carolin Chabowski Date: Mon, 11 Apr 2022 23:31:20 +0200 Subject: [PATCH] hashmap: Improve cursor / peek API - peek functions will now return pointers directly - cursor access methods can now return an error, if the cursor is already at the end Also rewrote some cursor loops to use the HASHMAP_EACH macro. --- src/context.c | 8 +++---- src/gc.c | 2 +- src/hashmap.c | 55 +++++++++++++++++++++++----------------------- src/hashmap.h | 14 +++++++----- src/hashmap_test.c | 21 ++++++------------ src/value.c | 44 ++++++++++++++++++------------------- 6 files changed, 70 insertions(+), 74 deletions(-) diff --git a/src/context.c b/src/context.c index fd4bde0..2b3184a 100644 --- a/src/context.c +++ b/src/context.c @@ -239,12 +239,12 @@ void apfl_gc_scope_traverse(struct scope *scope, gc_visitor cb, void *opaque) { HASHMAP_EACH(&scope->map, cur) { - struct apfl_string **k; - apfl_hashmap_cursor_peek_key(cur, (void **)&k); + struct apfl_string **k = apfl_hashmap_cursor_peek_key(cur); + assert(k != NULL); cb(opaque, GC_OBJECT_FROM(*k, GC_TYPE_STRING)); - variable *v; - apfl_hashmap_cursor_peek_value(cur, (void **)&v); + variable *v = apfl_hashmap_cursor_peek_value(cur); + assert(v != NULL); cb(opaque, GC_OBJECT_FROM(*v, GC_TYPE_VAR)); } } diff --git a/src/gc.c b/src/gc.c index d8804e8..63df428 100644 --- a/src/gc.c +++ b/src/gc.c @@ -238,7 +238,7 @@ visit_roots(struct gc *gc, gc_visitor visitor, void *opaque) { HASHMAP_EACH(&gc->roots, cur) { struct gc_object *obj; - apfl_hashmap_cursor_get_key(cur, &obj); + assert(apfl_hashmap_cursor_get_key(cur, &obj)); visitor(opaque, obj); } diff --git a/src/hashmap.c b/src/hashmap.c index 3f2e222..2bcc730 100644 --- a/src/hashmap.c +++ b/src/hashmap.c @@ -210,28 +210,23 @@ prepare_set_in_bucket( return true; } -static bool -peek_in_bucket(const struct apfl_hashmap map, struct apfl_hashmap_bucket *bucket, const void *key, void **value_ptr) +static void * +peek_in_bucket(const struct apfl_hashmap map, struct apfl_hashmap_bucket *bucket, const void *key) { size_t i; if (!find_key_in_bucket(map, bucket, key, &i)) { - return false; + return NULL; } - if (value_ptr != NULL) { - size_t valsize = map.valsize; - *value_ptr = KVADDR(bucket->values, valsize, i); - } - - return true; + return KVADDR(bucket->values, map.valsize, i); } static bool get_in_bucket(const struct apfl_hashmap map, struct apfl_hashmap_bucket *bucket, const void *key, void *value) { - void *value_ptr; + void *value_ptr = peek_in_bucket(map, bucket, key); - if (!peek_in_bucket(map, bucket, key, &value_ptr)) { + if (value_ptr == NULL) { return false; } @@ -329,10 +324,10 @@ apfl_hashmap_delete(struct apfl_hashmap *map, const void *key) delete_in_bucket(*map, bucket_by_key(*map, key), key); } -bool -apfl_hashmap_peek(const struct apfl_hashmap *map, const void *key, void **value_ptr) +void * +apfl_hashmap_peek(const struct apfl_hashmap *map, const void *key) { - return get_in_bucket(*map, bucket_by_key(*map, key), key, value_ptr); + return peek_in_bucket(*map, bucket_by_key(*map, key), key); } bool @@ -528,41 +523,47 @@ apfl_hashmap_cursor_next(struct apfl_hashmap_cursor *cursor) struct apfl_hashmap_bucket *bucket = cursor_get_bucket(cursor); \ \ if (bucket == NULL) { \ - return; /* End already reached */ \ + return NULL; /* End already reached */ \ } \ \ if (cursor.i >= bucket->len) { \ - return; \ + return NULL; \ } \ \ size_t size = cursor.map->sizememb; \ \ - out = KVADDR(bucket->bucketmemb, size, cursor.i); + return KVADDR(bucket->bucketmemb, size, cursor.i); -void -apfl_hashmap_cursor_peek_key(struct apfl_hashmap_cursor cursor, void **key_ptr) +void * +apfl_hashmap_cursor_peek_key(struct apfl_hashmap_cursor cursor) { CURSOR_PEEK(cursor, *key_ptr, keys, keysize) } -void -apfl_hashmap_cursor_peek_value(struct apfl_hashmap_cursor cursor, void **value_ptr) +void * +apfl_hashmap_cursor_peek_value(struct apfl_hashmap_cursor cursor) { CURSOR_PEEK(cursor, *value_ptr, values, valsize) } -void +bool apfl_hashmap_cursor_get_key(struct apfl_hashmap_cursor cursor, void *key) { - void *key_ptr; - apfl_hashmap_cursor_peek_key(cursor, &key_ptr); + void *key_ptr = apfl_hashmap_cursor_peek_key(cursor); + if (key_ptr == NULL) { + return false; + } copy_key(*cursor.map, key, key_ptr); + return true; } -void +bool apfl_hashmap_cursor_get_value(struct apfl_hashmap_cursor cursor, void *value) { - void *value_ptr; - apfl_hashmap_cursor_peek_value(cursor, &value_ptr); + void *value_ptr = apfl_hashmap_cursor_peek_value(cursor); + if (value_ptr == NULL) { + return false; + } copy_value(*cursor.map, value, value_ptr); + return true; } diff --git a/src/hashmap.h b/src/hashmap.h index 3031ebd..6b824da 100644 --- a/src/hashmap.h +++ b/src/hashmap.h @@ -73,7 +73,7 @@ bool apfl_hashmap_init( size_t valsize ); void apfl_hashmap_delete(struct apfl_hashmap *, const void *key); -bool apfl_hashmap_peek(const struct apfl_hashmap *, const void *key, void **value_ptr); +void *apfl_hashmap_peek(const struct apfl_hashmap *, const void *key); bool apfl_hashmap_get(const struct apfl_hashmap *, const void *key, void *value); bool apfl_hashmap_set(struct apfl_hashmap *, void *key, void *value); size_t apfl_hashmap_count(const struct apfl_hashmap); @@ -93,10 +93,14 @@ bool apfl_hashmap_copy(struct apfl_hashmap *dst, struct apfl_hashmap src); struct apfl_hashmap_cursor apfl_hashmap_get_cursor(struct apfl_hashmap *); bool apfl_hashmap_cursor_is_end(struct apfl_hashmap_cursor); void apfl_hashmap_cursor_next(struct apfl_hashmap_cursor *); -void apfl_hashmap_cursor_peek_key(struct apfl_hashmap_cursor, void **key_ptr); -void apfl_hashmap_cursor_peek_value(struct apfl_hashmap_cursor, void **value_ptr); -void apfl_hashmap_cursor_get_key(struct apfl_hashmap_cursor, void *key); -void apfl_hashmap_cursor_get_value(struct apfl_hashmap_cursor, void *value); + +// The returned pointer will only be NULL, if the iterator is at the end. +void *apfl_hashmap_cursor_peek_key(struct apfl_hashmap_cursor); +void *apfl_hashmap_cursor_peek_value(struct apfl_hashmap_cursor); + +// These will only return false, if the iterator is at the end. +bool apfl_hashmap_cursor_get_key(struct apfl_hashmap_cursor, void *key); +bool apfl_hashmap_cursor_get_value(struct apfl_hashmap_cursor, void *value); #define HASHMAP_EACH(map_ptr,cur) \ for ( \ diff --git a/src/hashmap_test.c b/src/hashmap_test.c index b080b48..2f257b8 100644 --- a/src/hashmap_test.c +++ b/src/hashmap_test.c @@ -1,3 +1,4 @@ +#include #include #include "test.h" @@ -282,15 +283,11 @@ TEST(int2int_iterating, t) { bool seen_1 = false; bool seen_0 = false; - for ( - struct apfl_hashmap_cursor it = apfl_hashmap_get_cursor(&map); - !apfl_hashmap_cursor_is_end(it); - apfl_hashmap_cursor_next(&it) - ) { + HASHMAP_EACH(&map, it) { int k, v; - apfl_hashmap_cursor_get_key(it, &k); - apfl_hashmap_cursor_get_value(it, &v); + assert(apfl_hashmap_cursor_get_key(it, &k)); + assert(apfl_hashmap_cursor_get_value(it, &v)); switch (k) { ITERATING_SWITCH_CASE(t, 123, seen_123, 456, v) @@ -487,16 +484,12 @@ TEST(str2str_iterating, t) { bool seen_baz = false; bool seen_blank = false; - for ( - struct apfl_hashmap_cursor it = apfl_hashmap_get_cursor(&map); - !apfl_hashmap_cursor_is_end(it); - apfl_hashmap_cursor_next(&it) - ) { + HASHMAP_EACH(&map, it) { char *k; char *v; - apfl_hashmap_cursor_get_key(it, &k); - apfl_hashmap_cursor_get_value(it, &v); + assert(apfl_hashmap_cursor_get_key(it, &k)); + assert(apfl_hashmap_cursor_get_value(it, &v)); if (strcmp(k, "foo") == 0) { ITERATING_SEEN_CHECK(t, "foo", seen_foo, "abc", v) diff --git a/src/value.c b/src/value.c index 3bf1516..58ee004 100644 --- a/src/value.c +++ b/src/value.c @@ -111,14 +111,13 @@ print(unsigned indent, FILE *out, struct apfl_value value, bool skip_first_inden return; } - struct apfl_hashmap_cursor cur = apfl_hashmap_get_cursor(&value.dict->map); - apfl_print_indented(first_indent, out, "[\n"); - for (; !apfl_hashmap_cursor_is_end(cur); apfl_hashmap_cursor_next(&cur)) { - struct apfl_value *k; - struct apfl_value *v; - apfl_hashmap_cursor_peek_key(cur, (void **)&k); - apfl_hashmap_cursor_peek_value(cur, (void **)&v); + + HASHMAP_EACH(&value.dict->map, cur) { + struct apfl_value *k = apfl_hashmap_cursor_peek_key(cur); + assert(k != NULL); + struct apfl_value *v = apfl_hashmap_cursor_peek_value(cur); + assert(v != NULL); print(indent+1, out, *k, false); fprintf(out, " -> "); @@ -173,17 +172,15 @@ dict_eq(const apfl_dict a, const apfl_dict b) return true; } - struct apfl_hashmap_cursor cur = apfl_hashmap_get_cursor(&a->map); size_t total = 0; - for (; !apfl_hashmap_cursor_is_end(cur); apfl_hashmap_cursor_next(&cur)) { - struct apfl_value *key; - struct apfl_value *val; + HASHMAP_EACH(&a->map, cur) { + struct apfl_value *key = apfl_hashmap_cursor_peek_key(cur); + assert(key != NULL); + struct apfl_value *val = apfl_hashmap_cursor_peek_value(cur); + assert(val != NULL); - apfl_hashmap_cursor_peek_key(cur, (void **)&key); - apfl_hashmap_cursor_peek_value(cur, (void **)&val); - - struct apfl_value *other_val; - if (!apfl_hashmap_peek(&b->map, &key, (void **)&other_val)) { + struct apfl_value *other_val = apfl_hashmap_peek(&b->map, key); + if (other_val == NULL) { return false; } @@ -339,10 +336,12 @@ dict_get_for_editing(struct gc *gc, struct dict_header **_dict) HASHMAP_EACH(©.map, cur) { struct apfl_value *item; - apfl_hashmap_cursor_peek_key(cur, (void **)&item); + item = apfl_hashmap_cursor_peek_key(cur); + assert(item != NULL); *item = apfl_value_set_cow_flag(*item); - apfl_hashmap_cursor_peek_value(cur, (void **)&item); + item = apfl_hashmap_cursor_peek_value(cur); + assert(item != NULL); *item = apfl_value_set_cow_flag(*item); } @@ -522,11 +521,10 @@ void apfl_gc_dict_traverse(struct dict_header *dict, gc_visitor cb, void *opaque) { HASHMAP_EACH(&dict->map, cur) { - struct apfl_value *k; - struct apfl_value *v; - - apfl_hashmap_cursor_peek_key(cur, (void **)&k); - apfl_hashmap_cursor_peek_value(cur, (void **)&v); + struct apfl_value *k = apfl_hashmap_cursor_peek_key(cur); + assert(k != NULL); + struct apfl_value *v = apfl_hashmap_cursor_peek_value(cur); + assert(v != NULL); call_visitor_for_value(cb, opaque, *k); call_visitor_for_value(cb, opaque, *v);