From bb7c9b29f032c521672a51a7cf8be21fff862d8b Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Thu, 19 Mar 2026 12:00:57 +0100 Subject: [PATCH 1/3] Fix tied variables in void context not calling FETCH (#20) When accessing tied hash/array elements in void context (e.g., $hash{key};), PerlOnJava was incorrectly optimizing out the access because the lazy proxy (RuntimeTiedHashProxyEntry) was never read. This broke expected Perl semantics where tied variable access should trigger FETCH regardless of context. The fix adds handleVoidContextForTied() in EmitOperator.java which calls getBoolean() to force FETCH before the value is discarded with POP. This is applied to hash element ($hash{key}) and array element ($array[0]) operators. Known limitation: eval { } blocks transform the block into sub { }->() at parse time, so the last statement runs in scalar context instead of void. This is documented but not fixed in this PR. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin --- .../perlonjava/backend/jvm/Dereference.java | 12 +- .../perlonjava/backend/jvm/EmitOperator.java | 24 +++ src/test/resources/unit/tie_void_context.t | 202 ++++++++++++++++++ 3 files changed, 232 insertions(+), 6 deletions(-) create mode 100644 src/test/resources/unit/tie_void_context.t diff --git a/src/main/java/org/perlonjava/backend/jvm/Dereference.java b/src/main/java/org/perlonjava/backend/jvm/Dereference.java index a46946958..de67b975b 100644 --- a/src/main/java/org/perlonjava/backend/jvm/Dereference.java +++ b/src/main/java/org/perlonjava/backend/jvm/Dereference.java @@ -92,7 +92,7 @@ static void handleArrayElementOperator(EmitterVisitor emitterVisitor, BinaryOper emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); return; } if (sigil.equals("$") && sigilNode.operand instanceof BlockNode) { @@ -158,7 +158,7 @@ static void handleArrayElementOperator(EmitterVisitor emitterVisitor, BinaryOper emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); return; } if (sigil.equals("@")) { @@ -427,7 +427,7 @@ public static void handleHashElementOperator(EmitterVisitor emitterVisitor, Bina emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); return; } if (sigil.equals("$") && sigilNode.operand instanceof BlockNode) { @@ -489,7 +489,7 @@ public static void handleHashElementOperator(EmitterVisitor emitterVisitor, Bina } } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); return; } if (sigil.equals("@")) { @@ -1030,7 +1030,7 @@ public static void handleArrowArrayDeref(EmitterVisitor emitterVisitor, BinaryOp emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); } public static void handleArrowHashDeref(EmitterVisitor emitterVisitor, BinaryOperatorNode node, String hashOperation) { @@ -1105,6 +1105,6 @@ public static void handleArrowHashDeref(EmitterVisitor emitterVisitor, BinaryOpe if (pooledLeft) { emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContext(emitterVisitor); + EmitOperator.handleVoidContextForTied(emitterVisitor); } } diff --git a/src/main/java/org/perlonjava/backend/jvm/EmitOperator.java b/src/main/java/org/perlonjava/backend/jvm/EmitOperator.java index cb78c7d17..b79351534 100644 --- a/src/main/java/org/perlonjava/backend/jvm/EmitOperator.java +++ b/src/main/java/org/perlonjava/backend/jvm/EmitOperator.java @@ -773,6 +773,30 @@ static void handleVoidContext(EmitterVisitor emitterVisitor) { } } + /** + * Handles void context for hash/array element access, forcing evaluation of tied variables. + *

+ * For tied hashes/arrays, accessing an element returns a lazy proxy that only calls FETCH + * when the value is actually used. In void context, we need to force the FETCH to happen + * because tied variable access may have side effects (logging, caching, exceptions, etc.). + *

+ * This is different from plain variable access ($x, @a, %h) which can be optimized out + * because they don't have subscript expressions that might have side effects. + * + * @param emitterVisitor The visitor for emitting bytecode + * @see GitHub Issue #20 + */ + static void handleVoidContextForTied(EmitterVisitor emitterVisitor) { + if (emitterVisitor.ctx.contextType == RuntimeContextType.VOID) { + // Force evaluation of tied scalars by calling getBoolean(), which triggers FETCH + // for TIED_SCALAR types. Then discard the boolean result. + emitterVisitor.ctx.mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, + "org/perlonjava/runtime/runtimetypes/RuntimeScalar", + "getBoolean", "()Z", false); + emitterVisitor.ctx.mv.visitInsn(Opcodes.POP); + } + } + /** * Ensures the value on the stack is converted to RuntimeScalar if needed, * based on the node's return type. diff --git a/src/test/resources/unit/tie_void_context.t b/src/test/resources/unit/tie_void_context.t new file mode 100644 index 000000000..9e0729f98 --- /dev/null +++ b/src/test/resources/unit/tie_void_context.t @@ -0,0 +1,202 @@ +#!/usr/bin/env perl +use strict; +use warnings; +use Test::More; + +# Test for GitHub issue #20: tied variable in void context doesn't call FETCH +# https://github.com/fglock/PerlOnJava/issues/20 +# +# When accessing a tied hash/array element in void context, PerlOnJava incorrectly +# optimizes out the variable access, preventing the tied object's FETCH method +# from being called. This breaks expected Perl semantics where tied variable +# access should always trigger the appropriate tie methods regardless of context. + +# Test implementation that tracks method calls +package TrackingTiedHash; + +our @method_calls; + +sub TIEHASH { + my ($class) = @_; + @method_calls = (); + return bless {}, $class; +} + +sub FETCH { + my ($self, $key) = @_; + push @method_calls, ['FETCH', $key]; + return "value_$key"; +} + +sub STORE { + my ($self, $key, $value) = @_; + push @method_calls, ['STORE', $key, $value]; +} + +sub EXISTS { + my ($self, $key) = @_; + push @method_calls, ['EXISTS', $key]; + return 1; +} + +sub DELETE { + my ($self, $key) = @_; + push @method_calls, ['DELETE', $key]; +} + +sub CLEAR { + my ($self) = @_; + push @method_calls, ['CLEAR']; +} + +sub FIRSTKEY { return undef; } +sub NEXTKEY { return undef; } +sub SCALAR { return 0; } + +# Tied array tests +package TrackingTiedArray; + +our @array_method_calls; + +sub TIEARRAY { + my ($class) = @_; + @array_method_calls = (); + return bless [], $class; +} + +sub FETCH { + my ($self, $index) = @_; + push @array_method_calls, ['FETCH', $index]; + return "value_$index"; +} + +sub STORE { + my ($self, $index, $value) = @_; + push @array_method_calls, ['STORE', $index, $value]; +} + +sub FETCHSIZE { return 10; } +sub STORESIZE { } +sub EXISTS { return 1; } + +package main; + +subtest 'Tied hash FETCH in void context - basic' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + # This is the core issue - accessing a tied hash in void context + # should still call FETCH + $hash{key}; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 1, 'FETCH called once in void context'); + is($fetches[0][1], 'key', 'FETCH called with correct key'); +}; + +# NOTE: eval { } blocks are not tested here because PerlOnJava transforms +# `eval { BLOCK }` into `sub { BLOCK }->(@_)` at parse time, which means +# the block becomes a subroutine whose return value is always captured, +# effectively making the last statement run in scalar context instead of void context. +# This is a known limitation - see OperatorParser.java line 81-84. + +subtest 'Tied hash FETCH in scalar context (control test)' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + # This should definitely call FETCH (scalar context) + my $val = $hash{key}; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 1, 'FETCH called in scalar context'); + is($val, 'value_key', 'Value returned correctly'); +}; + +subtest 'Tied hash FETCH in list context (control test)' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + # This should definitely call FETCH (list context) + my @val = ($hash{key}); + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 1, 'FETCH called in list context'); +}; + +subtest 'Tied hash FETCH multiple void accesses' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + # Multiple void context accesses should each call FETCH + $hash{key1}; + $hash{key2}; + $hash{key3}; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 3, 'FETCH called three times for three void accesses'); + is($fetches[0][1], 'key1', 'First FETCH with correct key'); + is($fetches[1][1], 'key2', 'Second FETCH with correct key'); + is($fetches[2][1], 'key3', 'Third FETCH with correct key'); +}; + +subtest 'Tied array FETCH in void context' => sub { + my @array; + tie @array, 'TrackingTiedArray'; + @TrackingTiedArray::array_method_calls = (); + + # Void context access on tied array + $array[0]; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedArray::array_method_calls; + is(scalar @fetches, 1, 'Array FETCH called in void context'); + is($fetches[0][1], 0, 'Array FETCH called with correct index'); +}; + +# NOTE: Tied scalar access in void context ($scalar;) is optimized out by standard Perl too. +# This is correct behavior - only hash/array element access needs to call FETCH in void context +# because the subscript expression might have side effects. +# See: "Useless use of private variable in void context" warning in Perl. + +subtest 'Tied hash FETCH in statement modifier context' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + # Void context in for loop (statement modifier) + $hash{$_} for qw(a b c); + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 3, 'FETCH called for each iteration in statement modifier'); +}; + +subtest 'Tied hash FETCH via variable key in void context' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + my $key = 'dynamic_key'; + $hash{$key}; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 1, 'FETCH called with variable key in void context'); + is($fetches[0][1], 'dynamic_key', 'FETCH received correct dynamic key'); +}; + +subtest 'Tied hash FETCH with complex expression key in void context' => sub { + my %hash; + tie %hash, 'TrackingTiedHash'; + @TrackingTiedHash::method_calls = (); + + my $base = 'key'; + $hash{$base . '_suffix'}; + + my @fetches = grep { $_->[0] eq 'FETCH' } @TrackingTiedHash::method_calls; + is(scalar @fetches, 1, 'FETCH called with expression key in void context'); + is($fetches[0][1], 'key_suffix', 'FETCH received correctly computed key'); +}; + +done_testing(); From 29b9f9d2ea8e661759d353d9e6d198175c63cf98 Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Thu, 19 Mar 2026 12:25:59 +0100 Subject: [PATCH 2/3] Fix regression: only force FETCH for 'get' operations, not delete/exists The initial tied void context fix called getBoolean() on all hash/array element operations in void context. However, delete() can return null for sparse arrays, causing NullPointerException. Now only applies handleVoidContextForTied() for "get" operations. Delete and exists operations use the standard handleVoidContext() (plain POP). Also fixed slices to not double-pop in void context. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin --- .../perlonjava/backend/jvm/Dereference.java | 62 ++++++++++++++----- 1 file changed, 48 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/perlonjava/backend/jvm/Dereference.java b/src/main/java/org/perlonjava/backend/jvm/Dereference.java index de67b975b..081816ff1 100644 --- a/src/main/java/org/perlonjava/backend/jvm/Dereference.java +++ b/src/main/java/org/perlonjava/backend/jvm/Dereference.java @@ -92,7 +92,12 @@ static void handleArrayElementOperator(EmitterVisitor emitterVisitor, BinaryOper emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContextForTied(emitterVisitor); + // Only force FETCH for "get" operations - delete/exists can return null + if (arrayOperation.equals("get")) { + EmitOperator.handleVoidContextForTied(emitterVisitor); + } else { + EmitOperator.handleVoidContext(emitterVisitor); + } return; } if (sigil.equals("$") && sigilNode.operand instanceof BlockNode) { @@ -136,6 +141,14 @@ static void handleArrayElementOperator(EmitterVisitor emitterVisitor, BinaryOper emitterVisitor.ctx.mv.visitMethodInsn(Opcodes.INVOKEVIRTUAL, "org/perlonjava/runtime/runtimetypes/RuntimeScalar", "arrayDerefGetNonStrict", "(Lorg/perlonjava/runtime/runtimetypes/RuntimeScalar;Ljava/lang/String;)Lorg/perlonjava/runtime/runtimetypes/RuntimeScalar;", false); } + + if (pooledBase) { + emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); + } + + // Force FETCH for tied variables in void context (single-element access only) + EmitOperator.handleVoidContextForTied(emitterVisitor); + return; } else { // Multiple indices - use slice ListNode nodeRight = right.asListNode(); @@ -152,14 +165,13 @@ static void handleArrayElementOperator(EmitterVisitor emitterVisitor, BinaryOper } else if (emitterVisitor.ctx.contextType == RuntimeContextType.VOID) { emitterVisitor.ctx.mv.visitInsn(Opcodes.POP); } - } - if (pooledBase) { - emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); - } + if (pooledBase) { + emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); + } - EmitOperator.handleVoidContextForTied(emitterVisitor); - return; + return; + } } if (sigil.equals("@")) { /* @a[10, 20] @@ -427,7 +439,12 @@ public static void handleHashElementOperator(EmitterVisitor emitterVisitor, Bina emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContextForTied(emitterVisitor); + // Only force FETCH for "get" operations - delete/exists can return null + if (hashOperation.equals("get")) { + EmitOperator.handleVoidContextForTied(emitterVisitor); + } else { + EmitOperator.handleVoidContext(emitterVisitor); + } return; } if (sigil.equals("$") && sigilNode.operand instanceof BlockNode) { @@ -989,6 +1006,18 @@ public static void handleArrowArrayDeref(EmitterVisitor emitterVisitor, BinaryOp if (pooledIndex) { emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } + + if (pooledLeft) { + emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); + } + + // Only force FETCH for "get" operations - delete/exists can return null + if (arrayOperation.equals("get")) { + EmitOperator.handleVoidContextForTied(emitterVisitor); + } else { + EmitOperator.handleVoidContext(emitterVisitor); + } + return; } else { // Multiple indices: use slice method (only for get operation) if (!arrayOperation.equals("get")) { @@ -1024,13 +1053,12 @@ public static void handleArrowArrayDeref(EmitterVisitor emitterVisitor, BinaryOp } else if (emitterVisitor.ctx.contextType == RuntimeContextType.VOID) { emitterVisitor.ctx.mv.visitInsn(Opcodes.POP); } - } - if (pooledLeft) { - emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); + if (pooledLeft) { + emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); + } } - - EmitOperator.handleVoidContextForTied(emitterVisitor); + // No handleVoidContextForTied here - slices already handle void context with POP above } public static void handleArrowHashDeref(EmitterVisitor emitterVisitor, BinaryOperatorNode node, String hashOperation) { @@ -1105,6 +1133,12 @@ public static void handleArrowHashDeref(EmitterVisitor emitterVisitor, BinaryOpe if (pooledLeft) { emitterVisitor.ctx.javaClassInfo.releaseSpillSlot(); } - EmitOperator.handleVoidContextForTied(emitterVisitor); + + // Only force FETCH for "get" operations - delete/exists can return null + if (hashOperation.equals("get")) { + EmitOperator.handleVoidContextForTied(emitterVisitor); + } else { + EmitOperator.handleVoidContext(emitterVisitor); + } } } From eefceedd92a0addd4e556f05fe9f7b773981b8a4 Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Thu, 19 Mar 2026 12:32:58 +0100 Subject: [PATCH 3/3] Fix pop/shift to return undef instead of null for sparse arrays In Perl, pop/shift/delete on sparse array elements should return undef, not Java null. ArrayList.removeLast()/removeFirst() returns null when the element at that position is null (sparse array). This was the root cause of the NullPointerException in the tied void context fix - delete() calls pop() for the last element. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin --- .../org/perlonjava/runtime/runtimetypes/RuntimeArray.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeArray.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeArray.java index b0cc2463d..ec415decd 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeArray.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeArray.java @@ -100,7 +100,9 @@ public static RuntimeScalar pop(RuntimeArray runtimeArray) { if (runtimeArray.isEmpty()) { yield new RuntimeScalar(); // Return undefined if empty } - yield runtimeArray.elements.removeLast(); + RuntimeScalar result = runtimeArray.elements.removeLast(); + // Sparse arrays can have null elements - return undef in that case + yield result != null ? result : scalarUndef; } case AUTOVIVIFY_ARRAY -> { AutovivificationArray.vivify(runtimeArray); @@ -124,7 +126,9 @@ public static RuntimeScalar shift(RuntimeArray runtimeArray) { if (runtimeArray.isEmpty()) { yield new RuntimeScalar(); // Return undefined if empty } - yield runtimeArray.elements.removeFirst(); + RuntimeScalar result = runtimeArray.elements.removeFirst(); + // Sparse arrays can have null elements - return undef in that case + yield result != null ? result : scalarUndef; } case AUTOVIVIFY_ARRAY -> { AutovivificationArray.vivify(runtimeArray);