Conversation
…o implement/hamt-shared-node
…o implement/hamt-shared-node
…o implement/hamt-shared-node
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add shared packed HAMT nodes with explicit intrusive ownership and deterministic tree reclamation.
AG;DR
Shared nodes retain borrowed children after successful construction and recursively release owned child references when their last reference is released. Release returns each original MemoryBlock, including oversized allocation metadata. Copy/move are disabled. Entry copies and destruction must be non-throwing; unexpected source exceptions terminate under noexcept allocation methods. Counts must match the bitmap and occupied child slots reject null before allocation.
Tests cover sharing, last-reference reclamation, null retain/release, const views, count mismatches, exhaustion without child-reference changes, exact source metadata, and null-child rejection before allocation. HAMT_STORAGE.md documents balancing references, source lifetime/identity, immutability, external synchronization, atomic-count limitations, and the 32-bit reference-count precondition.
The draft contains the fully green packed-block base 6f58e35. Added tests reject null, undersized, under-aligned, and misaligned offered blocks, returning exact metadata once and never retaining children. Allocation validity is checked explicitly before accessing optional payloads. Tests extract successful node pointers once and assert non-nullness before ownership operations. Packed entry and child addresses use creation-validated immutable counts without unchecked optional layout results or additional per-node layout storage. A byte-entry test exercises padding before child pointers. No coverage baselines or exclusions change.
The clang-tidy follow-up removes static calls through a source instance and replaces the oversized test source's inheritance with a standalone implementation, eliminating method shadowing. Required block-source contract spellings have narrowly scoped naming annotations; no general lint checks are disabled.
Latest repair b206de3 places the three required constexpr-name annotations before their declarations, so clang-format cannot move them away from the diagnostic location. Focused pre-commit passes after formatting. The preceding f02cbfa head passed all compiler, sanitizer, BCR, and coverage jobs, but clang-tidy exposed those three misplaced annotations; replacement CI is required.
Validation: focused shared-node tests and focused pre-commit pass; local clang-tidy and trunk-fmt remain skipped as requested. The repair is published as f02cbfa and recognized by GitHub as current. Its replacement CI remains pending. Previous head ab78cae passed every build, sanitizer, BCR, and coverage job but failed clang-tidy on these test-source issues. Coverage was L=97.20%, F=98.42%, B=93.02%, with patch L=98.46% and B=95.56%; those previous-head results do not substitute for replacement CI. This is internal ownership machinery for general HAMT containers, not the complete public map/set. No merging is requested.