Core: Share the AHashMap table implementation and add AHashSet

[dsarno]

Summary

Factors the open-addressing table out of AHashMap into a shared RawAHashTable, and adds AHashSet on top of it. AHashMap keeps its behaviour and API; the storage, probing, growth and erase logic now live in one place instead of being duplicated per container.

Six files: the new raw_a_hash_table.h and a_hash_set.h, the reworked a_hash_map.{h,cpp}, one call-site update, and a new test file.

Provenance

The first commit is third-party work — 36 commits squashed and rebased onto current master, original authorship preserved (Brogolem35). The merge-base was 3,420 commits behind. The second commit is review fixes.

This branch was produced by an AI agent. Everything below was built, run and measured, but it warrants a human read — this is a container used engine-wide, so the blast radius of a mistake here is large.

The tests were never being compiled

The most consequential finding. tests/SCsub globs */**/*.cpp, and the new test file was named test_a_hash_set.h — so its 263 lines of AHashSet tests had never been built or run. Renaming it to .cpp immediately exposed a latent compile error (AHashSet<int> copy_assign = set; against an explicit copy constructor), fixed to match the existing HashSet test.

A green CI run on the original branch would have meant nothing for this container.

Conflict resolution

• a_hash_map.h — upstream had reworked the doc comment, added _WARN_UNUSED_ on the class and _LIFETIME_BOUND_ on ~20 accessors, and renamed iterator comparison parameters. Took upstream's annotations, kept the CRTP base and using Base:: block, and applied the same conventions to the two new files (written before that refactor landed).
• a_hash_map.cpp — modify/delete. The change deleted it; upstream still maintains it for explicit template instantiation of five hot types. Kept upstream's file and restored the extern template block, then verified the explicit instantiation still links through the CRTP base.
• animation_blend_tree_editor_plugin.cpp — upstream changed the key type from String to NodePath. Resolved to AHashSet<NodePath>. Iteration order is unaffected: both containers are insertion-ordered and nothing erases on that path.

Defects found and fixed

severity	defect
High (perf)	virtual ~RawAHashTable() left over from the pre-CRTP design made every container polymorphic — sizeof(AHashMap<int,int>) 24 → 32 bytes engine-wide. Now a protected non-virtual destructor.
Medium	AHashSet::insert() overwrote an already-stored element; HashSet::insert() keeps it. With a comparator that treats distinguishable elements as equal, the stored element silently changed.
Medium	AHashSet had no move assignment, so s = std::move(x) bound to copy assignment and deep-copied. Added a swap-based one mirroring HashSet.
Low	reset() freed the metadata array without clearing the pointer, leaving a dangling pointer behind public API.

Two of the four were found by diffing the new container's API against HashSet's — worth completing that comparison rather than sampling it.

Deliberately not fixed

Three defects were confirmed pre-existing (they reproduce identically against the pre-refactor header) and are out of scope here:

• Heap-use-after-free when an insert() argument aliases the container's own storage and the insert triggers a rehash. ASAN trace captured.
• reserve() can shrink capacity on an unallocated container.
• Capacity math overflows above 2^31 entries.

The first deserves its own change.

Testing

• Targeted: *AHashSet* 17/17, *AHashMap* 18/18, *HashMap* 28/28, *HashSet* 30/30 — 58 cases / 57,217 assertions, 0 failed.
• Full suite: 1429 cases, 1 failure — [IP] resolve_hostname, which has no IPv6 loopback in this sandbox and fails byte-identically on the master baseline binary. Net delta vs master: +18 cases, +4,824 assertions, zero new failures.
• Differential fuzzer against std::unordered_map / std::unordered_set under ASAN+UBSAN: 40 seeds × 4 workloads, good and adversarial hashers, allocation-owning key/value types — clean, zero leaks.
• Behavioural signature diff (size, capacity growth, iteration order over 60 seeds × 6,000 ops) between pre- and post-refactor AHashMap: byte-identical, before and after the fixes.

Revert-proof: reverting the three testable fixes fails exactly 4 cases / 7 assertions, one per fix — including CHECK(32 == 24) for the vtable regression and CHECK(GODOT == Godot) for the insert-overwrite. The dangling-pointer fix has no failing test and is not claimed as one; it is hardening, with a guard test added.

Benchmark — the refactor is not a speedup

Standalone g++ -O2, median of 7, N = 1,000,000, ms, lower is better (two passes, order reversed):

container	insert	lookup hit	lookup miss	iterate	erase
AHashMap (this branch)	95.7 / 96.8	18.5 / 19.5	27.4 / 27.6	1.23 / 1.20	46.4 / 46.7
AHashMap (pre-refactor)	94.0 / 95.2	19.3 / 19.5	28.9 / 28.3	1.21 / 1.23	45.0 / 45.2
HashMap	153.3 / 152.3	39.1 / 42.4	37.4 / 39.5	7.16 / 7.07	81.2 / 82.3
AHashSet (this branch)	79.6 / 82.3	17.8 / 18.2	29.1 / 29.7	0.71 / 0.73	41.1 / 41.6
HashSet	126.0 / 110.4	32.7 / 30.6	41.1 / 38.3	1.08 / 1.02	79.1 / 67.0

At N = 10,000 every container is at the noise floor.

Honest reading: new vs pre-refactor AHashMap is within ±2% on every operation. The CRTP extraction is performance-neutral, which is exactly what it should be — this is a deduplication, not an optimisation. The large wins in the table (~1.6× insert, ~1.7–2.2× lookup, ~1.5–1.8× erase, ~6× map iteration over HashMap/HashSet) predate this change. The one thing this change measurably moved was the vtable regression, now removed.

Benchmark and fuzz harnesses were kept out of the commit; the working tree is clean.

What a human must decide

1. Restoring a_hash_map.cpp. The change deleted it, dropping explicit instantiation for five hot types; upstream's version was kept. If that deletion was deliberate, revisit.
2. AHashMap still has no move assignment — map = std::move(x) deep-copies. Only AHashSet got one, for parity with HashSet; adding it to AHashMap changes behaviour at every existing call site.
3. AHashSet deliberately omits the array API (get_by_index, get_index, erase_by_index, get_elements_ptr) that AHashMap exposes — which is arguably the main selling point of these containers.
4. CRTP with a protected non-virtual destructor is correct but relies on nobody deleting through a RawAHashTable*. Worth a header comment.
5. The GDScript test suite SIGSEGVs in an optimize=none build — confirmed pre-existing on master, but someone should check CI's optimised build is unaffected.