Everywhere: Remove HashMap::add() and remove(), as its not mutable

src/ast.cpp

@@ -114,36 +114,16 @@ HashMap::HashMap(const HashMap& that, ValuePtr meta)
 {
 }
 
-void HashMap::add(const std::string& key, ValuePtr value)
-{
-	if (value == nullptr) {
-		return;
-	}
-
-	m_elements.insert_or_assign(key, value);
-}
-
-void HashMap::add(ValuePtr key, ValuePtr value)
-{
-	if (key == nullptr || value == nullptr) {
-		return;
-	}
-
-	m_elements.insert_or_assign(getKeyString(key), value);
-}
-
-void HashMap::remove(const std::string& key)
-{
-	m_elements.erase(key);
-}
-
-void HashMap::remove(ValuePtr key)
+std::string HashMap::getKeyString(ValuePtr key)
 {
-	if (key == nullptr) {
-		return;
+	if (!is<String>(key.get()) && !is<Keyword>(key.get())) {
+		Error::the().add(::format("wrong argument type: string or keyword, {}", key));
+		return {};
 	}
 
-	m_elements.erase(getKeyString(key));
+	return is<String>(key.get())
+	           ? std::static_pointer_cast<String>(key)->data()
+	           : std::static_pointer_cast<Keyword>(key)->keyword();
 }
 
 bool HashMap::exists(const std::string& key)
@@ -170,18 +150,6 @@ ValuePtr HashMap::get(ValuePtr key)
 	return get(getKeyString(key));
 }
 
-std::string HashMap::getKeyString(ValuePtr key)
-{
-	if (!is<String>(key.get()) && !is<Keyword>(key.get())) {
-		Error::the().add(::format("wrong argument type: string or keyword, {}", key));
-		return {};
-	}
-
-	return is<String>(key.get())
-	           ? std::static_pointer_cast<String>(key)->data()
-	           : std::static_pointer_cast<Keyword>(key)->keyword();
-}
-
 // -----------------------------------------
 
 String::String(const std::string& data)

src/ast.h

@@ -174,20 +174,17 @@ private:
 
 // -----------------------------------------
 
+using Elements = std::map<std::string, ValuePtr>;
+
 // {}
 class HashMap final : public Value {
 public:
-	using Elements = std::map<std::string, ValuePtr>;
-
 	HashMap() = default;
 	HashMap(const Elements& elements);
 	HashMap(const HashMap& that, ValuePtr meta);
 	virtual ~HashMap() = default;
 
-	void add(const std::string& key, ValuePtr value);
-	void add(ValuePtr key, ValuePtr value);
-	void remove(const std::string& key);
-	void remove(ValuePtr key);
+	static std::string getKeyString(ValuePtr key);
 
 	bool exists(const std::string& key);
 	bool exists(ValuePtr key);
@@ -202,8 +199,6 @@ public:
 private:
 	virtual bool isHashMap() const override { return true; }
 
-	std::string getKeyString(ValuePtr key);
-
 	Elements m_elements;
 };
 

src/eval.cpp

@@ -181,8 +181,8 @@ ValuePtr Eval::evalAst(ValuePtr ast, EnvironmentPtr env)
 		return makePtr<Vector>(evaluated_nodes);
 	}
 	else if (is<HashMap>(ast_raw_ptr)) {
-		auto result = makePtr<HashMap>();
 		const auto& elements = std::static_pointer_cast<HashMap>(ast)->elements();
+		Elements evaluated_elements;
 		for (const auto& element : elements) {
 			m_ast_stack.push(element.second);
 			m_env_stack.push(env);
@@ -190,10 +190,10 @@ ValuePtr Eval::evalAst(ValuePtr ast, EnvironmentPtr env)
 			if (element_node == nullptr) {
 				return nullptr;
 			}
-			result->add(element.first, element_node);
+			evaluated_elements.insert_or_assign(element.first, element_node);
 		}
 
-		return result;
+		return makePtr<HashMap>(evaluated_elements);
 	}
 
 	return ast;

src/functions.cpp

@@ -712,16 +712,16 @@ ADD_FUNCTION(
 	{
 		CHECK_ARG_COUNT_EVEN("hash-map", SIZE());
 
-		auto result = makePtr<HashMap>();
-
+		Elements elements;
 		for (auto it = begin; it != end; std::advance(it, 2)) {
-			result->add(*it, *(std::next(it)));
+			const ValuePtr& value = *(std::next(it)); // temporary instance to get around const
+			elements.insert_or_assign(HashMap::getKeyString(*it), value);
 		}
 
-		return result;
+		return makePtr<HashMap>(elements);
 	});
 
-// (assoc {:a 1 :b 2} :a 3 :c 1)
+// (assoc {:a 1 :b 2} :a 3 :c 1) -> {:a 3 :b 2 :c 1}
 ADD_FUNCTION(
 	"assoc",
 	{
@@ -732,15 +732,16 @@ ADD_FUNCTION(
 
 		CHECK_ARG_COUNT_EVEN("assoc", SIZE());
 
-		auto result = makePtr<HashMap>(hash_map->elements());
-
+		Elements elements(hash_map->elements());
 		for (auto it = begin; it != end; std::advance(it, 2)) {
-			result->add(*it, *(std::next(it)));
+			const ValuePtr& value = *(std::next(it)); // temporary instance to get around const
+			elements.insert_or_assign(HashMap::getKeyString(*it), value);
 		}
 
-		return result;
+		return makePtr<HashMap>(elements);
 	});
 
+// (dissoc {:a 1 :b 2 :c 3} :a :c :d) -> {:b 2}
 ADD_FUNCTION(
 	"dissoc",
 	{
@@ -749,13 +750,12 @@ ADD_FUNCTION(
 		VALUE_CAST(hash_map, HashMap, (*begin));
 		begin++;
 
-		auto result = makePtr<HashMap>(hash_map->elements());
-
+		Elements elements(hash_map->elements());
 		for (auto it = begin; it != end; ++it) {
-			result->remove(*it);
+			elements.erase(HashMap::getKeyString(*it));
 		}
 
-		return result;
+		return makePtr<HashMap>(elements);
 	});
 
 // (get {:kw "value"} :kw) -> "value"

src/reader.cpp

@@ -174,7 +174,7 @@ ValuePtr Reader::readHashMap()
 {
 	ignore(); // {
 
-	auto hash_map = makePtr<HashMap>();
+	Elements elements;
 	while (!isEOF() && peek().type != Token::Type::BraceClose) {
 		auto key = readImpl();
 
@@ -193,7 +193,7 @@ ValuePtr Reader::readHashMap()
 		}
 
 		auto value = readImpl();
-		hash_map->add(key, value);
+		elements.insert_or_assign(HashMap::getKeyString(key), value);
 	}
 
 	if (!consumeSpecific(Token { .type = Token::Type::BraceClose })) { // }
@@ -201,7 +201,7 @@ ValuePtr Reader::readHashMap()
 		return nullptr;
 	}
 
-	return hash_map;
+	return makePtr<HashMap>(elements);
 }
 
 ValuePtr Reader::readQuote()