Fix interpreter $_ variable in foreach loops#220
Merged
Conversation
Two bugs fixed in the bytecode interpreter:
Bug 1: local $_ was producing $main::main::_ instead of $main::_
- Replace 6 manual packageName + "::" + name concatenations in the
'local' operator handler with NameNormalizer.normalizeVariableName()
Bug 2: $_ not aliased to loop element in foreach loops
- Add two new superinstructions:
LOCAL_SCALAR_SAVE_LEVEL (302): atomically saves getLocalLevel() before
calling makeLocal(), so POP_LOCAL_LEVEL can correctly restore $_ after
the loop regardless of nesting depth.
FOREACH_GLOBAL_NEXT_OR_EXIT (304): per-iteration superinstruction combining
hasNext + next() + aliasGlobalVariable + conditional exit.
POP_LOCAL_LEVEL (303): restores DynamicVariableManager to saved level.
- visit(BlockNode) detects the BlockNode([local $_, For1Node(needsArrayOfAlias)])
pattern and skips the 'local $_' child directly (safe local variable, not a
mutable field), letting For1Node own the full local/loop/restore sequence.
Verified:
for (1..3) { say $_ } => 1 2 3
$_ = 123; for (1..3) { show() } show() => 1 2 3 123
nested foreach loops with $_ restore correctly
Restructure For1Node bytecode from while-style to do-while style,
saving one GOTO per iteration:
Before:
loopStart:
FOREACH_NEXT_OR_EXIT -> exit (check at top)
body
GOTO loopStart (back-edge every iteration)
exit:
After:
GOTO loopCheck (one-time entry jump)
body:
body
loopCheck:
FOREACH_NEXT_OR_EXIT -> body (jump backward if has next)
exit: (fall through if exhausted)
The superinstruction semantics are inverted: the target is now
bodyStart (jump there if has next) rather than exitTarget (jump
there if exhausted). Applies to both FOREACH_NEXT_OR_EXIT and
FOREACH_GLOBAL_NEXT_OR_EXIT.
Addresses issue #219.
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
Two bugs fixed in the bytecode interpreter for
$_inforeachloops.Bug 1:
local $_produced$main::main::_instead of$main::_The
localoperator handler was manually concatenatingpackageName + "::" + name, but when$_is parsed itsIdentifierNode.nameis alreadymain::_, producing the double-qualified name. Fixed by usingNameNormalizer.normalizeVariableName()consistently (6 sites).Bug 2:
$_not aliased to loop element inforeachloopsThe interpreter's
For1Nodehandler stored the next iterator element into a register but never aliased it to$main::_, so$_was always undefined inside the loop body.Solution: two new superinstructions
LOCAL_SCALAR_SAVE_LEVEL rd levelReg nameIdx(302): atomically savesgetLocalLevel()intolevelRegbefore callingmakeLocal(name). The pre-push level is used byPOP_LOCAL_LEVELafter the loop to correctly restore$_at any nesting depth.FOREACH_GLOBAL_NEXT_OR_EXIT varReg iterReg nameIdx exitTarget(304): per-iteration superinstruction combininghasNext+next()+aliasGlobalVariable(name, element)+ conditional exit.POP_LOCAL_LEVEL rs(303): restoresDynamicVariableManagerto the saved pre-push level after loop exit.Safe coordination
visit(BlockNode)detects theBlockNode([local $_, For1Node(needsArrayOfAlias)])pattern and skips thelocal $_child using a local variableskipFirstChild(not a mutable field), soFor1Nodeowns the full local/loop/restore sequence.Verified