From c309fbbb589b0e38c5794f7d770bb4a3a2a447c5 Mon Sep 17 00:00:00 2001 From: Flavio Soibelmann Glock Date: Tue, 23 Dec 2025 12:29:12 +0100 Subject: [PATCH] Fix closure placement to tail position and increase bytecode limit - Rewrite buildNestedStructure to place closures at tail position instead of head - This ensures variable declarations execute before closures that use them - Preserves lexical scoping by keeping direct elements at the start of blocks - Remove error throwing for blocks with unsafe control flow - Simply skip refactoring for blocks containing last/next/redo/goto statements - Increase LARGE_BYTECODE_SIZE limit from 30000 to 40000 bytes - Allows larger blocks before requiring refactoring --- .../codegen/LargeBlockRefactorer.java | 137 +++++++----------- 1 file changed, 52 insertions(+), 85 deletions(-) diff --git a/src/main/java/org/perlonjava/codegen/LargeBlockRefactorer.java b/src/main/java/org/perlonjava/codegen/LargeBlockRefactorer.java index 79f2d685..3204ece2 100644 --- a/src/main/java/org/perlonjava/codegen/LargeBlockRefactorer.java +++ b/src/main/java/org/perlonjava/codegen/LargeBlockRefactorer.java @@ -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 @@ -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; } } @@ -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 for chunks) * @param tokenIndex token index for new nodes @@ -278,93 +264,74 @@ private static List buildNestedStructure(List segments, int tokenI return new ArrayList<>(); } - // Build from the end backwards to create nested structure - Node nestedClosure = null; List 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 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 chunk = (List) 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 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 nextChunk = (List) nextSegment; + if (nextChunk.size() >= MIN_CHUNK_SIZE) { + // Create nested closure for next chunk + List 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) 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 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 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 finalResult = new ArrayList<>(); - for (int i = result.size() - 1; i >= 0; i--) { - finalResult.add(result.get(i)); - } - return finalResult; + return result; } /**