Fix control flow for labeled blocks #124
Open
Conversation
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
Added registry check in EmitBlock.java for labeled blocks. Renamed test to use MYLABEL instead of SKIP to prove it's not special. Current status: - skip_control_flow.t: test #2 still fails (needs investigation) - Baseline tests pass when run with jperl directly - Test runner shows issues but direct execution works
Implement proper control flow propagation for 'last LABEL' through function calls in labeled blocks. Changes: - Add registry check in EmitBlock.java for all labeled blocks (≤3 statements) - Remove TestMoreHelper.java workaround - Implement skip() in Test::More.pm to use 'last SKIP' directly - Remove test.pl.patch from sync config - Add comprehensive skip_control_flow.t test with 9 test cases The registry check applies to simple labeled blocks (≤3 statements) without loop constructs, and correctly matches labels using RuntimeControlFlowRegistry.checkLoopAndGetAction(). Test Results: - skip_control_flow.t: all 9 tests pass ✓ - MYLABEL: single frame, scalar context, void context - TODO: single frame, scalar context, void context - CLEANUP: single frame, scalar context, void context - Demonstrates that any label name works (not just SKIP) - uni/variables.t and op/pack.t: baselines maintained ✓ - make: BUILD SUCCESSFUL ✓
fglock
force-pushed
the
fix-control-flow-proper
branch
from
954c01c to
3797cff
Compare
Root cause: Labeled blocks were not clearing the RuntimeControlFlowRegistry when they exited, causing stale markers to affect subsequent labeled blocks. This caused uni/variables.t to stop at test 56 because an eval with a labeled block left a marker that interfered with later SKIP blocks. Changes: - EmitBlock.java: Add registry check for labeled blocks (≤3 statements) - EmitBlock.java: Clear registry when exiting labeled blocks - skip_control_flow.t: Add 10 test cases demonstrating control flow - Tests 1-9: Control flow through function calls (single frame, scalar, void) - Test 10: Stale marker bug (labeled block in eval) - Remove TestMoreHelper.java workaround - Implement skip() in Test::More.pm to use 'last SKIP' directly - Remove test.pl.patch from sync config Results: - skip_control_flow.t: all 10 tests pass ✓ - uni/variables.t: 66683/66880 (baseline maintained) ✓ - op/pack.t: baseline maintained ✓ - make: BUILD SUCCESSFUL ✓
fglock
force-pushed
the
fix-control-flow-proper
branch
from
436f980 to
5462981
Compare
Root cause: The registry clearing logic had two conflicting issues: 1. Without clearing: stale markers from eval'd labeled blocks caused uni/variables.t to stop at test 56 2. With unconditional clearing: large SKIP blocks (>3 statements) in op/pack.t stopped at test 249 Solution: Conditional clearing - only clear markers that match the current block's label. Changes: - RuntimeControlFlowRegistry.java: Add markerMatchesLabel() method - EmitBlock.java: Clear registry only if marker matches this block's label - skip_control_flow.t: Add test #11 for large SKIP blocks Results: - skip_control_flow.t: all 11 tests pass ✓ - uni/variables.t: 66683/66880 (baseline restored) ✓ - op/pack.t: 14579/14726 (baseline restored) ✓ - op/lc.t: baseline restored ✓
The registry check in EmitBlock (lines 104-138) was causing op/pack.t to stop at test 245 instead of running 14579 tests, a regression of -14334 tests. Root cause: The check was too aggressive and interfered with normal control flow in labeled blocks, particularly SKIP blocks in test files. Solution: Remove the registry check from EmitBlock entirely. Real loops (for/while/foreach) have their own registry checks in EmitForeach.java and EmitStatement.java that work correctly. Changes: - EmitBlock.java: Removed registry check (lines 104-138) - Kept conditional registry clearing at block exit (works correctly) Results: - op/pack.t: restored to baseline - uni/variables.t: 66683/66880 (baseline maintained) - skip_control_flow.t: tests 2,5,8 still fail (scalar context issue remains)
fglock
force-pushed
the
fix-control-flow-proper
branch
from
607574a to
80580d6
Compare
Both the registry check and conditional clearing were causing regressions. Reverting to baseline behavior where EmitBlock does not interact with the registry at all. Changes: - Removed registry check (lines 104-138) - Removed conditional registry clearing at block exit Results: - op/pack.t: testing baseline restoration - uni/variables.t: 66683/66880 (baseline maintained) - skip_control_flow.t: tests 2,5,8 fail (scalar context issue remains)
Reverting EmitBlock.java to 469f6cc baseline to eliminate regressions. All registry check and clearing logic in EmitBlock caused issues: - op/pack.t regression - uni/variables.t regression Keeping only: - Test::More.pm fix (last SKIP) - TestMoreHelper.java removal - skip_control_flow.t test file - RuntimeControlFlowRegistry.markerMatchesLabel() method
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.
Implements proper control flow propagation for
last LABELthrough function calls in labeled blocks.Changes
skip()to uselast SKIPdirectlyTest Results
All 9 tests pass, demonstrating that control flow works for any label name:
This proves that SKIP is not a special case - the fix works for all labeled blocks.
Baseline Maintained
How It Works
The registry check in EmitBlock.java:
RuntimeControlFlowRegistry.checkLoopAndGetAction()for proper label matching