Skip to content

Fix closure placement to tail position and increase bytecode limit #103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fglock merged 1 commit into from
Dec 23, 2025
Merged

Fix closure placement to tail position and increase bytecode limit #103

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
137 changes: 52 additions & 85 deletions src/main/java/org/perlonjava/codegen/LargeBlockRefactorer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class LargeBlockRefactorer {

// Configuration thresholds
private static final int LARGE_BLOCK_ELEMENT_COUNT = 50; // Minimum elements before considering refactoring
private static final int LARGE_BYTECODE_SIZE = 30000;
private static final int LARGE_BYTECODE_SIZE = 40000;
private static final int MIN_CHUNK_SIZE = 4; // Minimum statements to extract as a chunk

// Reusable visitor for control flow detection
Expand Down Expand Up @@ -164,22 +164,7 @@ private static boolean trySmartChunking(BlockNode node, Parser parser) {
element.accept(controlFlowDetector);
if (controlFlowDetector.hasUnsafeControlFlow()) {
// Block contains last/next/redo/goto that would break if we wrap in closures
// Check if block is too large and throw appropriate error
if (node.elements.size() > LARGE_BLOCK_ELEMENT_COUNT) {
long estimatedSize = estimateTotalBytecodeSize(node.elements);
if (estimatedSize > LARGE_BYTECODE_SIZE) {
String message = "Block is too large (" + node.elements.size() + " elements, estimated " + estimatedSize + " bytes) " +
"and refactoring failed to reduce it below " + LARGE_BYTECODE_SIZE + " bytes. " +
"The block contains control flow statements that prevent safe refactoring. " +
"Consider breaking the code into smaller subroutines manually.";
if (parser != null) {
throw new PerlCompilerException(node.tokenIndex, message, parser.ctx.errorUtil);
} else {
throw new PerlCompilerException(message);
}
}
}
// Block is small enough or doesn't need refactoring
// Skip refactoring - we cannot safely refactor blocks with control flow
return false;
}
}
Expand Down Expand Up @@ -266,7 +251,8 @@ private static boolean shouldBreakChunk(Node element) {

/**
* Build nested closure structure from segments.
* Structure: direct1, sub{ chunk1, direct2, sub{ chunk2, direct3 }->(@_) }->(@_)
* Structure: direct1, direct2, sub{ chunk1, sub{ chunk2, chunk3 }->(@_) }->(@_)
* Closures are always placed at tail position to preserve variable scoping.
*
* @param segments List of segments (either Node for direct elements or List<Node> for chunks)
* @param tokenIndex token index for new nodes
Expand All @@ -278,93 +264,74 @@ private static List<Node> buildNestedStructure(List<Object> segments, int tokenI
return new ArrayList<>();
}

// Build from the end backwards to create nested structure
Node nestedClosure = null;
List<Node> result = new ArrayList<>();

for (int i = segments.size() - 1; i >= 0; i--) {

// Process segments forward, accumulating direct elements and building nested closures at the end
for (int i = 0; i < segments.size(); i++) {
Object segment = segments.get(i);

if (segment instanceof Node directNode) {
// Labels must NEVER be wrapped in closures - they must stay at block level
if (directNode instanceof LabelNode) {
// Flush any pending nested closure first
if (nestedClosure != null) {
result.add(nestedClosure);
nestedClosure = null;
}
// Add label directly to result
result.add(directNode);
} else if (nestedClosure != null) {
// Direct element - if we have a nested closure, we need to wrap everything
// Create closure containing this direct element and the nested closure
List<Node> blockElements = new ArrayList<>();
blockElements.add(directNode);
blockElements.add(nestedClosure);
BlockNode block = createMarkedBlock(blockElements, tokenIndex);
nestedClosure = new BinaryOperatorNode(
"->",
new SubroutineNode(null, null, null, block, false, tokenIndex),
new ListNode(new ArrayList<>(List.of(variableAst("@", "_", tokenIndex))), tokenIndex),
tokenIndex
);
} else {
// No nested closure yet, add to result (will be reversed later)
result.add(directNode);
}
// Direct elements (labels, variable declarations, control flow) stay at block level
result.add(directNode);
} else if (segment instanceof List) {
List<Node> chunk = (List<Node>) segment;
if (chunk.size() >= MIN_CHUNK_SIZE) {
// Create closure for this chunk
// Create closure for this chunk at tail position
// Collect remaining chunks to nest inside this closure
List<Node> blockElements = new ArrayList<>(chunk);
if (nestedClosure != null) {
blockElements.add(nestedClosure);

// Build nested closures for remaining chunks
for (int j = i + 1; j < segments.size(); j++) {
Object nextSegment = segments.get(j);
if (nextSegment instanceof Node) {
blockElements.add((Node) nextSegment);
} else if (nextSegment instanceof List) {
List<Node> nextChunk = (List<Node>) nextSegment;
if (nextChunk.size() >= MIN_CHUNK_SIZE) {
// Create nested closure for next chunk
List<Node> nestedElements = new ArrayList<>(nextChunk);
// Add all remaining segments to the nested closure
for (int k = j + 1; k < segments.size(); k++) {
Object remainingSegment = segments.get(k);
if (remainingSegment instanceof Node) {
nestedElements.add((Node) remainingSegment);
} else {
nestedElements.addAll((List<Node>) remainingSegment);
}
}
BlockNode nestedBlock = createMarkedBlock(nestedElements, tokenIndex);
Node nestedClosure = new BinaryOperatorNode(
"->",
new SubroutineNode(null, null, null, nestedBlock, false, tokenIndex),
new ListNode(new ArrayList<>(List.of(variableAst("@", "_", tokenIndex))), tokenIndex),
tokenIndex
);
blockElements.add(nestedClosure);
j = segments.size(); // Break outer loop
break;
} else {
blockElements.addAll(nextChunk);
}
}
}

BlockNode block = createMarkedBlock(blockElements, tokenIndex);
nestedClosure = new BinaryOperatorNode(
Node closure = new BinaryOperatorNode(
"->",
new SubroutineNode(null, null, null, block, false, tokenIndex),
new ListNode(new ArrayList<>(List.of(variableAst("@", "_", tokenIndex))), tokenIndex),
tokenIndex
);
result.add(closure);
break; // All remaining segments are now inside the closure
} else {
// Chunk too small - treat elements as direct
if (nestedClosure != null) {
// Wrap with nested closure
List<Node> blockElements = new ArrayList<>(chunk);
blockElements.add(nestedClosure);
BlockNode block = createMarkedBlock(blockElements, tokenIndex);
nestedClosure = new BinaryOperatorNode(
"->",
new SubroutineNode(null, null, null, block, false, tokenIndex),
new ListNode(new ArrayList<>(List.of(variableAst("@", "_", tokenIndex))), tokenIndex),
tokenIndex
);
} else {
// Add directly to result
result.addAll(chunk);
}
// Chunk too small - add elements directly
result.addAll(chunk);
}
}
}

// If we built a nested closure, add it at the beginning
if (nestedClosure != null) {
List<Node> finalResult = new ArrayList<>();
finalResult.add(nestedClosure);
// Add any direct elements that were at the end (in reverse order)
for (int i = result.size() - 1; i >= 0; i--) {
finalResult.add(result.get(i));
}
return finalResult;
}

// No nesting needed, reverse the result list
List<Node> finalResult = new ArrayList<>();
for (int i = result.size() - 1; i >= 0; i--) {
finalResult.add(result.get(i));
}
return finalResult;
return result;
}

/**
Expand Down