From d53bba97e8c76eaa76bdccb4d9a48851800b6032 Mon Sep 17 00:00:00 2001 From: Rangi42 Date: Tue, 20 Feb 2024 21:30:29 -0500 Subject: [PATCH] Remove our custom hashmap --- Makefile | 2 - include/hashmap.hpp | 70 ----------------- src/CMakeLists.txt | 2 - src/hashmap.cpp | 124 ------------------------------- test/asm/charmap-inheritance.asm | 2 +- test/asm/sym-collision.asm | 7 +- test/asm/sym-collision.err | 2 +- 7 files changed, 5 insertions(+), 204 deletions(-) delete mode 100644 include/hashmap.hpp delete mode 100644 src/hashmap.cpp diff --git a/Makefile b/Makefile index 7ada1ee6..a6d920c2 100644 --- a/Makefile +++ b/Makefile @@ -69,7 +69,6 @@ rgbasm_obj := \ src/extern/getopt.o \ src/extern/utf8decoder.o \ src/error.o \ - src/hashmap.o \ src/linkdefs.o \ src/opmath.o \ src/util.o @@ -89,7 +88,6 @@ rgblink_obj := \ src/extern/getopt.o \ src/extern/utf8decoder.o \ src/error.o \ - src/hashmap.o \ src/linkdefs.o \ src/opmath.o \ src/util.o diff --git a/include/hashmap.hpp b/include/hashmap.hpp deleted file mode 100644 index 4ee2fd53..00000000 --- a/include/hashmap.hpp +++ /dev/null @@ -1,70 +0,0 @@ -/* SPDX-License-Identifier: MIT */ - -// Generic hashmap implementation (C++ templates are calling...) -#ifndef RGBDS_LINK_HASHMAP_H -#define RGBDS_LINK_HASHMAP_H - -#include - -#define HASH_NB_BITS 32 -#define HALF_HASH_NB_BITS 16 -static_assert(HALF_HASH_NB_BITS * 2 == HASH_NB_BITS, ""); -#define HASHMAP_NB_BUCKETS (1 << HALF_HASH_NB_BITS) - -// HashMapEntry is internal, please do not attempt to use it -typedef struct HashMapEntry *HashMap[HASHMAP_NB_BUCKETS]; - -/* - * Adds an element to a hashmap. - * @warning Adding a new element with an already-present key will not cause an - * error, this must be handled externally. - * @warning Inserting a NULL will make `hash_GetElement`'s return ambiguous! - * @param map The HashMap to add the element to - * @param key The key with which the element will be stored and retrieved - * @param element The element to add - * @return A pointer to the pointer to the element. - */ -void **hash_AddElement(HashMap map, char const *key, void *element); - -/* - * Removes an element from a hashmap. - * @param map The HashMap to remove the element from - * @param key The key to search the element with - * @return True if the element was found and removed - */ -bool hash_RemoveElement(HashMap map, char const *key); - -/* - * Finds an element in a hashmap, and returns a pointer to its value field. - * @param map The map to consider the elements of - * @param key The key to search an element for - * @return A pointer to the pointer to the element, or NULL if not found. - */ -void **hash_GetNode(HashMap const map, char const *key); - -/* - * Finds an element in a hashmap. - * @param map The map to consider the elements of - * @param key The key to search an element for - * @return A pointer to the element, or NULL if not found. (NULL can be returned - * if such an element was added, but that sounds pretty silly.) - */ -void *hash_GetElement(HashMap const map, char const *key); - -/* - * Executes a function on each element in a hashmap. - * @param map The map to consider the elements of - * @param func The function to run. The first argument will be the element, - * the second will be `arg`. - * @param arg An argument to be passed to all function calls - */ -void hash_ForEach(HashMap const map, void (*func)(void *, void *), void *arg); - -/* - * Cleanly empties a hashmap from its contents. - * This does not `free` the data structure itself! - * @param map The map to empty - */ -void hash_EmptyMap(HashMap map); - -#endif // RGBDS_LINK_HASHMAP_H diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index f6dde0f1..7871975b 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -50,7 +50,6 @@ set(rgbasm_src "asm/symbol.cpp" "asm/warning.cpp" "extern/utf8decoder.cpp" - "hashmap.cpp" "linkdefs.cpp" "opmath.cpp" "util.cpp" @@ -84,7 +83,6 @@ set(rgblink_src "link/section.cpp" "link/symbol.cpp" "extern/utf8decoder.cpp" - "hashmap.cpp" "linkdefs.cpp" "opmath.cpp" "util.cpp" diff --git a/src/hashmap.cpp b/src/hashmap.cpp deleted file mode 100644 index 4ccff3ea..00000000 --- a/src/hashmap.cpp +++ /dev/null @@ -1,124 +0,0 @@ -/* SPDX-License-Identifier: MIT */ - -#include -#include -#include -#include - -#include "error.hpp" -#include "hashmap.hpp" - -// The lower half of the hash is used to index the "master" table, -// the upper half is used to help resolve collisions more quickly -#define UINT_BITS_(NB_BITS) uint##NB_BITS##_t -#define UINT_BITS(NB_BITS) UINT_BITS_(NB_BITS) -typedef UINT_BITS(HASH_NB_BITS) HashType; -typedef UINT_BITS(HALF_HASH_NB_BITS) HalfHashType; - -struct HashMapEntry { - HalfHashType hash; - char const *key; - void *content; - struct HashMapEntry *next; -}; - -#define FNV_OFFSET_BASIS 0x811C9DC5 -#define FNV_PRIME 16777619 - -// FNV-1a hash -static HashType hash(char const *str) -{ - HashType hash = FNV_OFFSET_BASIS; - - while (*str) { - hash ^= (uint8_t)*str++; - hash *= FNV_PRIME; - } - return hash; -} - -void **hash_AddElement(HashMap map, char const *key, void *element) -{ - HashType hashedKey = hash(key); - HalfHashType index = hashedKey; - struct HashMapEntry *newEntry = (struct HashMapEntry *)malloc(sizeof(*newEntry)); - - if (!newEntry) - err("%s: Failed to allocate new entry", __func__); - - newEntry->hash = hashedKey >> HALF_HASH_NB_BITS; - newEntry->key = key; - newEntry->content = element; - newEntry->next = map[index]; - map[index] = newEntry; - - return &newEntry->content; -} - -bool hash_RemoveElement(HashMap map, char const *key) -{ - HashType hashedKey = hash(key); - struct HashMapEntry **ptr = &map[(HalfHashType)hashedKey]; - - while (*ptr) { - if (hashedKey >> HALF_HASH_NB_BITS == (*ptr)->hash - && !strcmp((*ptr)->key, key)) { - struct HashMapEntry *next = (*ptr)->next; - - free(*ptr); - *ptr = next; - return true; - } - ptr = &(*ptr)->next; - } - return false; -} - -void **hash_GetNode(HashMap const map, char const *key) -{ - HashType hashedKey = hash(key); - struct HashMapEntry *ptr = map[(HalfHashType)hashedKey]; - - while (ptr) { - if (hashedKey >> HALF_HASH_NB_BITS == ptr->hash - && !strcmp(ptr->key, key)) { - return &ptr->content; - } - ptr = ptr->next; - } - return NULL; -} - -void *hash_GetElement(HashMap const map, char const *key) -{ - void **node = hash_GetNode(map, key); - - return node ? *node : NULL; -} - -void hash_ForEach(HashMap const map, void (*func)(void *, void *), void *arg) -{ - for (size_t i = 0; i < HASHMAP_NB_BUCKETS; i++) { - struct HashMapEntry *ptr = map[i]; - - while (ptr) { - func(ptr->content, arg); - ptr = ptr->next; - } - } -} - -void hash_EmptyMap(HashMap map) -{ - for (size_t i = 0; i < HASHMAP_NB_BUCKETS; i++) { - struct HashMapEntry *ptr = map[i]; - - while (ptr) { - struct HashMapEntry *next = ptr->next; - - free(ptr); - ptr = next; - } - map[i] = NULL; - } -} diff --git a/test/asm/charmap-inheritance.asm b/test/asm/charmap-inheritance.asm index fca6a92f..2c6b61f7 100644 --- a/test/asm/charmap-inheritance.asm +++ b/test/asm/charmap-inheritance.asm @@ -13,7 +13,7 @@ SECTION "test", ROM0 charmap "", $08 ; At this point, enough nodes were allocated for 'foo' to be reallocated. - ; Its value in the charmaps' hashmap should have been updated too, + ; Its value in the charmaps' std::map should have been updated too, ; so that usages of 'foo' will not segfault. ; This uses 'foo; by switching to it. diff --git a/test/asm/sym-collision.asm b/test/asm/sym-collision.asm index 961079c0..ed7695fb 100644 --- a/test/asm/sym-collision.asm +++ b/test/asm/sym-collision.asm @@ -1,9 +1,8 @@ -; Hashmap collisions are pretty poorly-tested code path... -; At some point, `PURGE` would malfunction with them - SECTION "Collision course", OAM[$FE00] -; All the following symbols collide! +; All the following symbols used to collide with our custom hashmap, +; which at some point caused `PURGE` to malfunction with them. +; We now use C++ `std::map` which reliably handles collisions. aqfj: ds 1 ; Give them different addresses cxje: ds 1 dgsd: ds 1 diff --git a/test/asm/sym-collision.err b/test/asm/sym-collision.err index 89ff0cbe..041c6e7d 100644 --- a/test/asm/sym-collision.err +++ b/test/asm/sym-collision.err @@ -1,3 +1,3 @@ -error: sym-collision.asm(26): +error: sym-collision.asm(25): Interpolated symbol "dork" does not exist error: Assembly aborted (1 error)!