You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by jw...@apache.org on 2016/10/16 06:14:06 UTC
groovy git commit: GROOVY-7291: Declaration of double without
assignment null in closure (closes #446)
Repository: groovy
Updated Branches:
refs/heads/master 0bd39f8c9 -> 302debb89
GROOVY-7291: Declaration of double without assignment null in closure (closes #446)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/302debb8
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/302debb8
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/302debb8
Branch: refs/heads/master
Commit: 302debb894308a398d50468650b3d86c5677e150
Parents: 0bd39f8
Author: zhangbo <zh...@nanchao.org>
Authored: Mon Oct 10 17:53:36 2016 +0800
Committer: John Wagenleitner <jw...@apache.org>
Committed: Sat Oct 15 23:11:25 2016 -0700
----------------------------------------------------------------------
.../classgen/asm/OptimizingStatementWriter.java | 190 ++++++++++---------
src/test/groovy/bugs/Groovy7291Bug.groovy | 41 ++++
2 files changed, 145 insertions(+), 86 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/302debb8/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java b/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
index 41a801a..b6a54d5 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java
@@ -43,14 +43,14 @@ import static org.codehaus.groovy.ast.tools.WideningCategories.*;
* A class to write out the optimized statements
*/
public class OptimizingStatementWriter extends StatementWriter {
-
+
private static class FastPathData {
private Label pathStart = new Label();
private Label afterPath = new Label();
}
-
+
public static class ClassNodeSkip{}
-
+
public static class StatementMeta {
private boolean optimize=false;
protected MethodNode target;
@@ -83,7 +83,7 @@ public class OptimizingStatementWriter extends StatementWriter {
MethodCaller.newStatic(BytecodeInterface8.class, "isOrigF"),
MethodCaller.newStatic(BytecodeInterface8.class, "isOrigZ"),
};
-
+
private static final MethodCaller disabledStandardMetaClass = MethodCaller.newStatic(BytecodeInterface8.class, "disabledStandardMetaClass");
private boolean fastPathBlocked = false;
private final WriterController controller;
@@ -92,26 +92,26 @@ public class OptimizingStatementWriter extends StatementWriter {
super(controller);
this.controller = controller;
}
-
+
private boolean notEnableFastPath(StatementMeta meta) {
// return false if cannot do fast path and if are already on the path
return fastPathBlocked || meta==null || !meta.optimize || controller.isFastPath();
}
-
+
private FastPathData writeGuards(StatementMeta meta, Statement statement) {
if (notEnableFastPath(meta)) return null;
controller.getAcg().onLineNumber(statement, null);
MethodVisitor mv = controller.getMethodVisitor();
FastPathData fastPathData = new FastPathData();
Label slowPath = new Label();
-
+
for (int i=0; i<guards.length; i++) {
if (meta.involvedTypes[i]) {
guards[i].call(mv);
mv.visitJumpInsn(IFEQ, slowPath);
}
}
-
+
// meta class check with boolean holder
String owner = BytecodeHelper.getClassInternalName(controller.getClassNode());
MethodNode mn = controller.getMethodNode();
@@ -119,37 +119,37 @@ public class OptimizingStatementWriter extends StatementWriter {
mv.visitFieldInsn(GETSTATIC, owner, Verifier.STATIC_METACLASS_BOOL, "Z");
mv.visitJumpInsn(IFNE, slowPath);
}
-
+
//standard metaclass check
disabledStandardMetaClass.call(mv);
mv.visitJumpInsn(IFNE, slowPath);
-
+
// other guards here
-
+
mv.visitJumpInsn(GOTO, fastPathData.pathStart);
mv.visitLabel(slowPath);
-
+
return fastPathData;
}
-
+
private void writeFastPathPrelude(FastPathData meta) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitJumpInsn(GOTO, meta.afterPath);
mv.visitLabel(meta.pathStart);
controller.switchToFastPath();
}
-
+
private void writeFastPathEpilogue(FastPathData meta) {
MethodVisitor mv = controller.getMethodVisitor();
mv.visitLabel(meta.afterPath);
controller.switchToSlowPath();
}
-
+
@Override
public void writeBlockStatement(BlockStatement statement) {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
if (fastPathData==null) {
// normal mode with different paths
// important is to not to have a fastpathblock here,
@@ -162,13 +162,13 @@ public class OptimizingStatementWriter extends StatementWriter {
fastPathBlocked = true;
super.writeBlockStatement(statement);
fastPathBlocked = oldFastPathBlock;
-
+
writeFastPathPrelude(fastPathData);
super.writeBlockStatement(statement);
writeFastPathEpilogue(fastPathData);
}
}
-
+
@Override
public void writeDoWhileLoop(DoWhileStatement statement) {
if (controller.isFastPath()) {
@@ -176,19 +176,19 @@ public class OptimizingStatementWriter extends StatementWriter {
} else {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeDoWhileLoop(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeDoWhileLoop(statement);
writeFastPathEpilogue(fastPathData);
}
}
-
+
@Override
protected void writeIteratorHasNext(MethodVisitor mv) {
if (controller.isFastPath()) {
@@ -197,7 +197,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeIteratorHasNext(mv);
}
}
-
+
@Override
protected void writeIteratorNext(MethodVisitor mv) {
if (controller.isFastPath()) {
@@ -206,7 +206,7 @@ public class OptimizingStatementWriter extends StatementWriter {
super.writeIteratorNext(mv);
}
}
-
+
@Override
protected void writeForInLoop(ForStatement statement) {
if (controller.isFastPath()) {
@@ -214,12 +214,12 @@ public class OptimizingStatementWriter extends StatementWriter {
} else {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeForInLoop(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeForInLoop(statement);
@@ -234,12 +234,12 @@ public class OptimizingStatementWriter extends StatementWriter {
} else {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeForLoopWithClosureList(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeForLoopWithClosureList(statement);
@@ -254,12 +254,12 @@ public class OptimizingStatementWriter extends StatementWriter {
} else {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeWhileLoop(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeWhileLoop(statement);
@@ -274,19 +274,19 @@ public class OptimizingStatementWriter extends StatementWriter {
} else {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeIfElse(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeIfElse(statement);
writeFastPathEpilogue(fastPathData);
}
}
-
+
private boolean isNewPathFork(StatementMeta meta) {
// meta.optimize -> can do fast path
if (meta==null || meta.optimize==false) return false;
@@ -296,7 +296,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (controller.isFastPath()) return false;
return true;
}
-
+
@Override
public void writeReturn(ReturnStatement statement) {
if (controller.isFastPath()) {
@@ -305,16 +305,16 @@ public class OptimizingStatementWriter extends StatementWriter {
StatementMeta meta = (StatementMeta) statement.getNodeMetaData(StatementMeta.class);
if (isNewPathFork(meta) && writeDeclarationExtraction(statement)) {
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeReturn(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeReturn(statement);
- writeFastPathEpilogue(fastPathData);
+ writeFastPathEpilogue(fastPathData);
} else {
super.writeReturn(statement);
}
@@ -340,19 +340,19 @@ public class OptimizingStatementWriter extends StatementWriter {
// (3) fast path possible and in slow or fastPath. Nothing to do here.
//
// the only case we need to handle is then (2).
-
+
if (isNewPathFork(meta) && writeDeclarationExtraction(statement)) {
FastPathData fastPathData = writeGuards(meta, statement);
-
+
boolean oldFastPathBlock = fastPathBlocked;
fastPathBlocked = true;
super.writeExpressionStatement(statement);
fastPathBlocked = oldFastPathBlock;
-
+
if (fastPathData==null) return;
writeFastPathPrelude(fastPathData);
super.writeExpressionStatement(statement);
- writeFastPathEpilogue(fastPathData);
+ writeFastPathEpilogue(fastPathData);
} else {
super.writeExpressionStatement(statement);
}
@@ -366,15 +366,15 @@ public class OptimizingStatementWriter extends StatementWriter {
ex = rs.getExpression();
} else if (statement instanceof ExpressionStatement) {
ExpressionStatement es = (ExpressionStatement) statement;
- ex = es.getExpression();
+ ex = es.getExpression();
} else {
throw new GroovyBugError("unknown statement type :"+statement.getClass());
- }
+ }
if (!(ex instanceof DeclarationExpression)) return true;
DeclarationExpression declaration = (DeclarationExpression) ex;
ex = declaration.getLeftExpression();
if (ex instanceof TupleExpression) return false;
-
+
// do declaration
controller.getCompileStack().defineVariable(declaration.getVariableExpression(), false);
// change statement to do assignment only
@@ -390,18 +390,18 @@ public class OptimizingStatementWriter extends StatementWriter {
rs.setExpression(assignment);
} else if (statement instanceof ExpressionStatement) {
ExpressionStatement es = (ExpressionStatement) statement;
- es.setExpression(assignment);
+ es.setExpression(assignment);
} else {
throw new GroovyBugError("unknown statement type :"+statement.getClass());
}
return true;
}
-
+
public static void setNodeMeta(TypeChooser chooser, ClassNode classNode) {
if (classNode.getNodeMetaData(ClassNodeSkip.class)!=null) return;
new OptVisitor(chooser).visitClass(classNode);
}
-
+
private static StatementMeta addMeta(ASTNode node) {
StatementMeta metaOld = (StatementMeta) node.getNodeMetaData(StatementMeta.class);
StatementMeta meta = metaOld;
@@ -410,13 +410,13 @@ public class OptimizingStatementWriter extends StatementWriter {
if (metaOld==null) node.setNodeMetaData(StatementMeta.class, meta);
return meta;
}
-
+
private static StatementMeta addMeta(ASTNode node, OptimizeFlagsCollector opt) {
StatementMeta meta = addMeta(node);
meta.chainInvolvedTypes(opt);
return meta;
}
-
+
private static class OptimizeFlagsCollector {
private static class OptimizeFlagsEntry {
private boolean canOptimize = false;
@@ -488,7 +488,7 @@ public class OptimizingStatementWriter extends StatementWriter {
current.involvedTypes = new boolean[typeMapKeyNames.length];
}
}
-
+
private static class OptVisitor extends ClassCodeVisitorSupport {
private final TypeChooser typeChooser;
@@ -503,7 +503,7 @@ public class OptimizingStatementWriter extends StatementWriter {
private boolean optimizeMethodCall = true;
private VariableScope scope;
private static final VariableScope nonStaticScope = new VariableScope();
-
+
@Override
public void visitClass(ClassNode node) {
this.optimizeMethodCall = !node.implementsInterface(GROOVY_INTERCEPTABLE_TYPE);
@@ -513,20 +513,20 @@ public class OptimizingStatementWriter extends StatementWriter {
this.scope=null;
this.node=null;
}
-
+
@Override
public void visitMethod(MethodNode node) {
scope = node.getVariableScope();
super.visitMethod(node);
opt.reset();
}
-
+
@Override
public void visitConstructor(ConstructorNode node) {
scope = node.getVariableScope();
super.visitConstructor(node);
}
-
+
@Override
public void visitReturnStatement(ReturnStatement statement) {
opt.push();
@@ -534,7 +534,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (opt.shouldOptimize()) addMeta(statement,opt);
opt.pop(opt.shouldOptimize());
}
-
+
@Override
public void visitUnaryMinusExpression(UnaryMinusExpression expression) {
//TODO: implement int operations for this
@@ -542,7 +542,7 @@ public class OptimizingStatementWriter extends StatementWriter {
StatementMeta meta = addMeta(expression);
meta.type = OBJECT_TYPE;
}
-
+
@Override
public void visitUnaryPlusExpression(UnaryPlusExpression expression) {
//TODO: implement int operations for this
@@ -550,7 +550,7 @@ public class OptimizingStatementWriter extends StatementWriter {
StatementMeta meta = addMeta(expression);
meta.type = OBJECT_TYPE;
}
-
+
@Override
public void visitBitwiseNegationExpression(BitwiseNegationExpression expression) {
//TODO: implement int operations for this
@@ -558,7 +558,7 @@ public class OptimizingStatementWriter extends StatementWriter {
StatementMeta meta = addMeta(expression);
meta.type = OBJECT_TYPE;
}
-
+
private void addTypeInformation(Expression expression, Expression orig) {
ClassNode type = typeChooser.resolveType(expression, node);
if (isPrimitiveType(type)) {
@@ -568,32 +568,50 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.chainInvolvedType(type);
}
}
-
+
@Override
public void visitPrefixExpression(PrefixExpression expression) {
super.visitPrefixExpression(expression);
addTypeInformation(expression.getExpression(),expression);
}
-
+
@Override
public void visitPostfixExpression(PostfixExpression expression) {
super.visitPostfixExpression(expression);
addTypeInformation(expression.getExpression(),expression);
- }
-
+ }
+
+ private void replaceEmptyToConstantZeroIfNecessary(DeclarationExpression expression) {
+ // GROOVY-7291 and GROOVY-5570, a variable referenced by closure cannot be primitive type
+ // So here's a trick: in this case replace EmptyExpression on the right side to a ConstantExpression
+ Expression leftExpression = expression.getLeftExpression();
+ Expression rightExpression=expression.getRightExpression();
+ if (leftExpression instanceof VariableExpression
+ && rightExpression instanceof EmptyExpression) {
+ VariableExpression leftVariableExpression = (VariableExpression) leftExpression;
+
+ if (isPrimitiveType(leftVariableExpression.getOriginType())
+ && leftVariableExpression.isClosureSharedVariable()) {
+ expression.setRightExpression(new ConstantExpression(0));
+ }
+ }
+ }
+
@Override
public void visitDeclarationExpression(DeclarationExpression expression) {
- Expression right = expression.getRightExpression();
- right.visit(this);
-
- ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
+ replaceEmptyToConstantZeroIfNecessary(expression);
+
Expression rightExpression = expression.getRightExpression();
+ rightExpression.visit(this);
+
+ ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType);
- if (rightType==null) rightType = typeChooser.resolveType(expression.getRightExpression(), node);
+
+ if (rightType==null) rightType = typeChooser.resolveType(rightExpression, node);
if (isPrimitiveType(leftType) && isPrimitiveType(rightType)) {
// if right is a constant, then we optimize only if it makes
// a block complete, so we set a maybe
- if (right instanceof ConstantExpression) {
+ if (rightExpression instanceof ConstantExpression) {
opt.chainCanOptimize(true);
} else {
opt.chainShouldOptimize(true);
@@ -605,23 +623,23 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.chainInvolvedType(rightType);
}
}
-
+
@Override
public void visitBinaryExpression(BinaryExpression expression) {
if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
super.visitBinaryExpression(expression);
-
+
ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node);
ClassNode rightType = typeChooser.resolveType(expression.getRightExpression(), node);
ClassNode resultType = null;
int operation = expression.getOperation().getType();
-
+
if (operation==Types.LEFT_SQUARE_BRACKET && leftType.isArray()) {
opt.chainShouldOptimize(true);
resultType = leftType.getComponentType();
} else {
switch (operation) {
- case Types.COMPARE_EQUAL:
+ case Types.COMPARE_EQUAL:
case Types.COMPARE_LESS_THAN:
case Types.COMPARE_LESS_THAN_EQUAL:
case Types.COMPARE_GREATER_THAN:
@@ -683,7 +701,7 @@ public class OptimizingStatementWriter extends StatementWriter {
}
}
}
-
+
if (resultType!=null) {
StatementMeta meta = addMeta(expression);
meta.type = resultType;
@@ -706,7 +724,7 @@ public class OptimizingStatementWriter extends StatementWriter {
ClassNode originalResultType = typeChooser.resolveType(binExp, node);
if ( !originalResultType.equals(BigDecimal_TYPE) ||
- !(isLongCategory(assignmentTartgetType) || isFloatingCategory(assignmentTartgetType))
+ !(isLongCategory(assignmentTartgetType) || isFloatingCategory(assignmentTartgetType))
) {
return null;
}
@@ -740,7 +758,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (opt.shouldOptimize()) addMeta(statement,opt);
opt.pop(opt.shouldOptimize());
}
-
+
@Override
public void visitBlockStatement(BlockStatement block) {
opt.push();
@@ -755,12 +773,12 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.chainCanOptimize(true);
opt.pop(true);
} else {
- opt.chainShouldOptimize(optAll);
+ opt.chainShouldOptimize(optAll);
if (optAll) addMeta(block,opt);
opt.pop(optAll);
}
}
-
+
@Override
public void visitIfElse(IfStatement statement) {
opt.push();
@@ -768,7 +786,7 @@ public class OptimizingStatementWriter extends StatementWriter {
if (opt.shouldOptimize()) addMeta(statement,opt);
opt.pop(opt.shouldOptimize());
}
-
+
@Override
public void visitStaticMethodCallExpression(StaticMethodCallExpression expression) {
if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
@@ -776,23 +794,23 @@ public class OptimizingStatementWriter extends StatementWriter {
setMethodTarget(expression,expression.getMethod(), expression.getArguments(), true);
}
-
+
@Override
public void visitMethodCallExpression(MethodCallExpression expression) {
if (expression.getNodeMetaData(StatementMeta.class)!=null) return;
super.visitMethodCallExpression(expression);
-
+
Expression object = expression.getObjectExpression();
boolean setTarget = AsmClassGenerator.isThisExpression(object);
if (!setTarget) {
if (!(object instanceof ClassExpression)) return;
setTarget = object.equals(node);
}
-
+
if (!setTarget) return;
setMethodTarget(expression, expression.getMethodAsString(), expression.getArguments(), true);
}
-
+
@Override
public void visitConstructorCallExpression(ConstructorCallExpression call) {
if (call.getNodeMetaData(StatementMeta.class)!=null) return;
@@ -802,7 +820,7 @@ public class OptimizingStatementWriter extends StatementWriter {
// check the meta class of the other class
// setMethodTarget(call, "<init>", call.getArguments(), false);
}
-
+
private void setMethodTarget(Expression expression, String name, Expression callArgs, boolean isMethod) {
if (name==null) return;
if (!optimizeMethodCall) return;
@@ -839,13 +857,13 @@ public class OptimizingStatementWriter extends StatementWriter {
target = selectConstructor(type, paraTypes);
if (target==null) return;
}
-
+
StatementMeta meta = addMeta(expression);
meta.target = target;
meta.type = type;
opt.chainShouldOptimize(true);
}
-
+
private static MethodNode selectConstructor(ClassNode node, Parameter[] paraTypes) {
List<ConstructorNode> cl = node.getDeclaredConstructors();
MethodNode res = null;
@@ -870,7 +888,7 @@ public class OptimizingStatementWriter extends StatementWriter {
public void visitClosureExpression(ClosureExpression expression) {
return;
}
-
+
@Override
public void visitForLoop(ForStatement statement) {
opt.push();
@@ -879,6 +897,6 @@ public class OptimizingStatementWriter extends StatementWriter {
opt.pop(opt.shouldOptimize());
}
}
-
+
}
http://git-wip-us.apache.org/repos/asf/groovy/blob/302debb8/src/test/groovy/bugs/Groovy7291Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7291Bug.groovy b/src/test/groovy/bugs/Groovy7291Bug.groovy
new file mode 100644
index 0000000..afac56d
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7291Bug.groovy
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you 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 groovy.bugs
+
+class Groovy7291Bug extends GroovyShellTestCase {
+ void testPrimitiveDouble() {
+ evaluate('''
+double a;
+def b = {
+ a = a + 1;
+}
+b();
+ ''');
+ }
+
+ void testDouble() {
+ shouldFail('''
+Double a;
+def b = {
+ a = a + 1;
+}
+b();
+ ''');
+ }
+}