Fix optional copy-assignment leak/self-assignment bug and set iterator types (#31)

[jkalias]

Summary

Fixes the two robustness defects tracked in #31. Both are small, self-contained correctness/safety fixes with no behavior change for valid usage.

1. optional::operator= — memory leak and self-assignment value loss

include/optional.h (the pre-C++17 fallback optional; C++17 uses std::optional)

optional& operator=(optional const& other)
{
    _value = nullptr;                 // leaks the existing allocation if non-null
    if (other.has_value()) {
        _value = new T{other.value()};
    }
    return *this;
}

Two problems:

• Leak: copy-assigning onto an optional that already holds a value overwrites _value without deleteing the previous heap allocation.
• Self-assignment value loss (worse): _value = nullptr runs first, so for v = v the following other.has_value() reads the just-nulled pointer, returns false, and the value is silently dropped (and the original leaked).

Fix — self-assignment guard, then reset() (which deletes the old value) before copying:

optional& operator=(optional const& other)
{
    if (this == &other) {
        return *this;
    }
    reset();
    if (other.has_value()) {
        _value = new T{other.value()};
    }
    return *this;
}

This mirrors the existing operator=(T const&) overload, which already calls reset() first.

2. set iterator accessors — comparator-independent iterator type

include/set.h

[[nodiscard]] typename std::set<TKey>::iterator begin()   // member is std::set<TKey, TCompare>

The four begin()/end() overloads declared their return types as std::set<TKey>::iterator (default comparator), while the underlying member is std::set<TKey, TCompare>. It compiles today only because a std::set's iterator type happens not to depend on its comparator in current implementations — that's not guaranteed by the standard. Spelled the return types correctly with TCompare.

Tests

Added OptionalTest.SelfAssignmentTest. I confirmed it fails on the old code (value dropped → value() aborts) and passes with the fix.

Verification

Built and ran the full suite under Debug & Release × C++11 & C++17 — all green (302 tests in C++17, 286 in C++11). The C++11 configuration is what exercises the fallback optional.

Scope note

The original #31 report also listed lazy_set::zip(vector) "dropping the comparator" and some cosmetic items. On inspection those are not bugs (the vector-zip overloads have no UCompare to preserve; std::less is the correct dedup ordering, consistent with vector::distinct), so they were intentionally left out — see the updated issue for details.

Closes #31.

https://claude.ai/code/session_011y3XcVg3UCcgi5JMEo2tso

[jkalias]

@codex review

[chatgpt-codex-connector]

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: eeadc695c8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".