Fix if/unless scope warning + analyze local package variable bug#330
Merged
Conversation
…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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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::Xfrom outside package Foo doesn't affect$Xaccessed inside Foo subroutines:Root Cause:
ourvariables cache a reference to the scalar object in a register at declaration time. Whenlocalreplaces the global with a new object, the subroutine's cached register still points to the old object.Impact: Breaks
$Carp::CarpLeveland affects 16+ Log4perl tests.Design Doc
Added
dev/design/local-package-variable-fix.mdwith detailed fix plan:ourvariables from global symbol table on each accessChanges
src/main/java/org/perlonjava/frontend/parser/StatementParser.java- Add scope for if/unless conditionsdev/design/log4perl-compatibility.md- Document root cause analysisdev/design/local-package-variable-fix.md- New fix plan documentTest Plan
my $x = 1; if (my $x = 2) {...}no longer warnsif (my $x = 1) {...} elsif (my $x = 2) {...}still warns correctlymake)Generated with Devin