Refactor bytecode compiler to use compileNode() helper#277
Merged
Conversation
…ve/restore Add compileNode(Node, int targetReg, int callContext) helper method that encapsulates the save/restore pattern for targetOutputReg and currentCallContext. Replace verbose manual save/restore boilerplate across BytecodeCompiler, CompileOperator, and CompileBinaryOperator. Also add compileScalarOperand() helper in CompileOperator for the common scalar-context compilation pattern. This eliminates the targetOutputRegStack (ArrayDeque-based) mechanism and reduces net code by ~280 lines while preserving identical behavior. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
Replace all 51 .accept() calls with compileNode() using explicit context, remove try/finally wrapper, eliminate 9 redundant intermediate context restores, and rename savedContext to outerContext for the 4 read-only comparison sites. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
Set targetOutputReg before compiling subroutine/block bodies in compile() so that allocateOutputRegister() can place results directly in the return register. Convert remaining raw .accept() calls in CompileBinaryOperator to compileNode() to properly scope targetOutputReg for intermediate operands. Reduces ALIAS opcodes from 31 to 4 in benchmark_lexical.pl disassembly. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
LOAD_GLOBAL_SCALAR for special variables like $1 returns a proxy object that reads from RuntimeRegex.lastCaptureGroups. When RESTORE_REGEX_STATE runs at the end of a block, it restores lastCaptureGroups to its previous value, causing the proxy to return undef. The fix is to use allocateRegister() instead of allocateOutputRegister() for LOAD_GLOBAL_SCALAR, so that the ALIAS operation copies the value before RESTORE_REGEX_STATE runs. Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
- Add local $hash{key} and local $array[index] support in interpreter
- Add __SUB__ field to RuntimeCode for self-reference in InterpretedCode
- Set __SUB__ when InterpretedCode subroutines are created
- Add __SUB__ operator handling in CompileOperator
- Fix local $Foo::x = value to use NameNormalizer for qualified names
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <noreply@cognition.ai>
- Fix hash/array element assignment compiling RHS twice, causing
side effects like shift() to execute twice
- Add support for exists $hashref->{key} and exists $arrayref->[idx]
using fast HASH_EXISTS/ARRAY_EXISTS opcodes instead of slow path
Generated with [Devin](https://cli.devin.ai/docs)
Co-Authored-By: Devin <noreply@cognition.ai>
- Add bounds checking for stringPool access in RETRIEVE_BEGIN_HASH - Add try-catch wrapper around disassembly loop to gracefully handle malformed bytecode instead of crashing - Add SLOW_OP handler (deprecated but needed for completeness) Generated with [Devin](https://cli.devin.ai/docs) Co-Authored-By: Devin <noreply@cognition.ai>
…ntext The handleCompoundAssignment() method was using node.left.accept(this) without setting SCALAR context, causing incorrect behavior when the outer context was different (e.g., in eval blocks). This caused expressions like `($x &= $y) .= "x"` to fail inside eval. Also removes the incorrect LIST_TO_COUNT conversion which was destroying lvalue references for non-simple left operands. 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
1 participant
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
compileNode(Node, int targetReg, int callContext)helper method toBytecodeCompilerthat encapsulates the save/restore pattern for bothtargetOutputRegandcurrentCallContextcompileScalarOperand()helper inCompileOperatorfor the common scalar-context compilation patternBytecodeCompiler.java,CompileOperator.java,CompileBinaryOperator.java, andCompileAssignment.javato use the new helperstargetOutputRegStack(ArrayDeque-based) mechanism entirelyexists $hashref->{key}, double RHS compilation in hash/array assignments,local,__SUB__, andSUPER::support$1capture variable regression in eval blocks--disassemble --interpreter($x &= $y) .= 'x') by usingcompileNode()with explicit SCALAR contextTest plan
mvn compilepassesmvn testpasses (all 156 tests)op/bop.tmaintains 490/522 (no regression from master)re/pat.tmaintains 1053/1296 (no regression from master)Generated with Devin