From 3d2031b7423b5c2844d5ac561359a008de848a7b Mon Sep 17 00:00:00 2001 From: Johannes Link Date: Thu, 6 Feb 2014 15:07:56 +0100 Subject: [PATCH] Added many comments. Prepared VariableAccessReplacer for CompileStatic --- .../tailrec/CollectRecursiveCalls.groovy | 2 + .../tailrec/HasRecursiveCalls.groovy | 61 ++++++++++--------- .../tailrec/InWhileLoopWrapper.groovy | 7 +++ .../tailrec/RecursivenessTester.groovy | 8 ++- .../tailrec/ReturnAdderForClosures.groovy | 5 +- ...ReturnStatementToIterationConverter.groovy | 46 ++++++++------ .../TernaryToIfStatementConverter.groovy | 2 + .../tailrec/VariableAccessReplacer.groovy | 24 +++++--- .../tailrec/VariableExpressionReplacer.groovy | 8 ++- .../VariableExpressionTransformer.groovy | 20 ++++++ 10 files changed, 123 insertions(+), 60 deletions(-) diff --git a/src/main/org/codehaus/groovy/transform/tailrec/CollectRecursiveCalls.groovy b/src/main/org/codehaus/groovy/transform/tailrec/CollectRecursiveCalls.groovy index 3ecfbed..0742c37 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/CollectRecursiveCalls.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/CollectRecursiveCalls.groovy @@ -23,6 +23,8 @@ import org.codehaus.groovy.ast.expr.MethodCallExpression import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; /** + * Collect all recursive calls within method + * * @author Johannes Link */ @CompileStatic diff --git a/src/main/org/codehaus/groovy/transform/tailrec/HasRecursiveCalls.groovy b/src/main/org/codehaus/groovy/transform/tailrec/HasRecursiveCalls.groovy index 98dabf9..377e3d5 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/HasRecursiveCalls.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/HasRecursiveCalls.groovy @@ -19,40 +19,43 @@ import groovy.transform.CompileStatic import org.codehaus.groovy.ast.CodeVisitorSupport import org.codehaus.groovy.ast.MethodNode import org.codehaus.groovy.ast.expr.MethodCallExpression -import org.codehaus.groovy.ast.expr.StaticMethodCallExpression; +import org.codehaus.groovy.ast.expr.StaticMethodCallExpression /** + * + * Check if there are any recursive calls in a method + * * @author Johannes Link */ @CompileStatic class HasRecursiveCalls extends CodeVisitorSupport { - MethodNode method - boolean hasRecursiveCalls = false + MethodNode method + boolean hasRecursiveCalls = false + + public void visitMethodCallExpression(MethodCallExpression call) { + if (isRecursive(call)) { + hasRecursiveCalls = true + } else { + super.visitMethodCallExpression(call) + } + } + + public void visitStaticMethodCallExpression(StaticMethodCallExpression call) { + if (isRecursive(call)) { + hasRecursiveCalls = true + } else { + super.visitStaticMethodCallExpression(call) + } + } - public void visitMethodCallExpression(MethodCallExpression call) { - if (isRecursive(call)) { - hasRecursiveCalls = true - } else { - super.visitMethodCallExpression(call) - } - } + private boolean isRecursive(call) { + new RecursivenessTester().isRecursive(method: method, call: call) + } - public void visitStaticMethodCallExpression(StaticMethodCallExpression call) { - if (isRecursive(call)) { - hasRecursiveCalls = true - } else { - super.visitStaticMethodCallExpression(call) - } - } - - private boolean isRecursive(call) { - new RecursivenessTester().isRecursive(method: method, call: call) - } - - synchronized boolean test(MethodNode method) { - hasRecursiveCalls = false - this.method = method - this.method.code.visit(this) - hasRecursiveCalls - } -} + synchronized boolean test(MethodNode method) { + hasRecursiveCalls = false + this.method = method + this.method.code.visit(this) + hasRecursiveCalls + } +} \ No newline at end of file diff --git a/src/main/org/codehaus/groovy/transform/tailrec/InWhileLoopWrapper.groovy b/src/main/org/codehaus/groovy/transform/tailrec/InWhileLoopWrapper.groovy index 40eeb43..0edd6ad 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/InWhileLoopWrapper.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/InWhileLoopWrapper.groovy @@ -31,6 +31,13 @@ import org.codehaus.groovy.ast.stmt.TryCatchStatement import org.codehaus.groovy.ast.stmt.WhileStatement /** + * Wrap the body of a method in a while loop, nested in a try-catch. + * This is the first step in making a tail recursive method iterative. + * + * There are two ways to invoke the next iteration step: + * 1. "continue _RECURE_HERE_" is used by recursive calls outside of closures + * 2. "throw LOOP_EXCEPTION" is used by recursive calls within closures b/c you cannot invoke "continue" from there + * * @author Johannes Link */ @CompileStatic diff --git a/src/main/org/codehaus/groovy/transform/tailrec/RecursivenessTester.groovy b/src/main/org/codehaus/groovy/transform/tailrec/RecursivenessTester.groovy index 65eec49..0e7bc43 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/RecursivenessTester.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/RecursivenessTester.groovy @@ -26,11 +26,13 @@ import org.codehaus.groovy.ast.expr.VariableExpression /** * * Test if a method call is recursive if called within a given method node. - * Tries to handle static calls as well. + * Handles static calls as well. * * Currently known simplifications: - * Does not check for matching argument types but only considers the number of arguments. - * Does not check for matching return types; even void and any object type are considered to be compatible. + * - Does not check for method overloading or overridden methods. + * - Does not check for matching return types; even void and any object type are considered to be compatible. + * - Argument type matching could be more specific in case of static compilation. + * - Method names via a GString are never considered to be recursive * * @author Johannes Link */ diff --git a/src/main/org/codehaus/groovy/transform/tailrec/ReturnAdderForClosures.groovy b/src/main/org/codehaus/groovy/transform/tailrec/ReturnAdderForClosures.groovy index 031a6aa..8652794 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/ReturnAdderForClosures.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/ReturnAdderForClosures.groovy @@ -25,9 +25,12 @@ import org.codehaus.groovy.ast.stmt.* import org.codehaus.groovy.classgen.BytecodeSequence /** - * @author Johannes Link + * Adds explicit return statements to implicit return points in a closure. This is necessary since + * tail-recursion is detected by having the recursive call within the return statement. * * Copied a lot of code from package org.codehaus.groovy.classgen.ReturnAdder which can only be used for Methods + * + * @author Johannes Link */ class ReturnAdderForClosures extends CodeVisitorSupport { diff --git a/src/main/org/codehaus/groovy/transform/tailrec/ReturnStatementToIterationConverter.groovy b/src/main/org/codehaus/groovy/transform/tailrec/ReturnStatementToIterationConverter.groovy index dadd726..1b7c926 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/ReturnStatementToIterationConverter.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/ReturnStatementToIterationConverter.groovy @@ -15,18 +15,24 @@ */ package org.codehaus.groovy.transform.tailrec -import groovy.transform.CompileStatic -import org.codehaus.groovy.ast.expr.BinaryExpression -import org.codehaus.groovy.ast.expr.Expression -import org.codehaus.groovy.ast.expr.MethodCallExpression -import org.codehaus.groovy.ast.expr.StaticMethodCallExpression -import org.codehaus.groovy.ast.expr.VariableExpression +import org.codehaus.groovy.ast.expr.* import org.codehaus.groovy.ast.stmt.BlockStatement import org.codehaus.groovy.ast.stmt.ExpressionStatement import org.codehaus.groovy.ast.stmt.ReturnStatement import org.codehaus.groovy.ast.stmt.Statement /** + * Translates all return statements into an invocation of the next iteration. This can be either + * - "continue LOOP_LABEL": Outside closures + * - "throw LOOP_EXCEPTION": Inside closures + * + * Moreover, before adding the recur statement the iteration parameters (originally the method args) + * are set to their new value. To prevent variable aliasing parameters will be copied into temp vars + * before they are changes so that their current iteration value can be used when setting other params. + * + * There's probably place for optimizing the amount of variable copying being done, e.g. + * parameters that are only handed through must not be copied at all. + * * @author Johannes Link */ class ReturnStatementToIterationConverter { @@ -68,23 +74,29 @@ class ReturnStatementToIterationConverter { return result } - private replaceAllArgUsages(List nodes, tempMapping) { - def unusedTempNames = new HashSet(tempMapping.values()*.name) + private replaceAllArgUsages(List nodes, Map tempMapping) { + Set unusedTempNames = new HashSet(tempMapping.values()*.name) + VariableReplacedListener tracker = new UsedVariableTracker() for (ExpressionStatement statement : nodes) { - unusedTempNames.removeAll(replaceArgUsageByTempUsage(statement.expression, tempMapping)) + replaceArgUsageByTempUsage(statement.expression, tempMapping, tracker) } + unusedTempNames = unusedTempNames - tracker.usedVariableNames return unusedTempNames } - private replaceArgUsageByTempUsage(BinaryExpression binary, Map tempMapping) { - Set usedTempNames = [] as Set - def useTempInstead = { Map tempNameAndType -> - usedTempNames << tempNameAndType.name - AstHelper.createVariableReference(tempNameAndType) - } - VariableAccessReplacer replacer = new VariableAccessReplacer(nameAndTypeMapping: tempMapping, replaceBy: useTempInstead) + private void replaceArgUsageByTempUsage(BinaryExpression binary, Map tempMapping, UsedVariableTracker tracker) { + VariableAccessReplacer replacer = new VariableAccessReplacer(nameAndTypeMapping: tempMapping, listener: tracker) // Replacement must only happen in binary.rightExpression. It's a hack in VariableExpressionReplacer which takes care of that. replacer.replaceIn(binary) - return usedTempNames + } +} + +class UsedVariableTracker implements VariableReplacedListener { + + final Set usedVariableNames = [] as Set + + @Override + void variableReplaced(VariableExpression oldVar, VariableExpression newVar) { + usedVariableNames.add(newVar.name) } } diff --git a/src/main/org/codehaus/groovy/transform/tailrec/TernaryToIfStatementConverter.groovy b/src/main/org/codehaus/groovy/transform/tailrec/TernaryToIfStatementConverter.groovy index 35600ea..b42f0f2 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/TernaryToIfStatementConverter.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/TernaryToIfStatementConverter.groovy @@ -21,6 +21,8 @@ import org.codehaus.groovy.ast.stmt.ReturnStatement import org.codehaus.groovy.ast.stmt.Statement /** + * Since a ternary statement has more than one exit point tail-recursiveness testing cannot be easily done. + * Therefore this class translates a ternary statement (or Elvis operator) into the equivalent if-else statement. * @author Johannes Link */ class TernaryToIfStatementConverter { diff --git a/src/main/org/codehaus/groovy/transform/tailrec/VariableAccessReplacer.groovy b/src/main/org/codehaus/groovy/transform/tailrec/VariableAccessReplacer.groovy index 6b6ccbd..2d5b638 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/VariableAccessReplacer.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/VariableAccessReplacer.groovy @@ -24,7 +24,7 @@ import org.codehaus.groovy.ast.expr.VariableExpression * The variable names to replace as well as their replacement name and type have to be configured * in nameAndTypeMapping before calling replaceIn(). * - * The closure replaceBy can be changed if clients want to do more in case of replacement. + * The VariableReplacedListener can be set if clients want to react to variable replacement. * * @author Johannes Link */ @@ -32,9 +32,8 @@ import org.codehaus.groovy.ast.expr.VariableExpression class VariableAccessReplacer { Map nameAndTypeMapping = [:] //e.g.: ['varToReplace': [name: 'newVar', type: TypeOfVar]] - Closure replaceBy = { Map nameAndType -> AstHelper.createVariableReference(nameAndType) } -// VariableReplacedListener listener = {VariableExpression oldVar, VariableExpression newVar ->} as VariableReplacedListener + VariableReplacedListener listener = VariableReplacedListener.NULL void replaceIn(ASTNode root) { Closure whenParam = { VariableExpression expr -> @@ -42,14 +41,23 @@ class VariableAccessReplacer { } Closure replaceWithLocalVariable = { VariableExpression expr -> Map nameAndType = nameAndTypeMapping[expr.name] - return replaceBy(nameAndType) + VariableExpression newVar = AstHelper.createVariableReference(nameAndType) + listener.variableReplaced(expr, newVar) + return newVar } new VariableExpressionReplacer(when: whenParam, replaceWith: replaceWithLocalVariable).replaceIn(root) } } -//@CompileStatic -//interface VariableReplacedListener { -// void variableReplaced(VariableExpression oldVar, VariableExpression newVar) -//} \ No newline at end of file +@CompileStatic +interface VariableReplacedListener { + void variableReplaced(VariableExpression oldVar, VariableExpression newVar) + + static VariableReplacedListener NULL = new VariableReplacedListener() { + @Override + void variableReplaced(VariableExpression oldVar, VariableExpression newVar) { + //do nothing + } + } +} \ No newline at end of file diff --git a/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionReplacer.groovy b/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionReplacer.groovy index abb3db4..36243ad 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionReplacer.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionReplacer.groovy @@ -30,8 +30,11 @@ import java.lang.reflect.Method /** * Tool for replacing VariableExpression instances in an AST by other VariableExpression instances. * Regardless of a real change taking place in nested expressions, all considered expression (trees) will be replaced. + * This could be optimized to accelerate compilation. * - * Within @TailRecursive it is used to swap the access of arg with the access of temp vars + * Within @TailRecursive it is used + * - to swap the access of method args with the access to iteration variables + * - to swap the access of iteration variables with the access of temp vars * * @author Johannes Link */ @@ -64,7 +67,8 @@ class VariableExpressionReplacer extends CodeVisitorSupport { } /** - * It's the only Expression in which replacing is considered. + * It's the only Expression type in which replacing is considered. + * That's an abuse of the class, but I couldn't think of a better way. */ public void visitBinaryExpression(BinaryExpression expression) { //A hack: Only replace right expression b/c ReturnStatementToIterationConverter needs it that way :-/ diff --git a/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionTransformer.groovy b/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionTransformer.groovy index 667653a..81ffb43 100644 --- a/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionTransformer.groovy +++ b/src/main/org/codehaus/groovy/transform/tailrec/VariableExpressionTransformer.groovy @@ -1,3 +1,18 @@ +/* + * Copyright 2013-2014 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.codehaus.groovy.transform.tailrec import groovy.transform.CompileStatic @@ -5,6 +20,11 @@ import org.codehaus.groovy.ast.expr.Expression import org.codehaus.groovy.ast.expr.ExpressionTransformer import org.codehaus.groovy.ast.expr.VariableExpression +/** + * An expression transformer used in the process of replacing the access to variables + * + * @author Johannes Link + */ @CompileStatic class VariableExpressionTransformer implements ExpressionTransformer {