From 4c4bde45cf43fe6338f91870b069a25e18ad0a6a Mon Sep 17 00:00:00 2001
From: Nav <nav@navnav.me>
Date: Mon, 30 Aug 2021 22:20:36 +0100
Subject: [PATCH] Fixed numerous bugs with hashing and sorting of
 TargetRegisterDescriptor objects These became apparent when using some STL
 contains (map, set, etc)

---
 CMakeLists.txt                 |  1 +
 src/Targets/TargetRegister.cpp | 14 ++++++++++++++
 src/Targets/TargetRegister.hpp | 23 +++++++++++++++++++----
 3 files changed, 34 insertions(+), 4 deletions(-)
 create mode 100644 src/Targets/TargetRegister.cpp

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 4cf7ec75..02f7901f 100755
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -98,6 +98,7 @@ add_executable(Bloom
 
     # Targets
     src/Targets/TargetDescription/TargetDescriptionFile.cpp
+    src/Targets/TargetRegister.cpp
     src/Targets/Microchip/AVR/AVR8/Avr8.cpp
     src/Targets/Microchip/AVR/AVR8/TargetDescription/TargetDescriptionFile.cpp
     build/resources/TargetDescriptionFiles/AVR/Mapping.json
diff --git a/src/Targets/TargetRegister.cpp b/src/Targets/TargetRegister.cpp
new file mode 100644
index 00000000..8841d9ec
--- /dev/null
+++ b/src/Targets/TargetRegister.cpp
@@ -0,0 +1,14 @@
+#include "TargetRegister.hpp"
+
+using namespace Bloom::Targets;
+
+std::size_t TargetRegisterDescriptor::getHash() const {
+    if (!this->cachedHash.has_value()) {
+        auto stringHasher = std::hash<std::string>();
+
+        this->cachedHash = stringHasher(std::to_string(this->startAddress.value_or(0))
+            + "_" + std::to_string(static_cast<std::uint8_t>(this->type)));
+    }
+
+    return this->cachedHash.value();
+}
diff --git a/src/Targets/TargetRegister.hpp b/src/Targets/TargetRegister.hpp
index 6471fcf4..805518c1 100644
--- a/src/Targets/TargetRegister.hpp
+++ b/src/Targets/TargetRegister.hpp
@@ -22,6 +22,14 @@ namespace Bloom::Targets
 
     struct TargetRegisterDescriptor
     {
+        friend std::hash<Bloom::Targets::TargetRegisterDescriptor>;
+
+    private:
+        mutable std::optional<std::size_t> cachedHash;
+
+        std::size_t getHash() const;
+
+    public:
         std::optional<std::uint32_t> startAddress;
         std::uint32_t size = 0;
         TargetRegisterType type = TargetRegisterType::OTHER;
@@ -30,19 +38,26 @@ namespace Bloom::Targets
         std::optional<std::string> groupName;
         std::optional<std::string> description;
 
+        bool readable = false;
+        bool writable = false;
+
         TargetRegisterDescriptor() = default;
         explicit TargetRegisterDescriptor(TargetRegisterType type): type(type) {};
 
         bool operator == (const TargetRegisterDescriptor& other) const {
-            return this->startAddress.value_or(0) == other.startAddress.value_or(0) && this->type == other.type;
+            return this->getHash() == other.getHash();
         }
 
         bool operator < (const TargetRegisterDescriptor& other) const {
-            if (this->startAddress.has_value() && other.startAddress.has_value()) {
+            if (this->type == other.type) {
                 return this->startAddress.value_or(0) < other.startAddress.value_or(0);
 
             } else {
-                return this->name.value_or("") < other.name.value_or("");
+                /*
+                 * If the registers are of different type, there is no meaningful way to sort them, so we just use
+                 * the unique hash.
+                 */
+                return this->getHash() < other.getHash();
             }
         }
     };
@@ -77,7 +92,7 @@ namespace std
     {
     public:
         std::size_t operator()(const Bloom::Targets::TargetRegisterDescriptor& descriptor) const {
-            return descriptor.startAddress.value_or(0) + static_cast<std::uint8_t>(descriptor.type);
+            return descriptor.getHash();
         }
     };
 }