Skip to content

Commit 10631d7

Browse files
fix: improve Memoize test pass rate
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>
1 parent d19b693 commit 10631d7

3 files changed

Lines changed: 128 additions & 5 deletions

File tree

src/main/java/org/perlonjava/core/Configuration.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public final class Configuration {
3333
* Automatically populated by Gradle/Maven during build.
3434
* DO NOT EDIT MANUALLY - this value is replaced at build time.
3535
*/
36-
public static final String gitCommitId = "c9b8e05dd";
36+
public static final String gitCommitId = "fb4614791";
3737

3838
/**
3939
* Git commit date of the build (ISO format: YYYY-MM-DD).
@@ -48,7 +48,7 @@ public final class Configuration {
4848
* Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at"
4949
* DO NOT EDIT MANUALLY - this value is replaced at build time.
5050
*/
51-
public static final String buildTimestamp = "Apr 20 2026 17:15:38";
51+
public static final String buildTimestamp = "Apr 20 2026 18:52:44";
5252

5353
// Prevent instantiation
5454
private Configuration() {

src/main/java/org/perlonjava/runtime/perlmodule/Storable.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ public static void initialize() {
5454
storable.registerMethod("retrieve", null);
5555
storable.registerMethod("nstore", null);
5656
storable.registerMethod("dclone", null);
57+
storable.registerMethod("last_op_in_netorder", null);
5758

5859
storable.defineExport("EXPORT", "store", "retrieve", "nstore", "freeze", "thaw", "nfreeze", "dclone");
5960

@@ -82,12 +83,26 @@ public static void initialize() {
8283
// Magic byte to identify binary format (distinguishes from old YAML+GZIP format)
8384
private static final char BINARY_MAGIC = '\u00FF';
8485

86+
// Tracks whether the last freeze/store operation used network byte order.
87+
// Set true by nfreeze()/nstore(); set false by freeze()/store().
88+
// Exposed via Storable::last_op_in_netorder().
89+
private static volatile boolean lastOpInNetorder = false;
90+
91+
/**
92+
* Returns 1 if the last freeze/store operation used network byte order
93+
* (i.e. was nfreeze or nstore), 0 otherwise.
94+
*/
95+
public static RuntimeList last_op_in_netorder(RuntimeArray args, int ctx) {
96+
return new RuntimeScalar(lastOpInNetorder ? 1 : 0).getList();
97+
}
98+
8599
/**
86100
* Freezes data to a binary format matching Perl 5 Storable's sort order.
87101
* Uses type bytes compatible with Perl 5's Storable so that string comparison
88102
* of frozen output produces the same ordering as Perl 5.
89103
*/
90104
public static RuntimeList freeze(RuntimeArray args, int ctx) {
105+
lastOpInNetorder = false;
91106
if (args.isEmpty()) {
92107
return WarnDie.die(new RuntimeScalar("freeze: not enough arguments"), new RuntimeScalar("\n")).getList();
93108
}
@@ -432,13 +447,16 @@ private static long readLong(String data, int[] pos) {
432447
* Network freeze (same as freeze for now).
433448
*/
434449
public static RuntimeList nfreeze(RuntimeArray args, int ctx) {
435-
return freeze(args, ctx);
450+
RuntimeList result = freeze(args, ctx);
451+
lastOpInNetorder = true;
452+
return result;
436453
}
437454

438455
/**
439456
* Stores data to file using YAML format.
440457
*/
441458
public static RuntimeList store(RuntimeArray args, int ctx) {
459+
lastOpInNetorder = false;
442460
if (args.size() < 2) {
443461
return WarnDie.die(new RuntimeScalar("store: not enough arguments"), new RuntimeScalar("\n")).getList();
444462
}
@@ -479,7 +497,9 @@ public static RuntimeList retrieve(RuntimeArray args, int ctx) {
479497
* Network store (same as store).
480498
*/
481499
public static RuntimeList nstore(RuntimeArray args, int ctx) {
482-
return store(args, ctx);
500+
RuntimeList result = store(args, ctx);
501+
lastOpInNetorder = true;
502+
return result;
483503
}
484504

485505
/**

src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -343,6 +343,72 @@ public static void clearInlineMethodCache() {
343343
public boolean isMapGrepBlock = false;
344344
// Flag to indicate this code is an eval BLOCK - non-local return should propagate through it
345345
public boolean isEvalBlock = false;
346+
347+
// Depth of active recursive calls to this subroutine, used by the
348+
// "Deep recursion on subroutine" warning. Incremented on entry and
349+
// decremented in a finally-block on exit.
350+
public transient int callDepth = 0;
351+
// Whether a "Deep recursion" warning has already been emitted for the
352+
// currently-active recursion chain. Reset when callDepth returns to 0.
353+
public transient boolean deepRecursionWarned = false;
354+
355+
// Depth threshold for the "Deep recursion on subroutine" warning.
356+
// Matches Perl's default PERL_SUB_DEPTH_WARN value.
357+
public static final int DEEP_RECURSION_WARN_DEPTH = 100;
358+
359+
// When the tail-call trampoline in the static apply() re-enters a sub,
360+
// we want to skip the "Deep recursion" tracking for that entry.
361+
// The goto &sub caller's `no warnings 'recursion'` scope has already
362+
// unwound by the time the trampoline runs, so we can't honor it; and
363+
// tail calls don't consume real Java stack, so a depth warning for
364+
// them is misleading anyway. Nested tail-call trampolines use an int
365+
// counter so re-entries only skip tracking for the outermost trampoline.
366+
private static final ThreadLocal<Integer> inTailCallTrampoline =
367+
ThreadLocal.withInitial(() -> 0);
368+
369+
/**
370+
* Increment the recursion depth counter and, if we've just crossed the
371+
* "Deep recursion on subroutine" threshold for the first time in this
372+
* recursion chain, emit a warning under the "recursion" warnings category.
373+
* Must be matched with a call to exitCall() in a finally-block.
374+
*
375+
* Map/grep/eval blocks are exempt so that map { ... } and eval { ... }
376+
* don't report their dispatch wrapper. Tail-call trampoline re-entries
377+
* are also exempt — see inTailCallTrampoline.
378+
*/
379+
private void enterCall() {
380+
if (isMapGrepBlock || isEvalBlock || isBuiltin) {
381+
return;
382+
}
383+
if (inTailCallTrampoline.get() > 0) {
384+
return;
385+
}
386+
int depth = ++callDepth;
387+
if (depth > DEEP_RECURSION_WARN_DEPTH && !deepRecursionWarned) {
388+
deepRecursionWarned = true;
389+
String name = (packageName != null && subName != null)
390+
? packageName + "::" + subName
391+
: (subName != null ? subName : "__ANON__");
392+
WarnDie.warnWithCategory(
393+
new RuntimeScalar("Deep recursion on subroutine \"" + name + "\""),
394+
RuntimeScalarCache.scalarEmptyString,
395+
"recursion");
396+
}
397+
}
398+
399+
/** Paired with enterCall() — decrements the recursion counter. */
400+
private void exitCall() {
401+
if (isMapGrepBlock || isEvalBlock || isBuiltin) {
402+
return;
403+
}
404+
if (inTailCallTrampoline.get() > 0) {
405+
return;
406+
}
407+
if (--callDepth <= 0) {
408+
callDepth = 0;
409+
deepRecursionWarned = false;
410+
}
411+
}
346412
// State variables
347413
public Map<String, Boolean> stateVariableInitialized = new HashMap<>();
348414
public Map<String, RuntimeScalar> stateVariable = new HashMap<>();
@@ -2330,7 +2396,16 @@ public static RuntimeList apply(RuntimeScalar runtimeScalar, RuntimeArray a, int
23302396
&& cfList.getControlFlowType() == ControlFlowType.TAILCALL) {
23312397
RuntimeScalar tailCodeRef = cfList.getTailCallCodeRef();
23322398
RuntimeArray tailArgs = cfList.getTailCallArgs();
2333-
result = apply(tailCodeRef, tailArgs != null ? tailArgs : a, callContext);
2399+
// Mark trampoline re-entry so enterCall/exitCall skip depth
2400+
// tracking (tail calls don't consume real Java stack, and the
2401+
// goto site's lexical `no warnings 'recursion'` scope has
2402+
// already unwound — see enterCall() comments).
2403+
inTailCallTrampoline.set(inTailCallTrampoline.get() + 1);
2404+
try {
2405+
result = apply(tailCodeRef, tailArgs != null ? tailArgs : a, callContext);
2406+
} finally {
2407+
inTailCallTrampoline.set(inTailCallTrampoline.get() - 1);
2408+
}
23342409
}
23352410
// Mortal-ize blessed refs with refCount==0 in void-context calls.
23362411
// These are objects that were created but never stored in a named
@@ -2669,6 +2744,23 @@ public static RuntimeList apply(RuntimeScalar runtimeScalar, String subroutineNa
26692744
// Method to apply (execute) a subroutine reference (legacy method for compatibility)
26702745
public static RuntimeList apply(RuntimeScalar runtimeScalar, String subroutineName, RuntimeBase list, int callContext) {
26712746

2747+
// If this is a tail-call trampoline re-entry (emitted by the JVM bytecode
2748+
// trampoline for `goto &sub`), mark it so enterCall/exitCall skip depth
2749+
// tracking. See enterCall() / inTailCallTrampoline for the rationale.
2750+
boolean isTailCall = "tailcall".equals(subroutineName);
2751+
if (isTailCall) {
2752+
inTailCallTrampoline.set(inTailCallTrampoline.get() + 1);
2753+
try {
2754+
return applyImpl(runtimeScalar, subroutineName, list, callContext);
2755+
} finally {
2756+
inTailCallTrampoline.set(inTailCallTrampoline.get() - 1);
2757+
}
2758+
}
2759+
return applyImpl(runtimeScalar, subroutineName, list, callContext);
2760+
}
2761+
2762+
private static RuntimeList applyImpl(RuntimeScalar runtimeScalar, String subroutineName, RuntimeBase list, int callContext) {
2763+
26722764
// Handle tied scalars - fetch the underlying value first
26732765
if (runtimeScalar.type == RuntimeScalarType.TIED_SCALAR) {
26742766
return apply(runtimeScalar.tiedFetch(), subroutineName, list, callContext);
@@ -3163,6 +3255,11 @@ public RuntimeList apply(RuntimeArray a, int callContext) {
31633255
// See also: the 3-arg instance method apply(name, array, ctx) which pushes true.
31643256
hasArgsStack.get().push(false);
31653257

3258+
// Check deep recursion BEFORE pushing the callee's warning bits,
3259+
// so the "Deep recursion on subroutine" warning is gated on the
3260+
// caller's lexical warning bits (matching Perl's ckWARN at the
3261+
// call site, not inside the callee).
3262+
enterCall();
31663263
// Push warning bits for FATAL warnings support
31673264
String warningBits = getWarningBitsForCode(this);
31683265
if (warningBits != null) {
@@ -3183,6 +3280,7 @@ public RuntimeList apply(RuntimeArray a, int callContext) {
31833280
if (warningBits != null) {
31843281
WarningBitsRegistry.popCurrent();
31853282
}
3283+
exitCall();
31863284
popArgs(); // also pops hasArgsStack — see popArgs() implementation
31873285
if (DebugState.debugMode) {
31883286
DebugHooks.exitSubroutine();
@@ -3268,6 +3366,10 @@ public RuntimeList apply(String subroutineName, RuntimeArray a, int callContext)
32683366
// See also: the 2-arg instance method apply(array, ctx) which pushes false.
32693367
hasArgsStack.get().push(true);
32703368

3369+
// Check deep recursion BEFORE pushing the callee's warning bits,
3370+
// so the "Deep recursion on subroutine" warning is gated on the
3371+
// caller's lexical warning bits.
3372+
enterCall();
32713373
// Push warning bits for FATAL warnings support
32723374
String warningBits = getWarningBitsForCode(this);
32733375
if (warningBits != null) {
@@ -3288,6 +3390,7 @@ public RuntimeList apply(String subroutineName, RuntimeArray a, int callContext)
32883390
if (warningBits != null) {
32893391
WarningBitsRegistry.popCurrent();
32903392
}
3393+
exitCall();
32913394
popArgs(); // also pops hasArgsStack — see popArgs() implementation
32923395
if (DebugState.debugMode) {
32933396
DebugHooks.exitSubroutine();

0 commit comments

Comments
 (0)