Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions .agents/prompts/bundle-sub-quote.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
# Task: Bundle Modified Sub::Defer/Sub::Quote for PerlOnJava

## Objective
Create bundled modified versions of Sub::Defer and Sub::Quote that pass as many tests as possible by modifying the modules and their tests to skip OOM-triggering cleanup. The goal is to make these modules work correctly on PerlOnJava without requiring a major system redesign of the selective refcounting system.

## Context
Sub::Defer and Sub::Quote are CPAN modules that use weak references extensively. On PerlOnJava, the reachability walker's GC forcing mechanism causes OutOfMemoryError during cleanup when running sub-defer.t and sub-quote.t tests. The tests pass all subtests but timeout during program exit/cleanup.

## Current Test Status
- leaks.t: 9 tests - currently failing due to weak refs to CODE objects not being cleared
- quotify.t: 2595 tests - should pass
- hints.t: 9 tests - 2 failing due to known limitation with caller(0) returning empty warning bits in main script context
- sub-defer.t: 33 tests - timeout/OOM during cleanup
- sub-quote.t: 51 tests - timeout/OOM during cleanup

## Approach
1. Fork Sub::Defer and Sub::Quote into `src/main/perl/lib/Sub/Defer.pm` and `src/main/perl/lib/Sub/Quote.pm`
2. Modify the modules to avoid OOM-triggering cleanup while maintaining compatibility
3. Copy and modify test files to `src/test/resources/modules/Sub-Quote/t/` to skip problematic cleanup
4. Add @INC manipulation to prefer bundled versions over CPAN
5. Test the bundled versions to maximize test pass rate

## Specific Instructions

### 1. Create Bundled Module Files
- Create `src/main/perl/lib/Sub/Defer.pm` based on the CPAN version
- Create `src/main/perl/lib/Sub/Quote.pm` based on the CPAN version
- Add a header comment indicating this is a PerlOnJava-tuned version
- Document the differences from upstream CPAN versions

### 2. Modify Modules to Avoid OOM
Focus on the CLONE method and weak reference usage:

**For Sub::Quote:**
- Modify the CLONE method to use explicit cleanup instead of relying on weak refs being cleared automatically
- Consider using a different data structure for %QUOTED (e.g., refaddr-based keys without weak refs)
- Add a mechanism to skip CLONE cleanup when running tests (e.g., check for environment variable)
- Ensure the CLONE method still works correctly for normal usage (non-test scenarios)

**For Sub::Defer:**
- Apply similar modifications to CLONE and %deferred_info
- Ensure compatibility with Sub::Quote since Sub::Defer depends on it

### 3. Modify Test Files
- Copy test files from the CPAN build directory to `src/test/resources/modules/Sub-Quote/t/`
- Modify sub-defer.t and sub-quote.t to skip or modify tests that trigger OOM
- Add a BEGIN block to set an environment variable (e.g., `$ENV{PERLONJAVA_SKIP_CLONE_CLEANUP} = 1`) to trigger the test-mode behavior
- Keep as many tests as possible - only modify the specific tests that cause OOM
- Document which tests were modified and why

### 4. Add @INC Manipulation
- Modify the module files to add the bundled lib directory to @INC at load time
- Ensure bundled versions take precedence over CPAN versions
- This can be done with:
```perl
use lib '/path/to/PerlOnJava/src/main/perl/lib';
```

### 5. Test and Iterate
- Run the bundled module tests to verify they pass
- Run the original CPAN tests to compare behavior
- Iterate on modifications to maximize test pass rate
- Ensure the bundled versions work correctly for normal usage (not just tests)

## Success Criteria
- leaks.t: All 9 tests pass (or as many as possible)
- quotify.t: All 2595 tests pass
- hints.t: As many tests as possible (may still have 2 failing due to caller(0) limitation)
- sub-defer.t: As many tests as possible without OOM/timeout
- sub-quote.t: As many tests as possible without OOM/timeout
- Bundled versions maintain compatibility with upstream CPAN versions for normal usage
- Changes are well-documented in code comments

## Files to Create/Modify
- `src/main/perl/lib/Sub/Defer.pm` (new)
- `src/main/perl/lib/Sub/Quote.pm` (new)
- `src/test/resources/modules/Sub-Quote/t/leaks.t` (new, modified)
- `src/test/resources/modules/Sub-Quote/t/quotify.t` (new)
- `src/test/resources/modules/Sub-Quote/t/hints.t` (new, modified if needed)
- `src/test/resources/modules/Sub-Quote/t/sub-defer.t` (new, modified)
- `src/test/resources/modules/Sub-Quote/t/sub-quote.t` (new, modified)

## Notes
- The CPAN version is currently at `/Users/fglock/.perlonjava/cpan/build/Sub-Quote-2.006009-3/`
- Sub::Defer is a dependency of Sub::Quote and is in the same directory
- Focus on practical solutions that work rather than perfect compatibility
- The goal is to maximize test pass rate, not necessarily pass 100% of tests
- Document any behavioral differences from upstream versions
82 changes: 82 additions & 0 deletions dev/design/caller_line_number_fix.md
Original file line number Diff line number Diff line change
Expand Up @@ -304,3 +304,85 @@ ok($result3 < $file_end - 10, "caller() line is not near EOF (was: $result3, EOF
- [x] Log::Log4perl tests improved (8→1 failures in t/024WarnDieCarp.t)
- [x] Full test suite passing (no regressions)
- [x] Code committed and merged (commit d4993893f)

---

## New Issue: #line Directive in Eval Context (2026-05-19)

### Problem

The `#line` directive is not being honored by `caller()` in eval context for modules like Sub::Quote.

**Test failures in Sub::Quote:**
- Test 55 expects "line 42" but gets actual file path with line 396
- Test 56 expects "welp.pl line 42" but gets actual file path with line 401

**Root Cause:**
Sub::Quote generates code with `#line` directives like `#line 41 "welp.pl"` in the `_context` function. When this code is eval'd, the `#line` directive should affect how `caller()` reports the file and line. However, the infrastructure exists (`ErrorMessageUtil.getSourceLocationAccurate`, Whitespace processing) but the information is not being correctly propagated to `caller()` at runtime.

### Investigation Findings

**Existing Infrastructure:**
1. `Whitespace.java` - Processes `#line` directives during parsing and calls `setFileName()`/`setLineNumber()` on errorUtil
2. `ErrorMessageUtil.getSourceLocationAccurate()` - Walks through tokens to find `#line` directives and returns adjusted location
3. `BytecodeInterpreter.getCallSiteInfo()` - Uses `getSourceLocationAccurate()` to get filename/line for caller()
4. `ByteCodeSourceMapper.java` - Maps bytecode PC to source location with `#line` adjustment

**Gap Identified:**
The `#line` directive is processed during parsing, but the token index mapping (`pcToTokenIndex`) might not include the `#line` directive tokens, or the token index being passed to `getSourceLocationAccurate()` is wrong. The `#line` directive is at the beginning of the eval'd code, but the token index being passed is for the actual code being executed (the sub body), which might be after the `#line` directive.

### Next Steps Plan

#### Phase C: Fix #line Directive in Eval Context

**Goal:** Ensure `#line` directives in eval'd code are honored by `caller()`.

**Investigation Steps:**
1. Verify that `#line` directive tokens are included in the token list passed to `ErrorMessageUtil`
2. Check if `pcToTokenIndex` map includes indices for `#line` directive tokens
3. Verify that `getSourceLocationAccurate()` is receiving the correct token index range

**Implementation Steps:**
1. Add debug logging to `BytecodeInterpreter.getCallSiteInfo()` to log:
- The token index being passed to `getSourceLocationAccurate()`
- The filename and line number returned
- The original filename for comparison
2. Verify that `#line` directives are in the token list at the expected positions
3. If `#line` directives are not in `pcToTokenIndex`, extend the map to include them
4. If `getSourceLocationAccurate()` is not finding `#line` directives, fix the token walking logic

**Files to modify:**
- `BytecodeInterpreter.java` - Add debug logging, fix token index lookup
- `ErrorMessageUtil.java` - Verify/fix `getSourceLocationAccurate()` logic
- `BytecodeCompiler.java` - Ensure `#line` directive tokens are included in `pcToTokenIndex`

**Testing:**
```bash
# Test Sub::Quote #line directive handling
./jperl -I/Users/fglock/projects/PerlOnJava2/src/main/perl/lib \
/Users/fglock/projects/PerlOnJava2/src/test/resources/modules/Sub-Quote/t/sub-quote.t

# Verify no regressions in croak-locations.t
cd ~/.cpan/build/Moo-2.005005-2 && \
/Users/fglock/projects/PerlOnJava2/jperl t/croak-locations.t
```

**Success Criteria:**
- Sub::Quote tests 55 and 56 pass without skip
- `caller()` returns the `#line`-adjusted filename and line number
- No regressions in existing caller stack fixes

### Status: IN PROGRESS

### Checklist
- [x] Root cause identified
- [x] Existing infrastructure documented
- [x] Gap identified
- [x] Next steps plan created
- [ ] Add debug logging to investigate token index issue
- [ ] Verify #line directive tokens in token list
- [ ] Fix pcToTokenIndex mapping if needed
- [ ] Fix getSourceLocationAccurate() logic if needed
- [ ] Remove test skips once fix is working
- [ ] Run full test suite for regressions
- [ ] Commit and merge changes
Original file line number Diff line number Diff line change
Expand Up @@ -3317,6 +3317,7 @@ private static CallerStack.CallerInfo getCallSiteInfo(InterpretedCode code, int
var entry = code.pcToTokenIndex.floorEntry(callPc);
if (entry != null && code.errorUtil != null) {
int tokenIndex = entry.getValue();
// Always use getSourceLocationAccurate to honor #line directives
ErrorMessageUtil.SourceLocation loc = code.errorUtil.getSourceLocationAccurate(tokenIndex);
filename = loc.fileName();
lineNumber = loc.lineNumber();
Expand Down
17 changes: 11 additions & 6 deletions src/main/java/org/perlonjava/frontend/parser/Whitespace.java
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,26 @@ public static int skipWhitespace(Parser parser, int tokenIndex, List<LexerToken>

case OPERATOR:
if (token.text.equals("#")) {
// # line directive must appear at the beginning of the line
// #line directive must appear at the beginning of the line
boolean maybeLineDirective = tokenIndex == 0 || tokens.get(tokenIndex - 1).type == LexerTokenType.NEWLINE;

// Skip optional whitespace after '#'
tokenIndex++;
while (tokenIndex < tokens.size() && tokens.get(tokenIndex).type == LexerTokenType.WHITESPACE) {
tokenIndex++;
}
// Check if it's a "# line" directive
// Check if it's a "#line" directive (with or without space after #)
if (maybeLineDirective && tokenIndex < tokens.size() && tokens.get(tokenIndex).text.equals("line")) {
tokenIndex = parseLineDirective(parser, tokenIndex, tokens);
}
// Skip comment until end of line
while (tokenIndex < tokens.size() && tokens.get(tokenIndex).type != LexerTokenType.NEWLINE) {
tokenIndex++;
// After processing #line directive, skip to end of line
while (tokenIndex < tokens.size() && tokens.get(tokenIndex).type != LexerTokenType.NEWLINE) {
tokenIndex++;
}
} else {
// Not a #line directive, treat as regular comment
while (tokenIndex < tokens.size() && tokens.get(tokenIndex).type != LexerTokenType.NEWLINE) {
tokenIndex++;
}
}
} else {
return tokenIndex; // Stop processing and return current index
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,10 @@ public String getFileName() {
return fileName;
}

public String getOriginalFileName() {
return originalFileName;
}

public void setFileName(String fileName) {
this.fileName = fileName;
}
Expand Down
34 changes: 11 additions & 23 deletions src/main/java/org/perlonjava/runtime/runtimetypes/RuntimeCode.java
Original file line number Diff line number Diff line change
Expand Up @@ -1592,35 +1592,23 @@ public static void storeSourceLines(String evalString, String filename, Node ast
* @param tokens Lexer tokens (may be null on compilation failure)
*/
private static void processLineDirectives(String evalString, String[] lines, List<LexerToken> tokens) {
String currentFilename = null;
int currentLineOffset = 0; // 0-based index into lines array

java.util.regex.Pattern pattern = java.util.regex.Pattern.compile("^\\s*#line\\s+(\\d+)\\s+\"([^\"]+)\"");
for (int i = 0; i < lines.length; i++) {
String line = lines[i];
// Simple #line directive parsing: #line N "filename"
// Allow optional leading whitespace
java.util.regex.Matcher m = java.util.regex.Pattern.compile("^\\s*#line\\s+(\\d+)\\s+\"([^\"]+)\"").matcher(line);
java.util.regex.Matcher m = pattern.matcher(line);
if (m.find()) {
int targetLine = Integer.parseInt(m.group(1)); // 1-based line number in target file
currentFilename = m.group(2);
currentLineOffset = i + 1; // Next line in eval corresponds to targetLine
// Ensure the target array exists and is properly sized
String targetKey = "main::_<" + currentFilename;
int lineNum = Integer.parseInt(m.group(1));
String filename = m.group(2);
// Populate @{"_<filename"} array with line mapping
String arrayName = "_<" + filename;
String targetKey = "main::" + arrayName;
RuntimeArray targetArray = GlobalVariable.getGlobalArray(targetKey);
// Ensure array is large enough (sparse behavior)
while (targetArray.elements.size() <= targetLine) {
targetArray.elements.add(RuntimeScalarCache.scalarUndef);
}
// Place the next line at the correct index
if (i + 1 < lines.length) {
targetArray.elements.set(targetLine, new RuntimeScalar(lines[i + 1] + "\n"));
if (targetArray == null) {
targetArray = new RuntimeArray();
GlobalVariable.getGlobalVariable(targetKey).set(targetArray);
}
} else if (currentFilename != null && i >= currentLineOffset) {
// Continue populating the current filename array
int targetLine = (i - currentLineOffset) + 1; // Convert to 1-based
String targetKey = "main::_<" + currentFilename;
RuntimeArray targetArray = GlobalVariable.getGlobalArray(targetKey);
// Ensure array is large enough (sparse behavior)
int targetLine = lineNum;
while (targetArray.elements.size() <= targetLine) {
targetArray.elements.add(RuntimeScalarCache.scalarUndef);
}
Expand Down
Loading
Loading