feat: Add comprehensive declared references support to interpreter#210
Merged
Conversation
Implements two critical missing operators that significantly improve test parity:
1. **tr/// (transliteration) operator** - TR_TRANSLITERATE opcode (221)
- Full support for tr/search/replace/modifiers syntax
- Handles all modifiers: /c (complement), /d (delete), /s (squash), /r (return)
- Uses existing RuntimeTransliterate.compile() and .transliterate() methods
- Added handler in SlowOpcodeHandler.executeTransliterate()
2. **Scalar dereference** - $$ref, $${expr} support
- Handles OperatorNode case in compileVariableReference()
- Uses existing DEREF opcode (69)
- Supports nested dereferencing ($$x, $$$x, etc.)
**Test Improvements:**
- op/tr.t: 90/318 (28.3%) → 266/318 (83.6%) ✓ +176 tests passing
- Was incomplete (crashing at test 92), now runs to completion
- uni/variables.t: 66760/66880 → 66761/66880 ✓ +1 test, no longer crashes
- op/bop.t: 480/522 (92.0%) stable ✓
**Documentation:**
- Updated SKILL.md with testing modes explanation:
- JPERL_EVAL_USE_INTERPRETER=1: eval STRING uses interpreter
- --interpreter: forces interpreter everywhere
**Updated LASTOP:**
- LASTOP = 220 → 221 (TR_TRANSLITERATE)
- All generated opcodes (LASTOP+1 through LASTOP+43) auto-adjust
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ndling This commit improves the interpreter's handling of eval STRING and declared references to reduce test gaps when JPERL_EVAL_USE_INTERPRETER=1. Changes: 1. Add JPERL_EVAL_VERBOSE environment variable - When set, eval compilation errors are printed to stderr - Helps debug interpreter issues during testing - Errors still stored in $@ as normal 2. Improve declared references handling in BytecodeCompiler - Skip reference declarations (my \($x), our \($x)) instead of crashing - Prevents "my list declaration requires identifier" errors - Allows tests to continue past reference declarations Test Impact: - op/tr.t: 266/318 passing with interpreter (83.6%, only -11 vs normal) - uni/variables.t: 66761/66880 passing (99.8%, only -119 vs normal) - op/decl-refs.t: Still needs full implementation (144/408, -126) The tr/// operator is fully working in interpreter mode. The main remaining issue is full implementation of declared references feature. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mpiler Add support for declared references (my \$x, my \($x, $y)) in the interpreter path. This allows JPERL_EVAL_USE_INTERPRETER to handle more test cases from op/decl-refs.t. Changes: 1. Single variable declared refs (my \$x) - Check isDeclaredReference annotation on OperatorNode - Emit CREATE_REF opcode after variable declaration - Returns reference to variable instead of variable itself 2. List declared refs (my \($x, $y)) - Check isDeclaredReference on parent node - Collect all declared variables into list - Create references for each using CREATE_REF - Build RuntimeList of references 3. Both work for captured and non-captured variables Direct mode testing: - my \$x returns SCALAR reference ✓ - Ready for further testing in eval mode Known issues: - eval STRING with declared refs needs more testing - Some edge cases in decl-refs.t still need investigation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Handle nested ListNode elements in my() declarations to avoid crashing when encountering constructs like my (($x, $y)). Changes: - Add check for ListNode elements in my() list declarations - Skip nested lists gracefully with continue - Prevents "my list declaration requires identifier: ListNode" error Testing shows declared refs now work in interpreter mode: - my \$x returns SCALAR reference ✓ - my \($a, $b) returns list of 2 elements ✓ Test status: - op/decl-refs.t: Still 144/408 (needs more investigation) - tr.t: 266/318 ✓ - uni/variables.t: 66761/66880 ✓ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implement proper handling of my (\$x) and my (\($x, $y)) constructs in the BytecodeCompiler for interpreter mode. Changes: - Detect backslash operator in list elements - Handle single variable case: my (\$x) - Handle nested list case: my (\($x, $y)) - Extract variables from nested structures and declare them Testing shows improvement: - my (\($d, $e)) now works in eval ✓ - Error changed from "ListNode" to "OperatorNode" (progress) Next: Handle \my (\$x, $y) construct (backslash outside my) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Implemented comprehensive handling for declared references in BytecodeCompiler: 1. Added support for `my \\$x` (double backslash) - reference to declared ref - Recursively compiles inner backslash and creates additional reference 2. Fixed `my (\($d, $e))` (nested list with backslash) - Track `foundBackslashInList` flag to trigger reference creation - Variables are collected and references created at list return 3. Handle backslash outside my: `\my (\$f, $g)` - Modified CREATE_REF opcode to detect multi-element RuntimeList - Call createListReference() for lists, createReference() for scalars - Distributes backslash over list elements as per Perl semantics Test improvements: - op/decl-refs.t: 144/408 (35.3%) → 181/408 (44.4%) - Improvement: +37 tests passing (+9.1%) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…eter Implemented comprehensive declared references support for all declaration types: ## State Variables (state) - Added state to variable declaration handling - State variables always use persistent storage (RETRIEVE_BEGIN_*) - Support for single and list declarations - Handle declared refs: `state \$x`, `state (\$x, $y)`, `state \\$x` ## Our Variables (our) - Enhanced our() to handle declared references - Support for backslash operators in lists - Handle nested constructs: `our (\($x, $y))`, `our (\\$x)` - Double backslash support: `our \\$x` - Create references for declared ref annotations ## Local Variables (local) - Implemented list support for local declarations - Added backslash handling for local with declared refs - Support: `local (\$x)`, `local (\($x, $y))`, `local \\$x` - Proper error handling for lexical variables ## Test Improvements - op/decl-refs.t: 144/408 (35.3%) → 258/408 (63.2%) - Improvement: +114 tests (+27.9%) - Gap from compiler (270/408): only -12 tests ## Technical Implementation 1. Unified backslash operator handling across my/state/our/local 2. foundBackslashInList flag for tracking nested references 3. CREATE_REF opcode handles both single values and multi-element lists 4. Recursive compilation for double backslash constructs 5. Consistent declared reference annotation checking ## Constructs Now Working ✓ my/state/our/local \$x - single declared ref ✓ my/state/our/local \($x, $y) - list declared refs ✓ my/state/our/local (\($d, $e)) - nested lists ✓ my/state/our/local \\$x - double backslash ✓ my/state/our/local (\\$x) - double backslash in list ✓ \my/state/our/local (\$f, $g) - backslash outside ✓ All sigils: $x, @x, %x Remaining issues mostly parser-related (attributes) and edge cases. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed comprehensive support for local with single and double backslash: ## Single Backslash (local \$x) - Properly handle local \$x, local \@x, local \%x - Localize variable, then create single reference - Check isDeclaredReference annotation ## Double Backslash (local \\$x) - Handle local \\$x with proper double reference creation - Check both node and backslash operator annotations - Localize, create ref, create ref to ref ## List Context - Handle local (\$x), local (\@x), local (\%x) - Handle local (\\$x) double backslash in list - Check backslash operator isDeclaredReference annotation - Support nested lists: local (\($x, $y)) - All sigils supported: $, @, % Test improvements: - op/decl-refs.t: 248/408 (60.8%) → 260/408 (63.7%) - Improvement: +12 tests (+3.0%) - Gap from compiler (270/408): -10 tests Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Fixed proper type initialization for declared refs with arrays and hashes: ## The Issue For `my \@x` or `my \%x`, we were creating scalar (undef) refs when we should create array/hash refs. The declared ref means the variable IS a scalar holding a reference, but the reference should point to proper container. ## Changes Made 1. **my/state (\@x, \%x)** - Initialize with NEW_ARRAY/NEW_HASH 2. **our (\@x, \%x)** - Load with LOAD_GLOBAL_ARRAY/LOAD_GLOBAL_HASH 3. **Nested lists** - Respect original sigil when creating containers 4. **Single variables** - Switch on originalSigil to initialize correctly ## Test Results - op/decl-refs.t: 260/408 (63.7%) → **270/408 (66.2%)** - **GOAL ACHIEVED: Matches compiler performance!** - Improvement: +10 tests (+2.5%) Array/hash tests now passing: - my (\(@d, @e)) returns correct refs ✓ - my (\%f, %g) returns correct refs ✓ - state/our/local with @/% sigils ✓ Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Owner
Author
Regression Fixed ✓The 6-test regression in re/reg_mesg.t has been identified and fixed. Root CauseThe regression was introduced in commit 77c6a98 which refactored:
Fix AppliedReverted both changes back to the previous working implementation:
Test Results
The fix has been merged into this feature branch and pushed to master. Ready to merge! ✅ |
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
This PR implements comprehensive declared references (declared_refs) support for the interpreter, achieving 100% parity with the JVM compiler on the test suite.
🎉 Achievement: 270/408 tests passing (66.2%)
Test Results
All Three Target Tests
Features Implemented
1. JPERL_EVAL_VERBOSE Environment Variable
2. My Variables (my $x, my @x, my %x)
my \$x,my \@x,my \%xmy \($x, $y)my (\($d, $e))my \\$x,my (\\$x)\my (\$f, $g)3. State Variables (state $x)
4. Our Variables (our $x)
5. Local Variables (local $x)
6. CREATE_REF Opcode Enhancement
createListReference()\(a, b)creates list of refs7. Array/Hash Type Support
my \@xcreates array ref (not scalar)my \%xcreates hash ref (not scalar)our \@xloads global arrayTechnical Details
Files Changed
src/main/java/org/perlonjava/runtime/RuntimeCode.java- Added JPERL_EVAL_VERBOSEsrc/main/java/org/perlonjava/interpreter/BytecodeCompiler.java- ~500+ lines of declared refs supportsrc/main/java/org/perlonjava/interpreter/BytecodeInterpreter.java- Enhanced CREATE_REF opcodeTest Progression
Known Limitations
Implementation Approach
The current implementation checks specifically for double backslash annotations, which doesn't scale to triple backslash (
\\\$x) or beyond. A more general recursive approach (like the JVM codegen) would be better for handling arbitrary nesting levels. However, this works for all constructs in the test suite.Remaining Test Failures (94 blocked)
(\$h) : attr$::varreferencesCommits
Testing
Test with:
Results: 270/408 tests passing (66.2%)
Performance Impact
Conclusion
This PR achieves the goal of matching compiler performance for declared references support in the interpreter. The 126-test improvement (87.5% increase) demonstrates comprehensive support for all common declared reference patterns across my/state/our/local declarations.
Ready to merge! ✅
Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com