Skip to content

Fix local package variable bug - our variables now see local changes#333

Merged
fglock merged 5 commits into
masterfrom
feature/fix-local-package-variable
Mar 19, 2026
Merged

Fix local package variable bug - our variables now see local changes#333
fglock merged 5 commits into
masterfrom
feature/fix-local-package-variable

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Mar 19, 2026

Summary

This fixes the bug where subroutines don't see localized values of our variables set from outside their package.

Problem

package Foo;
our $X = 0;
sub check { print "X=$X\n"; }  # Uses short name

package main;
local $Foo::X = 1;
Foo::check();  # jperl printed "X=0", Perl prints "X=1"

This affected:

  • $Carp::CarpLevel (error reporting in all modules using Carp)
  • 16+ Log4perl tests
  • Many CPAN modules using local $Module::Variable pattern

Root Cause

our variables were treated as lexical (stored in JVM local variable slots), capturing values at subroutine definition time. When local replaced the scalar in the symbol table, the subroutine's cached register still held the old reference.

The Fix

  1. EmitVariable.java: Distinguish between:

    • BEGIN-captured our variables (in PerlOnJava::_BEGIN_* packages) → remain lexical for compile-to-runtime persistence
    • Regular our variables → look up from GlobalVariable at runtime
  2. EmitSubroutine.java and SubroutineParser.java: Preserve original perlPackage when copying variables to subroutine scopes

  3. ScopedSymbolTable.java: Add 4-argument addVariable() overload for explicit package specification

Test Results

  • All existing tests pass
  • New cross-package our/local tests pass (54 tests in local.t)
  • Test::More/Test2 modules work correctly
  • BEGIN block lexical persistence works correctly

Test plan

  • make passes all unit tests
  • local.t includes new cross-package tests
  • use Test::More works correctly
  • BEGIN block persistence works correctly

Generated with Devin

fglock and others added 5 commits March 19, 2026 10:47
Added notes about an attempted approach (skipping our variables from
closure capture) that partially worked but broke Test::More due to
constructor signature mismatches. Documented that the fix needs to
happen at the variable access level, not closure capture level.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
- Added test cases in local.t (in __END__ section) for cross-package
  our/local scenarios that currently fail
- Updated design doc with detailed findings from implementation attempts:
  - Approach 1 (skip our from closures) broke constructor signatures
  - Approach 2 (non-lexical our) breaks Test::More/Test2 due to
    BEGIN block interactions
- The fix requires further investigation into how BEGIN blocks
  interact with lexical closure capture

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
This fixes the bug where subroutines don't see 'local'ized values
of 'our' variables set from outside their package.

Root cause: 'our' variables were treated as lexical (stored in JVM
local variable slots), capturing values at subroutine definition time.
This meant 'local $Pkg::Var' changes weren't visible inside subroutines.

The fix:
1. EmitVariable.java: Distinguish between BEGIN-captured 'our' variables
   (in PerlOnJava::_BEGIN_* packages, which should remain lexical for
   compile-to-runtime persistence) and regular 'our' variables (which
   should look up from GlobalVariable at runtime).

2. EmitSubroutine.java and SubroutineParser.java: Use 4-argument
   addVariable() to preserve the original perlPackage when copying
   variables to subroutine scopes.

3. ScopedSymbolTable.java: Add 4-argument addVariable() overload to
   allow explicit package specification.

Test results:
- All existing tests pass
- New cross-package our/local tests pass (54 tests in local.t)
- Test::More/Test2 modules work correctly
- BEGIN block lexical persistence works correctly

See dev/design/local-package-variable-fix.md for detailed analysis.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
…pilation error

In Perl, calling exit() inside a BEGIN block terminates the program immediately.
Previously, jperl would catch the PerlExitException and convert it to a
"BEGIN failed--compilation aborted" error, then continue parsing and fail.

This fix re-throws PerlExitException so it propagates to Main.main() which
converts it to System.exit().

Fixes tests that use `plan skip_all` in BEGIN blocks (e.g., Log4perl's
t/056SyncApp2.t and t/066SQLite.t) - they now skip cleanly without errors.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
…updates

- Document PR #331 fix for exit() inside BEGIN blocks
- Update test results (now 700 tests, 26 failures)
- Mark 'local' package variable bug as verified fixed
- Update t/020Easy.t issue category

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <noreply@cognition.ai>
@fglock fglock force-pushed the feature/fix-local-package-variable branch from fcc0d5e to fd6bb95 Compare March 19, 2026 09:48
@fglock fglock merged commit 9d88063 into master Mar 19, 2026
2 checks passed
@fglock fglock deleted the feature/fix-local-package-variable branch March 19, 2026 09:52
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