Groovy-7291: reenable tests and fix the fix for indy as well
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/3c074dc2 Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/3c074dc2 Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/3c074dc2 Branch: refs/heads/parrot Commit: 3c074dc2058d4c5115f172094385d9efd302f3ce Parents: 7266497 Author: Jochen Theodorou <blackd...@gmx.org> Authored: Sun Oct 16 15:59:50 2016 +0200 Committer: Jochen Theodorou <blackd...@gmx.org> Committed: Sun Oct 16 16:01:06 2016 +0200 ---------------------------------------------------------------------- .../groovy/classgen/AsmClassGenerator.java | 6 ++++- .../groovy/classgen/asm/CompileStack.java | 15 ++++++++--- .../classgen/asm/OptimizingStatementWriter.java | 28 ++++---------------- src/test/groovy/bugs/Groovy7291Bug.groovy | 21 +-------------- 4 files changed, 22 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/groovy/blob/3c074dc2/src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java index 30a49e0..3fb441a 100644 --- a/src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -396,7 +396,11 @@ public class AsmClassGenerator extends ClassGenerator { } // we use this NOP to have a valid jump target for the various labels //mv.visitInsn(NOP); - mv.visitMaxs(0, 0); + try { + mv.visitMaxs(0, 0); + } catch (Exception e) { + throw new GroovyRuntimeException("ASM reporting processing error for "+controller.getClassNode()+"#"+node.getName()+" with signature "+node.getTypeDescriptor()+" in "+sourceFile+":"+node.getLineNumber(), e); + } } mv.visitEnd(); } http://git-wip-us.apache.org/repos/asf/groovy/blob/3c074dc2/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java ---------------------------------------------------------------------- diff --git a/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java b/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java index 8fae1cf..d7220a7 100644 --- a/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java +++ b/src/main/org/codehaus/groovy/classgen/asm/CompileStack.java @@ -658,10 +658,8 @@ public class CompileStack implements Opcodes { public BytecodeVariable defineVariable(Variable v, boolean initFromStack) { return defineVariable(v, v.getOriginType(), initFromStack); } + public BytecodeVariable defineVariable(Variable v, ClassNode variableType, boolean initFromStack) { - //TODO: any usage of this method should have different operand stack handing - // then the remove(1) here and there in this one can be removed and others - // can be changed String name = v.getName(); BytecodeVariable answer = defineVar(name, variableType, v.isClosureSharedVariable(), v.isClosureSharedVariable()); stackVariables.put(name, answer); @@ -672,7 +670,16 @@ public class CompileStack implements Opcodes { ClassNode type = answer.getType().redirect(); OperandStack operandStack = controller.getOperandStack(); - if (!initFromStack) pushInitValue(type, mv); + if (!initFromStack) { + if (ClassHelper.isPrimitiveType(v.getOriginType()) && ClassHelper.getWrapper(v.getOriginType()) == variableType) { + pushInitValue(v.getOriginType(), mv); + operandStack.push(v.getOriginType()); + operandStack.box(); + operandStack.remove(1); + } else { + pushInitValue(type, mv); + } + } operandStack.push(answer.getType()); if (answer.isHolder()) { operandStack.box(); http://git-wip-us.apache.org/repos/asf/groovy/blob/3c074dc2/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 b6a54d5..c9de6f8 100644 --- a/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java +++ b/src/main/org/codehaus/groovy/classgen/asm/OptimizingStatementWriter.java @@ -581,37 +581,19 @@ public class OptimizingStatementWriter extends StatementWriter { 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) { - replaceEmptyToConstantZeroIfNecessary(expression); - - Expression rightExpression = expression.getRightExpression(); - rightExpression.visit(this); + Expression right = expression.getRightExpression(); + right.visit(this); ClassNode leftType = typeChooser.resolveType(expression.getLeftExpression(), node); + Expression rightExpression = expression.getRightExpression(); ClassNode rightType = optimizeDivWithIntOrLongTarget(rightExpression, leftType); - - if (rightType==null) rightType = typeChooser.resolveType(rightExpression, node); + if (rightType==null) rightType = typeChooser.resolveType(expression.getRightExpression(), 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 (rightExpression instanceof ConstantExpression) { + if (right instanceof ConstantExpression) { opt.chainCanOptimize(true); } else { opt.chainShouldOptimize(true); http://git-wip-us.apache.org/repos/asf/groovy/blob/3c074dc2/src/test/groovy/bugs/Groovy7291Bug.groovy ---------------------------------------------------------------------- diff --git a/src/test/groovy/bugs/Groovy7291Bug.groovy b/src/test/groovy/bugs/Groovy7291Bug.groovy index 1e83f81..22203d0 100644 --- a/src/test/groovy/bugs/Groovy7291Bug.groovy +++ b/src/test/groovy/bugs/Groovy7291Bug.groovy @@ -18,32 +18,13 @@ */ package groovy.bugs -import groovy.transform.NotYetImplemented - class Groovy7291Bug extends GroovyTestCase { - static final boolean runningWithIndy = Boolean.getBoolean('groovy.target.indy') - - @NotYetImplemented - void testPrimitiveDoubleIndy() { - if (!runningWithIndy) return - assertScript ''' - double a - def b = { - a = a + 1 - } - b() - assert a == 1.0d - ''' - } - void testPrimitiveDouble() { - // TODO: remove this conditional and method above when fixed for Indy - if (runningWithIndy) return - assertScript ''' double a def b = { + assert a.class == Double a = a + 1 } b()
