refactor: consolidate MAX_DEPTH RV handling into rv_clone_iterative#129
Merged
atoomic merged 1 commit intoApr 25, 2026
Merged
Conversation
The rdepth > MAX_DEPTH block had three separate RV cases (RV-to-AV, RV-to-HV, RV-to-scalar) that duplicated logic already present in rv_clone_iterative. The first two cases also lacked SvWEAKREF handling, silently converting weak references into strong ones when cloning past MAX_DEPTH. Consolidate all three into a single rv_clone_iterative call, which already handles AV/HV referent dispatch, blessing preservation, and weakref propagation. This removes ~12 lines of duplicated code and fixes the weakref loss bug. Add tests verifying weakref preservation on deeply nested structures past MAX_DEPTH. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
atoomic
approved these changes
Apr 25, 2026
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
2 participants
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.
What
Consolidates three separate RV cases in the
rdepth > MAX_DEPTHblock into a singlerv_clone_iterative()call.Why
The MAX_DEPTH handler had dedicated code paths for RV-to-AV and RV-to-HV that duplicated logic already in
rv_clone_iterative(). These two paths were also missingSvWEAKREFhandling — weak references to deeply nested arrays/hashes past MAX_DEPTH were silently converted to strong references during clone.This is another instance of the recursive-to-iterative translation gap pattern (cf. GH #107, #113, #116, #119, #120, #127): when the iterative paths were added incrementally, each new one carried a subset of the safety invariants from the recursive path.
How
Removed the separate RV-to-AV and RV-to-HV blocks (lines 408–422), letting all RV types fall through to the existing
rv_clone_iterative()call. This function already dispatches toav_clone_iterative()/hv_clone_iterative()for container referents and handles blessing + weakref propagation.Net effect: −12 lines of code, +1 bug fix, same behavior for non-weakref cases.
Testing
t/10-deep_recursion.tpass (added 5 new tests for weakref preservation past MAX_DEPTH)🤖 Generated with Claude Code
Quality Report
Changes: 2 files changed, 63 insertions(+), 22 deletions(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline