diff --git a/dev/design/cpan_client.md b/dev/design/cpan_client.md index b727b507f..239b6fe71 100644 --- a/dev/design/cpan_client.md +++ b/dev/design/cpan_client.md @@ -581,38 +581,46 @@ DateTime.pm | Issue | Module Affected | Status | |-------|-----------------|--------| | ~~`${ $stash{NAME} }` dereference~~ | ~~Module::Implementation~~ | **FIXED** | -| Package::Stash::PP doesn't return true | Package::Stash | **BUG - needs investigation** | +| evalContext null in parallel tests | Test2::API INIT blocks | **FIXED** | +| Bare block return value at file level | Package::Stash::PP | **PARTIAL - TODO tests added** | -### Proposed Solutions +### evalContext Issue Resolution (2026-03-19) -#### Option A: Fix Package::Stash::PP (Recommended) +The JUnit parallel test failures ("ctx is null" errors) were caused by stale INIT blocks referencing cleared evalContext entries. -Investigate why Package::Stash::PP.pm doesn't return a true value. +**Root cause:** +1. Test A loads Test2::API which has `INIT { eval '...' }` +2. The INIT block's compiled bytecode references a specific evalTag +3. `resetAll()` clears evalContext but INIT blocks persisted +4. Test B triggers the stale INIT block → evalContext.get() returns null → NPE -**Files to investigate:** -- `~/.perlonjava/lib/Package/Stash/PP.pm` - Check for compilation errors +**Fix applied:** +1. Added null check in `RuntimeCode.evalStringHelper()` with descriptive error +2. Clear special blocks (INIT/END/CHECK) in `GlobalVariable.resetAllGlobals()` +3. Updated JUnit test harness to skip TODO tests (`# TODO` in TAP output) -#### Option B: Create Bundled DateTime.pm +### Bare Block Return Value Issue -Create a simplified `DateTime.pm` in `src/main/perl/lib/` that: -1. Skips heavy dependencies (namespace::autoclean, Specio) -2. Uses our Java XS implementation or DateTime::PP -3. Provides core DateTime functionality +**Problem:** Files ending with a bare block `{ ... }` return `undef` instead of the block's last expression value. -### Investigation Commands +**Example:** +```perl +# File ends with: +{ 42; } +# do "file" returns undef in PerlOnJava, 42 in Perl +``` -```bash -# Test symbol table dereference (NOW FIXED) -./jperl -e 'package Foo; our $VERSION = "1.0"; print ${ $Foo::{VERSION} }, "\n"' -# Output: 1.0 (correct) +**Attempted fix:** Change context for bare blocks in RUNTIME context (EmitBlock.java/EmitStatement.java) -# Test DateTime - still blocked by namespace::autoclean -./jperl -MDateTime -e 'print DateTime->now->ymd, "\n"' -# Error: Can't locate namespace/autoclean.pm -``` +**Result:** Causes ASM stack frame verification failures because changing context for the block affects bytecode generation for ALL statements inside it, not just the return value capture. + +**Current status:** Tests for this feature are marked as TODO. The fix requires a more sophisticated approach that captures the return value without changing how interior statements compile. + +**Files changed:** +- `src/test/resources/unit/bare_block_return.t` - TODO tests for bare block return values ### Next Steps -1. **Install namespace::autoclean dependencies** via jcpan to test further -2. **If blocked by Package::Stash::PP**, investigate why modules ending with `sub foo { }` return undef +1. **Investigate bare block return value fix** - Need to capture last expression value without changing interior statement compilation +2. **Test Package::Stash::PP loading** - May need explicit `1;` at end of file 3. **Alternative**: Create bundled DateTime.pm wrapper that skips heavy dependencies diff --git a/src/main/java/org/perlonjava/backend/jvm/EmitBlock.java b/src/main/java/org/perlonjava/backend/jvm/EmitBlock.java index 28034310a..97af54b0e 100644 --- a/src/main/java/org/perlonjava/backend/jvm/EmitBlock.java +++ b/src/main/java/org/perlonjava/backend/jvm/EmitBlock.java @@ -255,11 +255,21 @@ public static void emitBlock(EmitterVisitor emitterVisitor, BlockNode node) { ByteCodeSourceMapper.setDebugInfoLineNumber(emitterVisitor.ctx, element.getIndex()); + // Check if this block should store its result in a register (for bare block expressions) + Object resultRegObj = node.getAnnotation("resultRegister"); + int resultReg = (resultRegObj instanceof Integer) ? (Integer) resultRegObj : -1; + // Emit the statement with current context if (i == lastNonNullIndex) { // Special case for the last element if (CompilerOptions.DEBUG_ENABLED) emitterVisitor.ctx.logDebug("Last element: " + element); - element.accept(emitterVisitor); + if (resultReg >= 0) { + // Visit in SCALAR context to get a value, store it, then pop + element.accept(emitterVisitor.with(RuntimeContextType.SCALAR)); + mv.visitVarInsn(Opcodes.ASTORE, resultReg); + } else { + element.accept(emitterVisitor); + } } else { // General case for all other elements if (CompilerOptions.DEBUG_ENABLED) emitterVisitor.ctx.logDebug("Element: " + element); diff --git a/src/main/java/org/perlonjava/backend/jvm/EmitStatement.java b/src/main/java/org/perlonjava/backend/jvm/EmitStatement.java index f1522a190..b1da51f4a 100644 --- a/src/main/java/org/perlonjava/backend/jvm/EmitStatement.java +++ b/src/main/java/org/perlonjava/backend/jvm/EmitStatement.java @@ -181,7 +181,29 @@ public static void emitFor3(EmitterVisitor emitterVisitor, For3Node node) { isUnlabeledTarget); // Visit the loop body - node.body.accept(voidVisitor); + // For simple blocks (bare blocks like { ... }) in non-void context, + // use register spilling to capture the result: allocate a local variable, + // tell the block to store its last element's value there, then load it after. + // This ensures consistent stack state across all code paths. + // Only apply for SCALAR/LIST contexts, not RUNTIME (which is for subroutine bodies). + boolean needsReturnValue = node.isSimpleBlock + && (emitterVisitor.ctx.contextType == RuntimeContextType.SCALAR + || emitterVisitor.ctx.contextType == RuntimeContextType.LIST); + if (needsReturnValue) { + // Allocate a local variable for the result + int resultReg = emitterVisitor.ctx.symbolTable.allocateLocalVariable(); + // Initialize it to undef + EmitOperator.emitUndef(mv); + mv.visitVarInsn(Opcodes.ASTORE, resultReg); + // Tell the block to store its last element's value in this register + node.body.setAnnotation("resultRegister", resultReg); + // Visit body in VOID context (consistent stack state) + node.body.accept(voidVisitor); + // Load the result + mv.visitVarInsn(Opcodes.ALOAD, resultReg); + } else { + node.body.accept(voidVisitor); + } } else { // Within a `while` modifier, next/redo/last labels are not active @@ -228,7 +250,11 @@ public static void emitFor3(EmitterVisitor emitterVisitor, For3Node node) { } // If the context is not VOID, push "undef" to the stack - if (emitterVisitor.ctx.contextType != RuntimeContextType.VOID) { + // Exception: for simple blocks in SCALAR/LIST context, we already loaded the result from the register + boolean simpleBlockHandled = node.isSimpleBlock + && (emitterVisitor.ctx.contextType == RuntimeContextType.SCALAR + || emitterVisitor.ctx.contextType == RuntimeContextType.LIST); + if (emitterVisitor.ctx.contextType != RuntimeContextType.VOID && !simpleBlockHandled) { EmitOperator.emitUndef(emitterVisitor.ctx.mv); } diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java index 018b12141..c588fc9fc 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/GlobalVariable.java @@ -76,6 +76,13 @@ public static void resetAllGlobals() { RuntimeCode.clearCaches(); + // Clear special blocks (INIT, END, CHECK, UNITCHECK) to prevent stale code references. + // When the classloader is replaced, old INIT blocks may reference evalTags that no longer + // exist in the cleared evalContext, causing "ctx is null" errors. + SpecialBlock.initBlocks.elements.clear(); + SpecialBlock.endBlocks.elements.clear(); + SpecialBlock.checkBlocks.elements.clear(); + // Method resolution caches can grow across test scripts. InheritanceResolver.invalidateCache(); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java index 11f3d5ece..a354c9d22 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java @@ -368,6 +368,19 @@ public static Class evalStringHelper(RuntimeScalar code, String evalTag, Obje // Retrieve the eval context that was saved at program compile-time EmitterContext ctx = RuntimeCode.evalContext.get(evalTag); + // Handle missing eval context - this can happen when compiled code (e.g., INIT blocks + // with eval) is executed after the runtime has been reset. In JUnit parallel tests, + // PerlLanguageProvider.resetAll() clears evalContext between tests, but compiled + // bytecode (which references specific evalTags) survives and may be re-executed. + if (ctx == null) { + throw new RuntimeException( + "Eval context not found for tag: " + evalTag + + ". This can happen when eval is called from code that was compiled " + + "in a previous session (e.g., INIT blocks in cached modules). " + + "The module may need to be reloaded. " + + "If this occurs in tests, ensure module caches are cleared along with eval contexts."); + } + // Save the current scope so we can restore it after eval compilation. // This is critical because eval may be called from code compiled with different // warning/feature flags than the caller, and we must not leak the eval's scope. @@ -818,6 +831,19 @@ public static RuntimeList evalStringWithInterpreter( // Retrieve the eval context that was saved at program compile-time EmitterContext ctx = RuntimeCode.evalContext.get(evalTag); + // Handle missing eval context - this can happen when compiled code (e.g., INIT blocks + // with eval) is executed after the runtime has been reset. In JUnit parallel tests, + // PerlLanguageProvider.resetAll() clears evalContext between tests, but compiled + // bytecode (which references specific evalTags) survives and may be re-executed. + if (ctx == null) { + throw new RuntimeException( + "Eval context not found for tag: " + evalTag + + ". This can happen when eval is called from code that was compiled " + + "in a previous session (e.g., INIT blocks in cached modules). " + + "The module may need to be reloaded. " + + "If this occurs in tests, ensure module caches are cleared along with eval contexts."); + } + // Save the current scope so we can restore it after eval execution. // This is critical because eval may be called from code compiled with different // warning/feature flags than the caller, and we must not leak the eval's scope. diff --git a/src/main/perl/lib/ExtUtils/MakeMaker.pm b/src/main/perl/lib/ExtUtils/MakeMaker.pm index fef75abde..d360962c2 100644 --- a/src/main/perl/lib/ExtUtils/MakeMaker.pm +++ b/src/main/perl/lib/ExtUtils/MakeMaker.pm @@ -22,6 +22,24 @@ require ExtUtils::MM; # Installation directory (configurable via environment) our $INSTALL_BASE = $ENV{PERLONJAVA_LIB}; +# Parse command-line arguments like INSTALL_BASE=/path +# This is called by Makefile.PL scripts that use MY->parse_args(@ARGV) +sub parse_args { + my ($self, @args) = @_; + $self = {} unless ref $self; + foreach (@args) { + next unless m/(.*?)=(.*)/; + my ($name, $value) = ($1, $2); + # Expand ~ in paths + if ($value =~ m/^~/) { + my $home = $ENV{HOME} || $ENV{USERPROFILE} || '.'; + $value =~ s/^~/$home/; + } + $self->{ARGS}{uc $name} = $self->{uc $name} = $value; + } + return $self; +} + # Find the default lib directory sub _default_install_base { # Check if running from JAR diff --git a/src/main/perl/lib/Package/Stash/Conflicts.pm b/src/main/perl/lib/Package/Stash/Conflicts.pm new file mode 100644 index 000000000..65e7f1d7b --- /dev/null +++ b/src/main/perl/lib/Package/Stash/Conflicts.pm @@ -0,0 +1,35 @@ +package Package::Stash::Conflicts; +use strict; +use warnings; + +our $VERSION = '0.40'; + +# PerlOnJava stub - conflict checking not needed +# +# The original module uses Dist::CheckConflicts to verify there are no +# conflicting versions of Package::Stash with old versions of: +# - Class::MOP 1.08 +# - MooseX::Method::Signatures 0.36 +# - MooseX::Role::WithOverloading 0.08 +# - namespace::clean 0.18 +# +# In PerlOnJava's environment, these old conflicting versions don't exist, +# so we provide a no-op stub. + +sub check_conflicts { } + +1; + +__END__ + +=head1 NAME + +Package::Stash::Conflicts - PerlOnJava stub for conflict checking + +=head1 DESCRIPTION + +This is a stub implementation for PerlOnJava. The original module checks +for conflicting versions of Package::Stash, but this is not needed in +PerlOnJava's environment. + +=cut diff --git a/src/test/java/org/perlonjava/PerlScriptExecutionTest.java b/src/test/java/org/perlonjava/PerlScriptExecutionTest.java index 0a74d6204..887246242 100644 --- a/src/test/java/org/perlonjava/PerlScriptExecutionTest.java +++ b/src/test/java/org/perlonjava/PerlScriptExecutionTest.java @@ -265,7 +265,8 @@ private void executeTest(String filename) { lineNumber++; // Check for test failures - works with both Test::More TAP format and simple tests - if (line.trim().startsWith("not ok")) { + // Skip TODO tests (they are expected to fail) - TAP format: "not ok ... # TODO ..." + if (line.trim().startsWith("not ok") && !line.contains("# TODO")) { errorFound = true; fail("Test failure in " + filename + " at line " + lineNumber + ": " + line); break; diff --git a/src/test/resources/unit/bare_block_return.t b/src/test/resources/unit/bare_block_return.t new file mode 100644 index 000000000..593077c7d --- /dev/null +++ b/src/test/resources/unit/bare_block_return.t @@ -0,0 +1,257 @@ +use strict; +use warnings; +use Test::More; + +# Test that bare blocks return their last expression value in different contexts +# This is important for modules that end with a bare block containing +# lexical variables (like Package::Stash::PP) + +# ============================================================ +# VOID context tests - bare blocks should execute without error +# ============================================================ + +# Test: Bare block in void context +{ + my $executed = 0; + { $executed = 1; 42; } + is($executed, 1, 'bare block in void context executes'); +} + +# Test: Bare block with subroutine call in void context +{ + my $result; + sub set_result { $result = shift; return 99; } + { set_result(42); } + is($result, 42, 'bare block with sub call in void context'); +} + +# ============================================================ +# SCALAR context tests - bare blocks should return last value +# ============================================================ + +# Test: Simple bare block in scalar context +{ + my $val = do { 42; }; + is($val, 42, 'bare block in scalar context returns value'); +} + +# Test: Bare block with lexical in scalar context +{ + my $val = do { my $x = 99; $x; }; + is($val, 99, 'bare block with lexical in scalar context'); +} + +# Test: Bare block with multiple statements in scalar context +{ + my $val = do { my $a = 10; my $b = 20; $a + $b; }; + is($val, 30, 'bare block with multiple statements in scalar context'); +} + +# Test: Nested bare blocks in scalar context +{ + my $val = do { { { 123; } } }; + is($val, 123, 'nested bare blocks in scalar context'); +} + +# Test: Bare block with hash in scalar context (Package::Stash::PP pattern) +{ + my $val = do { my %h = (a => 1, b => 2); scalar keys %h; }; + is($val, 2, 'bare block with hash in scalar context'); +} + +# ============================================================ +# LIST context tests - bare blocks should return last value(s) +# ============================================================ + +# Test: Bare block returning single value in list context +{ + my @vals = do { 42; }; + is_deeply(\@vals, [42], 'bare block returning scalar in list context'); +} + +# Test: Bare block returning list in list context +{ + my @vals = do { (1, 2, 3); }; + is_deeply(\@vals, [1, 2, 3], 'bare block returning list in list context'); +} + +# Test: Bare block returning array in list context +{ + my @vals = do { my @arr = (4, 5, 6); @arr; }; + is_deeply(\@vals, [4, 5, 6], 'bare block returning array in list context'); +} + +# Test: Bare block with hash in list context +{ + my %h = do { (a => 1, b => 2); }; + is_deeply(\%h, {a => 1, b => 2}, 'bare block returning hash pairs in list context'); +} + +# ============================================================ +# RUNTIME context tests (file-level for require/do) +# These test the actual bug fix for Package::Stash::PP +# ============================================================ + +use File::Temp qw(tempfile); + +# TODO: The following tests are for bare block return values in RUNTIME context +# (do "file", require "file"). This feature is not yet fully implemented. +# The fix is complex because changing the context for bare blocks affects +# bytecode generation and can cause ASM stack frame verification failures. +# See cpan_client.md Phase 11a for details. + +# Test: Simple bare block return value via do-file +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "{ 42; }\n"; + close $fh; + my $result = do $filename; + is($result, 42, 'bare block in file (RUNTIME) returns last expression'); +} + +# Test: Bare block with lexical variable via do-file +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "{ my \$x = 99; \$x; }\n"; + close $fh; + my $result = do $filename; + is($result, 99, 'bare block with lexical in file (RUNTIME)'); +} + +# Test: Bare block with hash via do-file (Package::Stash::PP pattern) +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "{ my \%h = (a => 1, b => 2); scalar keys \%h; }\n"; + close $fh; + my $result = do $filename; + is($result, 2, 'bare block with hash in file (RUNTIME)'); +} + +# Test: Module ending with bare block returns true for require +# Note: This test has explicit `1;` inside the block, but due to the bare block +# return value issue, the file doesn't return true. Wrap in TODO. +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pm', UNLINK => 1); + print $fh <<'EOF'; +package TestModuleBareBlock; +{ + my %MAP = ('$' => 'SCALAR', '@' => 'ARRAY'); + sub get_type { $MAP{$_[0]} } + 1; # true value inside block +} +EOF + close $fh; + my $result = eval { require $filename }; + if ($@) { + fail('module with bare block loads successfully (RUNTIME)'); + fail('subroutine in bare block works'); + } else { + is($result, 1, 'module with bare block loads successfully (RUNTIME)'); + is(TestModuleBareBlock::get_type('@'), 'ARRAY', 'subroutine in bare block works'); + } +} + +# Test: Nested bare blocks via do-file +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "{ { { 123; } } }\n"; + close $fh; + my $result = do $filename; + is($result, 123, 'nested bare blocks in file (RUNTIME)'); +} + +# Test: Bare block as last statement after other statements via do-file +TODO: { + local $TODO = "Bare block return value in RUNTIME context not yet implemented"; + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "my \$x = 1; { \$x + 100; }\n"; + close $fh; + my $result = do $filename; + is($result, 101, 'bare block as last statement in file (RUNTIME)'); +} + +# ============================================================ +# Labeled block tests - should NOT be affected by the fix +# ============================================================ + +# Test: Labeled block in void context (Test::More TODO pattern) +{ + my $executed = 0; + SKIP: { $executed = 1; } + is($executed, 1, 'labeled block in void context executes'); +} + +# Test: Labeled block with last +{ + my $count = 0; + OUTER: { + $count++; + last OUTER; + $count++; + } + is($count, 1, 'labeled block with last works correctly'); +} + +# ============================================================ +# Complex bare block tests - Test::More functions inside blocks +# These test the actual problematic patterns +# ============================================================ + +# Test: Bare block containing Test::More ok() call +{ + my $result; + { $result = ok(1, 'ok inside bare block'); } + ok($result, 'bare block with ok() executes correctly'); +} + +# Test: Bare block containing Test::More is() call +{ + my $result; + { $result = is(1, 1, 'is inside bare block'); } + ok($result, 'bare block with is() executes correctly'); +} + +# Test: Bare block containing is_deeply() call +{ + my $result; + { $result = is_deeply([1,2], [1,2], 'is_deeply inside bare block'); } + ok($result, 'bare block with is_deeply() executes correctly'); +} + +# Test: File with bare block containing function calls +# TODO: This test fails due to stack frame issues when Test::More ok() is called +# inside a bare block in RUNTIME (file) context. The register spilling mechanism +# in For3Node has issues with complex control flow patterns. +{ + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh <<'EOF'; +use Test::More; +{ ok(1, 'test in file bare block'); 42; } +EOF + close $fh; + my $result = do $filename; + TODO: { + local $TODO = 'Stack frame issues with Test::More in file-level bare blocks'; + is($result, 42, 'file with bare block containing ok() returns value'); + } +} + +# Test: File with ONLY a bare block containing ok() - minimum repro +# TODO: Same issue as above +{ + my ($fh, $filename) = tempfile(SUFFIX => '.pl', UNLINK => 1); + print $fh "use Test::More; { ok(1); }\n"; + close $fh; + my $result = do $filename; + TODO: { + local $TODO = 'Stack frame issues with Test::More in file-level bare blocks'; + ok(defined($result), 'file with bare block ok() returns defined value'); + } +} + +done_testing();