From cbab5005390a58fbe8146efa0b9d0fdcdee1dc2b Mon Sep 17 00:00:00 2001 From: "Flavio S. Glock" Date: Tue, 21 Apr 2026 22:33:08 +0200 Subject: [PATCH] fix(eval STRING): preserve 'our' decls so caller-side local() is visible Inside an eval STRING, variables declared with our in an outer scope were being treated as plain lexicals bound to the scalar captured at eval-entry time. That made local $OurVar / local @OurArr / local %OurHash in the caller invisible to the eval body: it kept reading the pre-local scalar (or the empty array/hash whose slot had been swapped out by makeLocal). The root cause was in the bytecode-interpreter eval path: RuntimeCode and EvalStringHandler build an adjustedRegistry of captured variables and hand it to BytecodeCompiler, which seeded every captured variable in its symbol table with decl="my". compileVariableReference then took the register path for 'our' vars instead of emitting LOAD_GLOBAL_SCALAR. Changes: - RuntimeCode.evalStringWithInterpreter now builds a parallel adjustedDecls map from the captured symbol table and passes it to BytecodeCompiler so 'our' stays 'our'. - BytecodeCompiler gets a new constructor overload that accepts parentDecls and preserves the outer decl when seeding its symbol table. - handleArrayElementAccess and handleHashElementAccess now check isOurVariable and route 'our' aggregates through LOAD_GLOBAL_ARRAY / LOAD_GLOBAL_HASH so element reads see the current (possibly localized) slot. - EvalStringHandler keeps the old behaviour: for nested evals the symbol table intentionally seeds captured outer `my` vars as `our` in a synthetic package so the parser accepts them, but at runtime those must stay register-bound. A comment explains why we do not propagate adjustedDecls here. Observable impact: jcpan -t TimeDate goes from 340/1215 failing subtests across 10 files to 8/1215 in 1 file (t/lang-encoding.t, a pre-existing utf8-flag issue unrelated to eval/local). make and make test-bundled-modules both pass. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../backend/bytecode/BytecodeCompiler.java | 37 +++++++++++++++++-- .../backend/bytecode/EvalStringHandler.java | 5 +++ .../org/perlonjava/core/Configuration.java | 4 +- .../runtime/runtimetypes/RuntimeCode.java | 20 +++++++++- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/src/main/java/org/perlonjava/backend/bytecode/BytecodeCompiler.java b/src/main/java/org/perlonjava/backend/bytecode/BytecodeCompiler.java index 05135dfd7..15d6bed34 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/BytecodeCompiler.java +++ b/src/main/java/org/perlonjava/backend/bytecode/BytecodeCompiler.java @@ -141,6 +141,26 @@ public BytecodeCompiler(String sourceName, int sourceLine) { */ public BytecodeCompiler(String sourceName, int sourceLine, ErrorMessageUtil errorUtil, Map parentRegistry) { + this(sourceName, sourceLine, errorUtil, parentRegistry, null); + } + + /** + * Constructor for eval STRING with parent scope variable registry and declaration kinds. + * Initializes symbolTable with variables from parent scope, preserving 'our' vs 'my' decls + * so that captured 'our' (package) variables are still resolved via GlobalVariable at + * runtime instead of being bound to the scalar captured at eval-entry time (which would + * make `local $OurVar` in the caller invisible to the eval body). + * + * @param sourceName Source name for error messages + * @param sourceLine Source line for error messages + * @param errorUtil Error message utility + * @param parentRegistry Variable registry from parent scope (for eval STRING) + * @param parentDecls Optional map of varName -> decl kind ("my"/"our"/"state"). Any name + * not present defaults to "my". + */ + public BytecodeCompiler(String sourceName, int sourceLine, ErrorMessageUtil errorUtil, + Map parentRegistry, + Map parentDecls) { this.sourceName = sourceName; this.sourceLine = sourceLine; this.errorUtil = errorUtil; @@ -153,12 +173,17 @@ public BytecodeCompiler(String sourceName, int sourceLine, ErrorMessageUtil erro this.isEvalString = true; if (parentRegistry != null) { - // Add parent scope variables to symbolTable (for eval STRING variable capture) + // Add parent scope variables to symbolTable (for eval STRING variable capture). + // Preserve the outer declaration kind: 'our' stays 'our' so the variable is + // resolved through GlobalVariable (visible to caller-side local), while 'my'/'state' + // use the captured register slot. for (Map.Entry entry : parentRegistry.entrySet()) { String varName = entry.getKey(); int regIndex = entry.getValue(); if (regIndex >= 3) { - symbolTable.addVariableWithIndex(varName, regIndex, "my"); + String decl = parentDecls != null ? parentDecls.get(varName) : null; + if (decl == null || decl.isEmpty()) decl = "my"; + symbolTable.addVariableWithIndex(varName, regIndex, decl); } } @@ -1385,9 +1410,11 @@ void handleArrayElementAccess(BinaryOperatorNode node, OperatorNode leftOp) { emitReg(arrayReg); emit(nameIdx); emit(currentSubroutineBeginId); - } else if (hasVariable(arrayVarName)) { + } else if (hasVariable(arrayVarName) && !isOurVariable(arrayVarName)) { arrayReg = getVariableRegister(arrayVarName); } else { + // 'our' arrays (or undeclared globals) must be fetched from the global + // registry on every access so callers' `local @OurArr` is visible. arrayReg = allocateRegister(); String globalArrayName = NameNormalizer.normalizeVariableName( varName, @@ -1615,9 +1642,11 @@ void handleHashElementAccess(BinaryOperatorNode node, OperatorNode leftOp) { emitReg(hashReg); emit(nameIdx); emit(currentSubroutineBeginId); - } else if (hasVariable(hashVarName)) { + } else if (hasVariable(hashVarName) && !isOurVariable(hashVarName)) { hashReg = getVariableRegister(hashVarName); } else { + // 'our' hashes (or undeclared globals) must be fetched from the global + // registry on every access so callers' `local %OurHash` is visible. hashReg = allocateRegister(); String globalHashName = NameNormalizer.normalizeVariableName( varName, diff --git a/src/main/java/org/perlonjava/backend/bytecode/EvalStringHandler.java b/src/main/java/org/perlonjava/backend/bytecode/EvalStringHandler.java index f519a29aa..39051db59 100644 --- a/src/main/java/org/perlonjava/backend/bytecode/EvalStringHandler.java +++ b/src/main/java/org/perlonjava/backend/bytecode/EvalStringHandler.java @@ -283,6 +283,11 @@ public static RuntimeList evalStringList(String perlCode, // Step 4: Compile AST to interpreter bytecode with adjusted variable registry. // The compile-time package is already propagated via ctx.symbolTable. + // NOTE: We do NOT propagate 'our' decls from the seeded symbol table here, + // because nested evals (this code path) inject captured outer `my` variables + // as `our` in a synthetic seed package purely for parser purposes. Those + // captured-lexical variables must still live in registers at runtime, so + // we keep the default "my" decl in the BytecodeCompiler. BytecodeCompiler compiler = new BytecodeCompiler( evalFileName, sourceLine, diff --git a/src/main/java/org/perlonjava/core/Configuration.java b/src/main/java/org/perlonjava/core/Configuration.java index f2dc2f310..477147cf5 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 = "6d48b1ad0"; + public static final String gitCommitId = "c1e3e0acb"; /** * 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 21:54:08"; + public static final String buildTimestamp = "Apr 21 2026 22:36:17"; // Prevent instantiation private Configuration() { diff --git a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java index ecfc5c114..7732586d9 100644 --- a/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java +++ b/src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java @@ -1366,6 +1366,23 @@ public static RuntimeList evalStringWithInterpreter( } } + // Build a parallel map of declaration kinds so the BytecodeCompiler can distinguish + // 'our' (package) variables from true lexicals. 'our' variables must resolve via + // GlobalVariable.getGlobalVariable() on each access so that `local $OurVar` in the + // caller is visible inside the eval. Without this hint, captured 'our' vars are + // treated as lexical 'my' vars bound to the scalar captured at eval-entry time. + Map adjustedDecls = new HashMap<>(); + if (ctx.capturedEnv != null) { + for (int i = 3; i < ctx.capturedEnv.length; i++) { + String varName = ctx.capturedEnv[i]; + if (varName == null) continue; + SymbolTable.SymbolEntry entry = capturedSymbolTable.getSymbolEntry(varName); + if (entry != null && !entry.decl().isEmpty()) { + adjustedDecls.put(varName, entry.decl()); + } + } + } + // Compile to InterpretedCode with variable registry. // // setCompilePackage() is safe here (unlike EvalStringHandler) because: @@ -1381,7 +1398,8 @@ public static RuntimeList evalStringWithInterpreter( evalCompilerOptions.fileName, 1, evalCtx.errorUtil, - adjustedRegistry); + adjustedRegistry, + adjustedDecls); compiler.setCompilePackage(capturedSymbolTable.getCurrentPackage()); interpretedCode = compiler.compile(ast, evalCtx); evalTrace("evalStringWithInterpreter compiled tag=" + evalTag +