Skip to content

fix(eval STRING): preserve 'our' decls so caller-side local() is visible#532

Merged
fglock merged 1 commit intomasterfrom
fix/eval-string-local-our-visibility
Apr 21, 2026
Merged

fix(eval STRING): preserve 'our' decls so caller-side local() is visible#532
fglock merged 1 commit intomasterfrom
fix/eval-string-local-our-visibility

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Apr 21, 2026

Summary

Fixes eval STRING so that local applied to outer our-declared
package variables is visible inside the eval body.

Minimal reproducer that broke before this patch:

our $x = 1;
sub test { local $x = 42; eval q{print "eval x=$x\n"}; }
test();   # was printing 1, should (and now does) print 42

The same issue affected local @our_array / local %our_hash seen
from inside eval STRING, including element access like $our_hash{k}.

Root cause

The bytecode-interpreter eval path (default on PerlOnJava) builds an
adjustedRegistry of captured variables in RuntimeCode /
EvalStringHandler and hands 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, so the eval body read the
scalar that was in the global slot at eval-entry time — not the
currently-localized one.

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 deliberately does not propagate
    adjustedDecls: nested evals seed outer my vars as our in a
    synthetic package purely for the parser, and at runtime those must
    stay register-bound. A comment explains this.

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::is_utf8 issue unrelated to eval/local). The original
Date::Parse::str2time failures ("Fri Dec 17 00:00:00 1901 GMT",
"Wed, 16 Jun 94 07:29:35 CST", …) were all caused by this bug:
Date::Parse builds its regex closures via eval "$strptime" inside
local($day_ref,$mon_ref,$suf_ref), and the eval was seeing the
pre-local (undef) scalars instead of the freshly-localized ones.

Test plan

  • make (all unit tests pass, including
    unit/eval_nested_lexicals.t which exercises the
    EvalStringHandler path we intentionally left alone)
  • make test-bundled-modules
  • jcpan -t TimeDate — 1/28 test files fail, 8/1215 subtests
    fail (down from 10/28 and 340/1215)

Generated with Devin

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>
@fglock fglock force-pushed the fix/eval-string-local-our-visibility branch from d8503f9 to cbab500 Compare April 21, 2026 20:36
@fglock fglock merged commit 3f33367 into master Apr 21, 2026
2 checks passed
@fglock fglock deleted the fix/eval-string-local-our-visibility branch April 21, 2026 20:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant