From 0f8b355b2ebd85ebda2a61fc6d626816bd20b2f5 Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Tue, 21 Apr 2026 21:11:02 +0200 Subject: [PATCH] fix(state): share storage key between state init and named-sub capture A `state` variable declared in a block at top level (no enclosing sub) and referenced by a named sub defined in the same block was reading an uninitialized scalar: the sub saw undef even though the outer block had set the value. Root cause: the persistent-variable key differed between the two sides. - The `state $x = ...` initializer stores the value in global storage using `PersistentVariable.beginVariable(sigilNode.id, name)` where `sigilNode.id` is assigned by `OperatorParser` for `state` decls. - When a named sub captures the same lexical, `SubroutineParser` allocated a *separate* begin id via `RuntimeCode.evalBeginIds.computeIfAbsent(ast, k -> classCounter++)`, producing a different global key. The sub's captured field was thus a fresh empty RuntimeScalar. Fix: in the named-sub capture path, reuse the state variable's existing `ast.id` as the begin id (and register it in `evalBeginIds`). As a secondary safety net, `StateVariable.retrieveStateScalar/Array/ Hash` now fall back to global storage when the per-sub map has no entry, so a sub accessing a state var declared in an outer (non-sub) scope sees the shared storage even if capture didn't wire it up. Before this fix, `jcpan -t QRCode::Encoder` failed in `t/01-hello.t`, `t/02-best.t`, `t/03-version-7.t`: `state $table = [...]` in the block-scoped `QRSpec` helpers was undef inside `qrspec_data_size` and friends, producing "no suitable version", "not enough bits, wrong version?", and a cascade of `Use of uninitialized value` warnings. After the fix, QRCode::Encoder's full test suite passes. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../org/perlonjava/core/Configuration.java | 4 +- .../frontend/parser/SubroutineParser.java | 16 ++++++-- .../runtime/runtimetypes/StateVariable.java | 39 ++++++++++++++----- 3 files changed, 45 insertions(+), 14 deletions(-) diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index 048343092..f2dc2f310 100644 --- a/src/main/java/org/perlonjava/core/Configuration.java +++ b/src/main/java/org/perlonjava/core/Configuration.java @@ -33,7 +33,7 @@ public final class Configuration { * Automatically populated by Gradle/Maven during build. * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String gitCommitId = "2cc770db1"; + public static final String gitCommitId = "6d48b1ad0"; /** * Git commit date of the build (ISO format: YYYY-MM-DD). @@ -48,7 +48,7 @@ public final class Configuration { * Parsed by App::perlbrew and other tools via: perl -V | grep "Compiled at" * DO NOT EDIT MANUALLY - this value is replaced at build time. */ - public static final String buildTimestamp = "Apr 21 2026 19:57:31"; + public static final String buildTimestamp = "Apr 21 2026 21:54:08"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java index c61e4c4ea..a405e1c98 100644 --- a/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java +++ b/src/main/java/org/perlonjava/frontend/parser/SubroutineParser.java @@ -1157,9 +1157,19 @@ public static ListNode handleNamedSubWithFilter(Parser parser, String subName, S entry.perlPackage()); } else { OperatorNode ast = entry.ast(); - int beginId = RuntimeCode.evalBeginIds.computeIfAbsent( - ast, - k -> EmitterMethodCreator.classCounter++); + // For state variables, the persistent-variable id is already + // assigned (see OperatorParser "state" handling). Reuse it so + // that the storage key here matches the key used by the state + // initializer / retrieveStateScalar (which also use ast.id). + int beginId; + if ("state".equals(entry.decl()) && ast != null && ast.id != 0) { + beginId = ast.id; + RuntimeCode.evalBeginIds.putIfAbsent(ast, beginId); + } else { + beginId = RuntimeCode.evalBeginIds.computeIfAbsent( + ast, + k -> EmitterMethodCreator.classCounter++); + } variableName = NameNormalizer.normalizeVariableName( entry.name().substring(1), PersistentVariable.beginPackage(beginId)); diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/StateVariable.java b/src/main/java/org/perlonjava/runtime/runtimetypes/StateVariable.java index ea8652e5b..ec5c8a592 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/StateVariable.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/StateVariable.java @@ -117,10 +117,19 @@ public static RuntimeScalar retrieveStateScalar(RuntimeScalar codeRef, String va // Retrieve variable in the specific code context. RuntimeCode code = (RuntimeCode) codeRef.value; RuntimeScalar variable = code.stateVariable.get(beginVar); - if (variable == null) { - variable = new RuntimeScalar(); - code.stateVariable.put(beginVar, variable); + if (variable != null) { + return variable; } + // Fallback: the state variable may have been declared in an + // enclosing scope outside any sub (then stored globally with the + // same beginVar key). Check the global storage before allocating + // a fresh per-sub entry so that a state var shared between the + // enclosing scope and an inner sub refers to the same storage. + if (GlobalVariable.existsGlobalVariable(beginVar)) { + return GlobalVariable.getGlobalVariable(beginVar); + } + variable = new RuntimeScalar(); + code.stateVariable.put(beginVar, variable); return variable; } } @@ -142,10 +151,16 @@ public static RuntimeArray retrieveStateArray(RuntimeScalar codeRef, String var, // Retrieve array in the specific code context. RuntimeCode code = (RuntimeCode) codeRef.value; RuntimeArray variable = code.stateArray.get(beginVar); - if (variable == null) { - variable = new RuntimeArray(); - code.stateArray.put(beginVar, variable); + if (variable != null) { + return variable; + } + // Fallback: the state variable may have been declared in an + // enclosing scope outside any sub (stored globally). + if (GlobalVariable.existsGlobalArray(beginVar)) { + return GlobalVariable.getGlobalArray(beginVar); } + variable = new RuntimeArray(); + code.stateArray.put(beginVar, variable); return variable; } } @@ -167,10 +182,16 @@ public static RuntimeHash retrieveStateHash(RuntimeScalar codeRef, String var, i // Retrieve hash in the specific code context. RuntimeCode code = (RuntimeCode) codeRef.value; RuntimeHash variable = code.stateHash.get(beginVar); - if (variable == null) { - variable = new RuntimeHash(); - code.stateHash.put(beginVar, variable); + if (variable != null) { + return variable; + } + // Fallback: the state variable may have been declared in an + // enclosing scope outside any sub (stored globally). + if (GlobalVariable.existsGlobalHash(beginVar)) { + return GlobalVariable.getGlobalHash(beginVar); } + variable = new RuntimeHash(); + code.stateHash.put(beginVar, variable); return variable; } }