From eeadc695c84b759038f7d5c60765ce4c8b8702a9 Mon Sep 17 00:00:00 2001
From: Claude <noreply@anthropic.com>
Date: Sun, 14 Jun 2026 06:48:53 +0000
Subject: [PATCH] Fix optional self-assignment/leak and set iterator types
 (#31)

optional::operator=(optional const&) (the pre-C++17 fallback) set _value to
nullptr without deleting the previous allocation, leaking it. For self-
assignment it was worse: nulling _value first made has_value() return false,
so the value was silently dropped. Add a self-assignment guard and reset()
the old value before copying.

set's begin()/end() declared their return types as std::set<TKey>::iterator
(default comparator) while the member is std::set<TKey, TCompare>. These are
the same type in practice but not guaranteed; spell them with TCompare.

Added OptionalTest.SelfAssignmentTest, which fails on the old code (value
dropped) and passes with the fix.
---
 include/optional.h     |  5 ++++-
 include/set.h          |  8 ++++----
 tests/optional_test.cc | 11 +++++++++++
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/optional.h b/include/optional.h
index 81c5946..f67dcaf 100644
--- a/include/optional.h
+++ b/include/optional.h
@@ -54,7 +54,10 @@ using optional_t = std::optional<T>;
 
         optional& operator=(optional const& other)
         {
-            _value = nullptr;
+            if (this == &other) {
+                return *this;
+            }
+            reset();
             if (other.has_value()) {
                 _value = new T{other.value()};
             }
diff --git a/include/set.h b/include/set.h
index f6ef8f4..6630b9d 100644
--- a/include/set.h
+++ b/include/set.h
@@ -1065,25 +1065,25 @@ namespace fcpp {
         }
 
         // Returns the begin iterator, useful for other standard library algorithms
-        [[nodiscard]] typename std::set<TKey>::iterator begin()
+        [[nodiscard]] typename std::set<TKey, TCompare>::iterator begin()
         {
             return m_set.begin();
         }
 
         // Returns the const begin iterator, useful for other standard library algorithms
-        [[nodiscard]] typename std::set<TKey>::const_iterator begin() const
+        [[nodiscard]] typename std::set<TKey, TCompare>::const_iterator begin() const
         {
             return m_set.begin();
         }
 
         // Returns the end iterator, useful for other standard library algorithms
-        [[nodiscard]] typename std::set<TKey>::iterator end()
+        [[nodiscard]] typename std::set<TKey, TCompare>::iterator end()
         {
             return m_set.end();
         }
 
         // Returns the const end iterator, useful for other standard library algorithms
-        [[nodiscard]] typename std::set<TKey>::const_iterator end() const
+        [[nodiscard]] typename std::set<TKey, TCompare>::const_iterator end() const
         {
             return m_set.end();
         }
diff --git a/tests/optional_test.cc b/tests/optional_test.cc
index a48dadf..a5b1899 100644
--- a/tests/optional_test.cc
+++ b/tests/optional_test.cc
@@ -108,3 +108,14 @@ TEST(OptionalTest, CopyConstructorNullTest)
     const optional_t<int> v1(v2);
     EXPECT_FALSE(v1.has_value());
 }
+
+TEST(OptionalTest, SelfAssignmentTest)
+{
+    optional_t<int> v(5);
+    // Assign through an alias so the self-assignment is not diagnosed by the
+    // compiler but still exercises operator=(optional const&) with this == &other.
+    const optional_t<int>& alias = v;
+    v = alias;
+    EXPECT_TRUE(v.has_value());
+    EXPECT_EQ(5, v.value());
+}