Skip to content

fix: improve Memoize test pass rate (Storable::last_op_in_netorder, deep recursion warning)#516

Merged
fglock merged 1 commit intomasterfrom
fix/memoize-test-issues
Apr 20, 2026
Merged

fix: improve Memoize test pass rate (Storable::last_op_in_netorder, deep recursion warning)#516
fglock merged 1 commit intomasterfrom
fix/memoize-test-issues

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Apr 20, 2026

Summary

Two independent fixes targeting failures seen in ./jcpan -t Memoize:

  1. Storable::last_op_in_netorder — implements the flag reporting whether the most recent freeze/store used network byte order. Set by nfreeze()/nstore(), cleared by freeze()/store().
  2. "Deep recursion on subroutine" warningRuntimeCode now tracks per-sub recursion depth and warns (under the recursion warnings category) the first time depth exceeds 100, matching Perl's PERL_SUB_DEPTH_WARN behavior. Map/grep/eval blocks and builtins are exempt.

Memoize test results

Test Before After
t/correctness.t StackOverflowError after test 16 17/17 pass
t/tie_storable.t Died at test 6 (Undefined subroutine &Storable::last_op_in_netorder) 5/6 pass

The lone remaining tie_storable.t failure requires tied-hash DESTROY to fire when my %cache goes out of scope — a separate pre-existing limitation unrelated to these fixes.

Not included: threads->new / ->join stubs

An earlier version of this branch added new as an alias for create and a synchronous join that 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's t/threadsafe.t test 8 is exactly such a case — the "thread" calls unmemoize('count_up'), and with a synchronous join that really unmemoizes count_up on the main thread, so the main-thread assertion fails. Leaving threads-using tests failing loudly is better than a convincing-but-wrong stub. t/tie.t / t/tie_db.t failures are caused by a broken user-local CPAN install of DB_File (which requires XS) and aren't addressed here either.

Test plan

  • make passes (all unit tests)
  • jcpan -t Memoize: unblocks correctness.t and tie_storable.t
  • Deep recursion warning honors no warnings "recursion"
  • Deep recursion warning does not fire for fib(20) or other non-pathological recursion

Generated with Devin

@fglock fglock force-pushed the fix/memoize-test-issues branch from b40c0be to fb46147 Compare April 20, 2026 16:19
@fglock fglock changed the title fix: improve Memoize test pass rate (deep recursion warning, Storable::last_op_in_netorder, threads->new/join) fix: improve Memoize test pass rate (Storable::last_op_in_netorder, deep recursion warning) Apr 20, 2026
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>
@fglock fglock force-pushed the fix/memoize-test-issues branch from fb46147 to 10631d7 Compare April 20, 2026 16:53
@fglock fglock merged commit 3f276a9 into master Apr 20, 2026
2 checks passed
@fglock fglock deleted the fix/memoize-test-issues branch April 20, 2026 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant