Skip to content

Commit e40845f

Browse files
authored
Merge pull request #301 from fglock/feature/defer-blocks
Implement Perl defer blocks (use feature 'defer')
2 parents 6a1da0f + 67c5ee0 commit e40845f

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+2135
-405
lines changed

.cognition/skills/debug-perlonjava/SKILL.md

Lines changed: 77 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,32 @@ perl dev/tools/perl_test_runner.pl perl5_t/t/op
6060
perl dev/tools/perl_test_runner.pl --jobs 8 --timeout 60 perl5_t/t
6161
```
6262

63+
### Test runner environment variables
64+
The test runner (`dev/tools/perl_test_runner.pl`) automatically sets environment variables for specific tests:
65+
66+
```perl
67+
# JPERL_UNIMPLEMENTED="warn" for these tests:
68+
re/pat_rt_report.t | re/pat.t | re/regex_sets.t | re/regexp_unicode_prop.t
69+
op/pack.t | op/index.t | op/split.t | re/reg_pmod.t | op/sprintf.t | base/lex.t
70+
71+
# JPERL_OPTS="-Xss256m" for these tests:
72+
re/pat.t | op/repeat.t | op/list.t
73+
74+
# PERL_SKIP_BIG_MEM_TESTS=1 for ALL tests
75+
```
76+
77+
To reproduce what the test runner does for a specific test:
78+
```bash
79+
# For re/pat.t (needs all three):
80+
cd perl5_t/t && JPERL_UNIMPLEMENTED=warn JPERL_OPTS=-Xss256m PERL_SKIP_BIG_MEM_TESTS=1 ../../jperl re/pat.t
81+
82+
# For re/subst.t (only PERL_SKIP_BIG_MEM_TESTS):
83+
cd perl5_t/t && PERL_SKIP_BIG_MEM_TESTS=1 ../../jperl re/subst.t
84+
85+
# For op/bop.t (only PERL_SKIP_BIG_MEM_TESTS):
86+
cd perl5_t/t && PERL_SKIP_BIG_MEM_TESTS=1 ../../jperl op/bop.t
87+
```
88+
6389
### Interpreter mode
6490
```bash
6591
./jperl --interpreter script.pl
@@ -123,39 +149,32 @@ git checkout branch && mvn package -q -DskipTests
123149
./jperl -e 'failing code'
124150
```
125151

126-
### 2. Bisect to find the bad commit
127-
**IMPORTANT**: Always rebuild after switching commits!
128-
```bash
129-
git log master..branch --oneline
130-
git checkout <commit> && mvn package -q -DskipTests && ./jperl -e 'test'
131-
```
132-
133-
### 3. Create minimal reproducer
152+
### 2. Create minimal reproducer
134153
Reduce the failing test to the smallest code that demonstrates the bug:
135154
```bash
136155
./jperl -e 'my $x = 58; eval q{($x) .= "z"}; print "x=$x\n"'
137156
```
138157

139-
### 4. Compare with system Perl
158+
### 3. Compare with system Perl
140159
```bash
141160
perl -e 'same code'
142161
```
143162

144-
### 5. Use --parse to check AST
163+
### 4. Use --parse to check AST
145164
When parsing issues are suspected, compare the parse tree:
146165
```bash
147166
./jperl --parse -e 'code' # Show PerlOnJava AST
148167
perl -MO=Deparse -e 'code' # Compare with Perl's interpretation
149168
```
150169
This helps identify operator precedence issues and incorrect parsing.
151170

152-
### 6. Use disassembly to understand
171+
### 5. Use disassembly to understand
153172
```bash
154173
./jperl --disassemble -e 'minimal code' # JVM bytecode
155174
./jperl --disassemble --interpreter -e 'minimal code' # Interpreter bytecode
156175
```
157176

158-
### 7. Profile with JFR (for performance issues)
177+
### 6. Profile with JFR (for performance issues)
159178
```bash
160179
# Record profile
161180
$JAVA_HOME/bin/java -XX:StartFlightRecording=duration=10s,filename=profile.jfr \
@@ -166,14 +185,14 @@ $JAVA_HOME/bin/jfr print --events jdk.ExecutionSample profile.jfr 2>&1 | \
166185
grep -E "^\s+[a-z].*line:" | sed 's/line:.*//' | sort | uniq -c | sort -rn | head -20
167186
```
168187

169-
### 8. Add debug prints (if needed)
188+
### 7. Add debug prints (if needed)
170189
In Java source, add:
171190
```java
172191
System.err.println("DEBUG: var=" + var);
173192
```
174193
Then rebuild with `mvn package -q -DskipTests`.
175194

176-
### 9. Fix and verify
195+
### 8. Fix and verify
177196
```bash
178197
# After fixing
179198
mvn package -q -DskipTests
@@ -258,6 +277,50 @@ Both backends share the parser (same AST) and runtime (same operators, same Runt
258277

259278
All paths relative to `src/main/java/org/perlonjava/`.
260279

280+
## CRITICAL: Investigate JVM Backend First
281+
282+
**When fixing interpreter bugs, ALWAYS investigate how the JVM backend handles the same operation before implementing a fix.**
283+
284+
The interpreter and JVM backends share the same runtime classes (`RuntimeScalar`, `RuntimeArray`, `RuntimeHash`, `RuntimeList`, `PerlRange`, etc.). The JVM backend is the reference implementation - if the interpreter handles something differently, it's likely wrong.
285+
286+
### How to investigate JVM behavior
287+
288+
1. **Disassemble the JVM bytecode** to see what runtime methods it calls:
289+
```bash
290+
./jperl --disassemble -e 'code that works'
291+
```
292+
293+
2. **Look for the runtime method calls** in the disassembly (INVOKEVIRTUAL, INVOKESTATIC):
294+
```
295+
INVOKEVIRTUAL org/perlonjava/runtime/runtimetypes/RuntimeList.addToArray
296+
INVOKEVIRTUAL org/perlonjava/runtime/runtimetypes/RuntimeBase.setFromList
297+
```
298+
299+
3. **Read those runtime methods** to understand the correct behavior:
300+
- How does `setFromList()` handle different input types?
301+
- What methods does it call internally (`addToArray`, `getList`, etc.)?
302+
303+
4. **Use the same runtime methods in the interpreter** instead of reimplementing the logic with special cases.
304+
305+
### Example: Hash slice assignment with PerlRange
306+
307+
**Wrong approach** (special-casing types in interpreter):
308+
```java
309+
if (valuesBase instanceof RuntimeList) { ... }
310+
else if (valuesBase instanceof RuntimeArray) { ... }
311+
else if (valuesBase instanceof PerlRange) { ... } // BAD: special case
312+
else { ... }
313+
```
314+
315+
**Correct approach** (use same runtime methods as JVM):
316+
```java
317+
// JVM calls addToArray() which handles all types uniformly
318+
RuntimeArray valuesArray = new RuntimeArray();
319+
valuesBase.addToArray(valuesArray); // Works for RuntimeList, RuntimeArray, PerlRange, etc.
320+
```
321+
322+
The JVM's `setFromList()``addToArray()` chain already handles `PerlRange` correctly via `PerlRange.addToArray()``toList().addToArray()`. The interpreter should use the same mechanism.
323+
261324
## Common Bug Patterns
262325

263326
### 1. Context not propagated correctly
@@ -335,10 +398,6 @@ perl -MO=Deparse -e 'code'
335398
# Compare output
336399
diff <(./jperl -e 'code') <(perl -e 'code')
337400

338-
# Bisect
339-
git log master..HEAD --oneline
340-
git checkout <sha> && mvn package -q -DskipTests && ./jperl -e 'test'
341-
342401
# Git workflow (always use branches!)
343402
git checkout -b fix-name
344403
# ... make changes ...

.cognition/skills/interpreter-parity/SKILL.md

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,18 @@ JPERL_INTERPRETER=1 ./jperl script.pl
5959
JPERL_INTERPRETER=1 ./jperl -e 'code'
6060
```
6161

62+
**CRITICAL: eval STRING uses interpreter by default!**
63+
Even when running with JVM backend, `eval STRING` compiles code with the interpreter.
64+
This means interpreter bugs can cause test failures even without `--interpreter`.
65+
66+
To trace eval STRING execution:
67+
```bash
68+
JPERL_EVAL_TRACE=1 ./jperl script.pl 2>&1 | grep -i interpreter
69+
```
70+
71+
Fallback for large subs (`JPERL_SHOW_FALLBACK=1`) does NOT show eval STRING usage.
72+
One-liners won't trigger fallback - test with actual test files!
73+
6274
## Architecture: Two Backends, Shared Everything Else
6375

6476
```
@@ -154,6 +166,53 @@ All paths relative to `src/main/java/org/perlonjava/`.
154166

155167
## Debugging Workflow
156168

169+
### CRITICAL: Save Master Baselines ONCE, Don't Rebuild Repeatedly
170+
171+
**Save master baseline to files FIRST** (do this once per debugging session):
172+
```bash
173+
# Switch to master and build
174+
git stash && git checkout master
175+
mvn package -q -DskipTests
176+
177+
# Save master test output for JVM backend
178+
cd perl5_t/t && ../../jperl re/subst.t 2>&1 > /tmp/master_subst.log
179+
grep "^not ok" /tmp/master_subst.log > /tmp/master_subst_fails.txt
180+
181+
# ALSO save interpreter baseline!
182+
cd perl5_t/t && ../../jperl --interpreter re/subst.t 2>&1 > /tmp/master_subst_interp.log
183+
184+
# Switch back to feature branch
185+
git checkout feature-branch && git stash pop
186+
```
187+
188+
**After making changes**, compare against saved baselines:
189+
```bash
190+
mvn package -q -DskipTests
191+
192+
# Test JVM backend
193+
cd perl5_t/t && ../../jperl re/subst.t 2>&1 > /tmp/feature_subst.log
194+
diff /tmp/master_subst_fails.txt <(grep "^not ok" /tmp/feature_subst.log)
195+
196+
# MUST ALSO test with interpreter!
197+
cd perl5_t/t && ../../jperl --interpreter re/subst.t 2>&1 > /tmp/feature_subst_interp.log
198+
```
199+
200+
### CRITICAL: Always Test with BOTH Backends
201+
202+
A fix that works for JVM backend may break interpreter, or vice versa.
203+
204+
**For quick tests (one-liners):**
205+
```bash
206+
./jperl -e 'test code' # JVM backend
207+
./jperl --interpreter -e 'test code' # Interpreter backend
208+
```
209+
210+
**For test files (use env var so require/do/eval also use interpreter):**
211+
```bash
212+
./jperl test.t # JVM backend
213+
JPERL_INTERPRETER=1 ./jperl test.t # Interpreter backend (full)
214+
```
215+
157216
### 1. Reproduce with minimal code
158217
```bash
159218
# Find the failing construct
@@ -162,6 +221,18 @@ JPERL_INTERPRETER=1 ./jperl -e 'failing code'
162221
./jperl -e 'failing code'
163222
```
164223

224+
**CRITICAL: Save baselines to files!** When comparing test suites across branches:
225+
```bash
226+
# On master - save results so you don't have to rebuild later
227+
git checkout master && mvn package -q -DskipTests
228+
cd perl5_t/t && JPERL_INTERPRETER=1 ../../jperl test.t 2>&1 | tee /tmp/test_master.log
229+
JPERL_INTERPRETER=1 ../../jperl test.t 2>&1 | grep "^ok\|^not ok" > /tmp/test_master_results.txt
230+
grep "^ok" /tmp/test_master_results.txt | wc -l # Save this number!
231+
232+
# Return to feature branch - now you can compare without rebuilding master
233+
git checkout feature-branch && mvn package -q -DskipTests
234+
```
235+
165236
### 2. Use --disassemble to see interpreter bytecode
166237
```bash
167238
JPERL_INTERPRETER=1 ./jperl --disassemble -e 'code' 2>&1

AGENTS.md

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,3 +80,53 @@ See `.cognition/skills/` for specialized debugging and development skills:
8080
- `interpreter-parity` - JVM vs interpreter parity issues
8181
- `debug-exiftool` - ExifTool test debugging
8282
- `profile-perlonjava` - Performance profiling
83+
84+
## Regression Tracking (feature/defer-blocks branch)
85+
86+
### Summary
87+
88+
All reported regressions have been investigated. The issues fall into two categories:
89+
1. **Fixed in this branch**: goto-related issues
90+
2. **Pre-existing on master**: MethodHandle conversion errors, regex octal escape parsing
91+
92+
### Regression Status
93+
94+
| Test | Status | Details |
95+
|------|--------|---------|
96+
| op/die_goto.t | **FIXED** (5/5) | `goto &sub` in `$SIG{__DIE__}` handlers now works |
97+
| uni/goto.t | **FIXED** (2/4) | Tests 1-2 pass (goto &{expr}). Tests 3-4 fail due to pre-existing regex octal escape bug |
98+
| op/bop.t | Pre-existing | MethodHandle conversion error - fails on master too |
99+
| op/warn.t | Pre-existing | MethodHandle conversion error - fails on master too |
100+
| re/subst.t | Pre-existing | MethodHandle conversion error - fails on master too |
101+
| re/pat_rt_report.t | Pre-existing | MethodHandle conversion error - fails on master too |
102+
| lib/croak.t | Pre-existing | `class` feature incomplete |
103+
104+
### Fixes Applied in This Branch
105+
106+
1. **EmitControlFlow.java**: Added `handleGotoSubroutineBlock()` for `goto &{expr}` tail call support
107+
2. **CompileOperator.java**: Added `goto &{expr}` support to bytecode interpreter
108+
3. **RuntimeControlFlowList.java**: Added validation for undefined subroutines in tail calls
109+
4. **RuntimeCode.java**: Added `gotoErrorPrefix()` for "Goto undefined subroutine" error messages
110+
5. **CompileAssignment.java**: Added `vec` lvalue support for interpreter
111+
6. **OpcodeHandlerExtended.java**: Fixed `|=` and `^=` to use polymorphic `bitwiseOr`/`bitwiseXor`
112+
7. **WarnDie.java**: Added TAILCALL trampoline for `goto &sub` in `$SIG{__DIE__}` handlers
113+
114+
### Pre-existing Issues (on master too)
115+
116+
- **MethodHandle conversion errors**: Affects op/warn.t, re/subst.t, re/pat_rt_report.t, op/bop.t
117+
- **Regex octal escapes**: `\345` in patterns is parsed as backreference `\3` + `45`
118+
- **op/bop.t**: `new version ~$_` crashes in Version.java
119+
- **String bitwise ops**: Interpreter uses numeric ops instead of string ops
120+
121+
### How to Check Regressions
122+
123+
```bash
124+
# Run specific test
125+
cd perl5_t/t && ../../jperl <test>.t
126+
127+
# Count passing tests
128+
../../jperl <test>.t 2>&1 | grep "^ok" | wc -l
129+
130+
# Check for interpreter fallback
131+
JPERL_SHOW_FALLBACK=1 ../../jperl <test>.t 2>&1 | grep -i fallback
132+
```

0 commit comments

Comments
 (0)