Conversation
b40c0be to
fb46147
Compare
Two independent fixes targeting failures seen in `jcpan -t Memoize`: 1. Storable::last_op_in_netorder: implement the flag that reports whether the most recent freeze/store used network byte order. Set by nfreeze()/nstore(), cleared by freeze()/store(). Unblocks t/tie_storable.t. 2. "Deep recursion on subroutine" warning: RuntimeCode now tracks per-sub recursion depth and warns (under the "recursion" category) the first time depth exceeds 100, matching Perl's PERL_SUB_DEPTH_WARN behavior. Map/grep/eval blocks and builtins are exempt. Unblocks t/correctness.t (which relied on the warning firing before StackOverflowError). Test results for `jcpan -t Memoize` on this branch: - Before: t/correctness.t crashed with StackOverflowError after test 16; t/tie_storable.t died at test 6 with "Undefined subroutine &Storable::last_op_in_netorder". - After: correctness.t 17/17 pass; tie_storable.t 5/6 pass (test 6 requires tied-hash DESTROY at scope exit, a separate pre-existing limitation). threads->new / ->join stubs were considered but rejected: running a thread body synchronously in the current thread silently produces incorrect results whenever the code relies on thread isolation (as Memoize's t/threadsafe.t test 8 demonstrated — unmemoize ran for real on the main thread). Leaving t/threadsafe.t failing loudly is safer than shipping a convincing-but-wrong stub. t/tie.t and t/tie_db.t failures are caused by a broken user-local DB_File.pm AUTOLOAD (CPAN-installed; DB_File needs XS and is not supported on PerlOnJava) and are not addressed here. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
fb46147 to
10631d7
Compare
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.
Summary
Two independent fixes targeting failures seen in
./jcpan -t Memoize:Storable::last_op_in_netorder— implements the flag reporting whether the most recentfreeze/storeused network byte order. Set bynfreeze()/nstore(), cleared byfreeze()/store().RuntimeCodenow tracks per-sub recursion depth and warns (under therecursionwarnings category) the first time depth exceeds 100, matching Perl'sPERL_SUB_DEPTH_WARNbehavior. Map/grep/eval blocks and builtins are exempt.Memoize test results
t/correctness.tt/tie_storable.tUndefined subroutine &Storable::last_op_in_netorder)The lone remaining
tie_storable.tfailure requires tied-hashDESTROYto fire whenmy %cachegoes out of scope — a separate pre-existing limitation unrelated to these fixes.Not included:
threads->new/->joinstubsAn earlier version of this branch added
newas an alias forcreateand a synchronousjointhat ran the thread body in the current thread. That was rejected as unsafe: whenever a caller relies on thread isolation, running the body synchronously silently produces incorrect results. Memoize'st/threadsafe.ttest 8 is exactly such a case — the "thread" callsunmemoize('count_up'), and with a synchronousjointhat really unmemoizescount_upon the main thread, so the main-thread assertion fails. Leavingthreads-using tests failing loudly is better than a convincing-but-wrong stub.t/tie.t/t/tie_db.tfailures are caused by a broken user-local CPAN install ofDB_File(which requires XS) and aren't addressed here either.Test plan
makepasses (all unit tests)jcpan -t Memoize: unblockscorrectness.tandtie_storable.tno warnings "recursion"fib(20)or other non-pathological recursionGenerated with Devin