Skip to content

Fix if/unless scope warning + analyze local package variable bug#330

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

Fix if/unless scope warning + analyze local package variable bug#330
fglock merged 4 commits into
masterfrom
fix/local-package-variable-analysis

Conversation

@fglock
Copy link
Copy Markdown
Owner

@fglock fglock commented Mar 18, 2026

Summary

Fix: Spurious 'my variable masks earlier declaration' warning

Variables declared in if (my $x = ...) conditions now correctly create a new scope, matching Perl's behavior:

  • my $x = 1; if (my $x = 2) {...} - no warning (different scopes) ✓
  • if (my $x = 1) {...} elsif (my $x = 2) {...} - warns correctly (same if-chain scope) ✓

Before: TAP::Formatter::Session showed spurious warnings during jcpan test runs.
After: No spurious warnings.

Analysis: Root cause of Log4perl/Carp test failures

Discovered that local $Foo::X from outside package Foo doesn't affect $X accessed inside Foo subroutines:

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

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

Root Cause: our variables cache a reference to the scalar object in a register at declaration time. When local replaces the global with a new object, the subroutine's cached register still points to the old object.

Impact: Breaks $Carp::CarpLevel and affects 16+ Log4perl tests.

Design Doc

Added dev/design/local-package-variable-fix.md with detailed fix plan:

  • Recommends re-fetching our variables from global symbol table on each access
  • 5-phase implementation plan
  • Risk assessment and timeline (1-2 days)

Changes

  • src/main/java/org/perlonjava/frontend/parser/StatementParser.java - Add scope for if/unless conditions
  • dev/design/log4perl-compatibility.md - Document root cause analysis
  • dev/design/local-package-variable-fix.md - New fix plan document

Test Plan

  • my $x = 1; if (my $x = 2) {...} no longer warns
  • if (my $x = 1) {...} elsif (my $x = 2) {...} still warns correctly
  • Full test suite passes (make)
  • No spurious warnings from TAP::Formatter::Session

Generated with Devin

fglock and others added 4 commits March 18, 2026 21:42
…nless conditions

Variables declared in if (my $x = ...) conditions should be in a new scope,
not the enclosing scope. This matches Perl behavior where:
- my $x = 1; if (my $x = 2) {...} - no warning (different scopes)
- if (my $x = 1) {...} elsif (my $x = 2) {...} - warns (same if-chain scope)

The fix adds enterScope/exitScope around if/unless statements during parsing,
similar to how while/for statements already handle this.

Fixes TAP::Formatter::Session warnings during jcpan test runs.

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

Co-Authored-By: Devin <noreply@cognition.ai>
Investigation revealed that 'local $Foo::X' from outside package Foo
doesn't affect '$X' accessed inside Foo subroutines. Root cause:

- 'our' variables are loaded once at declaration time and cached in registers
- When 'local' replaces the global with a new object, cached registers
  still point to the old object
- Proper fix requires 'our' variables to re-fetch from symbol table on each access

This affects $Carp::CarpLevel and likely many other modules that use
'local $Module::Variable' pattern.

Added detailed reproduction case and updated priority order in design doc.

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

Co-Authored-By: Devin <noreply@cognition.ai>
Detailed plan to fix the issue where 'our' variables in subroutines
don't see 'local'ized values set from outside the package.

Recommends Option A: re-fetch 'our' variables from global symbol table
on each access instead of using cached register values.

Includes:
- Root cause analysis
- Three solution options with pros/cons
- Detailed 5-phase implementation plan
- Risk assessment and timeline estimate

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

Co-Authored-By: Devin <noreply@cognition.ai>
The global name for an 'our' variable is captured at declaration time,
not access time. This ensures 'our $x' in main still refers to $main::x
even after 'package ZZZ;'.

Added test case and key insight to Phase 1 of implementation plan.

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

Co-Authored-By: Devin <noreply@cognition.ai>
@fglock fglock merged commit bb334e1 into master Mar 19, 2026
2 checks passed
@fglock fglock deleted the fix/local-package-variable-analysis branch March 19, 2026 05:57
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