diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java index dba0284fab5..b25c542500a 100644 --- a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java +++ b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java @@ -896,18 +896,6 @@ public void visitLambdaExpression(final LambdaExpression expression) { controller.getLambdaWriter().writeLambda(expression); } - /** - * Loads either this object or if we're inside a closure then load the top level owner - */ - protected void loadThisOrOwner() { - ClassNode classNode = controller.getClassNode(); - if (classNode.getOuterClass() == null) { - loadThis(VariableExpression.THIS_EXPRESSION); - } else { - fieldX(classNode.getDeclaredField("owner")).visit(this); - } - } - /** * Generates byte code for constants. * @@ -1417,11 +1405,17 @@ public void visitVariableExpression(final VariableExpression expression) { if (variable != null) { controller.getOperandStack().loadOrStoreVariable(variable, expression.isUseReferenceDirectly()); } else { - PropertyExpression pexp = thisPropX(true, expression.getName()); + PropertyExpression pexp = thisPropX(/*implicit-this*/true, expression.getName()); pexp.getProperty().setSourcePosition(expression); pexp.setType(expression.getType()); pexp.copyNodeMetaData(expression); pexp.visit(this); + + if (!compileStack.isLHS() && !expression.isDynamicTyped()) { + ClassNode variableType = controller.getTypeChooser() + .resolveType(expression, controller.getClassNode()); + controller.getOperandStack().doGroovyCast(variableType); + } } if (!compileStack.isLHS()) { @@ -2375,7 +2369,7 @@ private void loadThis(final VariableExpression thisOrSuper) { OperandStack operandStack = controller.getOperandStack(); if (controller.isInGeneratedFunction() && !controller.getCompileStack().isImplicitThis()) { mv.visitMethodInsn(INVOKEVIRTUAL, "groovy/lang/Closure", "getThisObject", "()Ljava/lang/Object;", false); - ClassNode expectedType = controller.getTypeChooser().resolveType(thisOrSuper, controller.getOutermostClass()); + ClassNode expectedType = controller.getTypeChooser().resolveType(thisOrSuper, controller.getThisType() ); if (!isObjectType(expectedType) && !isPrimitiveType(expectedType)) { BytecodeHelper.doCast(mv, expectedType); operandStack.push(expectedType); diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java index 4e81f7b08ba..8cf40bc7f8e 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java @@ -79,10 +79,12 @@ import static org.apache.groovy.ast.tools.ExpressionUtils.isSuperExpression; import static org.apache.groovy.ast.tools.ExpressionUtils.isThisOrSuper; import static org.codehaus.groovy.ast.ClassHelper.isGStringType; +import static org.codehaus.groovy.ast.ClassHelper.isGeneratedFunction; import static org.codehaus.groovy.ast.ClassHelper.isObjectType; import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid; import static org.codehaus.groovy.ast.ClassHelper.isStringType; import static org.codehaus.groovy.ast.tools.GeneralUtils.args; +import static org.codehaus.groovy.ast.tools.GeneralUtils.attrX; import static org.codehaus.groovy.ast.tools.GeneralUtils.callX; import static org.codehaus.groovy.ast.tools.GeneralUtils.classX; import static org.codehaus.groovy.ast.tools.GeneralUtils.constX; @@ -91,6 +93,7 @@ import static org.codehaus.groovy.ast.tools.GeneralUtils.propX; import static org.codehaus.groovy.ast.tools.GeneralUtils.stmt; import static org.codehaus.groovy.ast.tools.GeneralUtils.varX; +import static org.codehaus.groovy.ast.tools.GenericsUtils.makeClassSafe0; import static org.codehaus.groovy.transform.trait.Traits.isTrait; import static org.objectweb.asm.Opcodes.ACONST_NULL; import static org.objectweb.asm.Opcodes.ALOAD; @@ -213,6 +216,33 @@ public void writeSpecialConstructorCall(final ConstructorCallExpression call) { controller.getCompileStack().pop(); } + private Expression thisObjectExpression(final ClassNode source, final ClassNode target) { + ClassNode thisType = source; + while (isGeneratedFunction(thisType)) { + thisType = thisType.getOuterClass(); + } + Expression thisExpr; + if (isTrait(thisType.getOuterClass())) { + thisExpr = varX("thisObject"); // GROOVY-7242, GROOVY-8127 + } else if (controller.isStaticContext()) { + thisExpr = varX("thisObject", makeClassSafe0(ClassHelper.CLASS_Type, thisType.asGenericsType())); + } else { + thisExpr = varX("thisObject", thisType); + // adjust for multiple levels of nesting + while (!thisType.isDerivedFrom(target) && !thisType.implementsInterface(target)) { + FieldNode thisZero = thisType.getDeclaredField("this$0"); + if (thisZero != null) { + thisExpr = attrX(thisExpr, "this$0"); + thisExpr.setType(thisZero.getType()); + thisType = thisType.getOuterClass(); + if (thisType != null) continue; + } + break; + } + } + return thisExpr; + } + /** * Attempts to make a direct method call on a bridge method, if it exists. */ @@ -238,32 +268,22 @@ protected boolean tryBridgeMethod(final MethodNode target, final Expression rece lookupClassNode = target.getDeclaringClass().redirect(); } Map bridges = lookupClassNode.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS); - MethodNode bridge = bridges == null ? null : bridges.get(target); + MethodNode bridge = (bridges == null ? null : bridges.get(target)); if (bridge != null) { - Expression fixedReceiver = receiver; + Expression newReceiver = receiver; if (implicitThis) { if (!controller.isInGeneratedFunction()) { if (!thisClass.isDerivedFrom(lookupClassNode)) - fixedReceiver = propX(classX(lookupClassNode), "this"); + newReceiver = propX(classX(lookupClassNode), "this"); } else if (thisClass != null) { - ClassNode current = thisClass.getOuterClass(); - fixedReceiver = varX("thisObject", current); - // adjust for multiple levels of nesting if needed - while (current.getOuterClass() != null && !lookupClassNode.equals(current)) { - FieldNode thisField = current.getField("this$0"); - current = current.getOuterClass(); - if (thisField != null) { - fixedReceiver = propX(fixedReceiver, "this$0"); - fixedReceiver.setType(current); - } - } + newReceiver = thisObjectExpression(thisClass, lookupClassNode); } } - ArgumentListExpression newArgs = args(target.isStatic() ? nullX() : fixedReceiver); + ArgumentListExpression newArguments = args(target.isStatic() ? nullX() : newReceiver); for (Expression expression : args.getExpressions()) { - newArgs.addExpression(expression); + newArguments.addExpression(expression); } - return writeDirectMethodCall(bridge, implicitThis, fixedReceiver, newArgs); + return writeDirectMethodCall(bridge, implicitThis, newReceiver, newArguments); } return false; } @@ -284,22 +304,10 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i List argumentList = new ArrayList<>(); if (emn.isStaticExtension()) { argumentList.add(nullX()); + } else if (!isThisOrSuper(receiver) || !controller.isInGeneratedFunction()) { + argumentList.add(receiver); } else { - Expression fixedReceiver = null; - if (isThisOrSuper(receiver) && classNode.getOuterClass() != null && controller.isInGeneratedFunction()) { - ClassNode current = classNode.getOuterClass(); - fixedReceiver = varX("thisObject", current); - // adjust for multiple levels of nesting if needed - while (current.getOuterClass() != null && !classNode.equals(current)) { - FieldNode thisField = current.getField("this$0"); - current = current.getOuterClass(); - if (thisField != null) { - fixedReceiver = propX(fixedReceiver, "this$0"); - fixedReceiver.setType(current); - } - } - } - argumentList.add(fixedReceiver != null ? fixedReceiver : receiver); + argumentList.add(thisObjectExpression(classNode, target.getDeclaringClass())); } argumentList.addAll(args.getExpressions()); loadArguments(argumentList, parameters); @@ -367,20 +375,8 @@ protected boolean writeDirectMethodCall(final MethodNode target, final boolean i && controller.isInGeneratedFunction() && !classNode.isDerivedFrom(target.getDeclaringClass()) && !classNode.implementsInterface(target.getDeclaringClass())) { - ClassNode thisType = controller.getThisType(); - if (isTrait(thisType.getOuterClass())) thisType = ClassHelper.dynamicType(); // GROOVY-7242 - - fixedReceiver = varX("thisObject", thisType); - // account for multiple levels of inner types - while (thisType.getOuterClass() != null && !target.getDeclaringClass().equals(thisType)) { - FieldNode thisField = thisType.getField("this$0"); - thisType = thisType.getOuterClass(); - if (thisField != null) { - fixedReceiver = propX(fixedReceiver, "this$0"); - fixedReceiver.setType(thisType); - fixedImplicitThis = false; - } - } + fixedReceiver = thisObjectExpression(classNode, target.getDeclaringClass()); + if (!(fixedReceiver instanceof VariableExpression)) fixedImplicitThis = false; } } if (receiver != null && !isSuperExpression(receiver) && !isClassWithSuper(receiver)) { diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java index 67c6cdaa0d9..985e640108d 100644 --- a/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java +++ b/src/main/java/org/codehaus/groovy/classgen/asm/sc/StaticTypesCallSiteWriter.java @@ -476,7 +476,7 @@ private boolean makeGetPropertyWithGetter(final Expression receiver, final Class } // check outer class if (implicitThis && receiverType instanceof InnerClassNode && !receiverType.isStaticClass()) { - if (makeGetPropertyWithGetter(receiver, receiverType.getOuterClass(), propertyName, safe, implicitThis)) { + if (makeGetPropertyWithGetter(receiver, receiverType.getOuterClass(), propertyName, safe, implicitThis)) { return true; } } @@ -524,8 +524,8 @@ boolean makeGetField(final Expression receiver, final ClassNode receiverType, fi } mv.visitLabel(skip); } + operandStack.replace(resultType); } - operandStack.replace(resultType); return true; } return false; diff --git a/src/spec/test/builder/ObjectGraphBuilderTest.groovy b/src/spec/test/builder/ObjectGraphBuilderTest.groovy index 7191f94051c..27028e440c1 100644 --- a/src/spec/test/builder/ObjectGraphBuilderTest.groovy +++ b/src/spec/test/builder/ObjectGraphBuilderTest.groovy @@ -22,11 +22,11 @@ import asciidoctor.Utils import groovy.test.GroovyTestCase /** -* Tests for ObjectGraphBuilder. The tests directly in this file are specific -* to ObjectGraphBuilder. Functionality in common with other Builders -* is tested in the parent class. -*/ -class ObjectGraphBuilderTest extends GroovyTestCase { + * Tests for ObjectGraphBuilder. The tests directly in this file are specific + * to ObjectGraphBuilder. Functionality in common with other Builders + * is tested in the parent class. + */ +final class ObjectGraphBuilderTest extends GroovyTestCase { void testBuilder() { assertScript '''// tag::domain_classes[] @@ -78,7 +78,6 @@ assert employee.address instanceof Address ''' } - void testBuildImmutableFailure() { def err = shouldFail ''' package com.acme @@ -133,12 +132,12 @@ builder.newInstanceResolver = { Class klazz, Map attributes -> } // end::newinstanceresolver[] def person = builder.person(name:'Jon', age:17) - ''' } void testId() { - assertScript '''package com.acme + assertScript ''' +package com.acme class Company { String name @@ -179,6 +178,6 @@ assert e1.name == 'Duke' assert e2.name == 'John' assert e1.address.line1 == '123 Groovy Rd' assert e2.address.line1 == '123 Groovy Rd' -''' + ''' } -} \ No newline at end of file +} diff --git a/src/test/groovy/bugs/Groovy8446.groovy b/src/test/groovy/bugs/Groovy8446.groovy index 44a78c5a7fe..ec9dd3359fd 100644 --- a/src/test/groovy/bugs/Groovy8446.groovy +++ b/src/test/groovy/bugs/Groovy8446.groovy @@ -18,19 +18,16 @@ */ package groovy.bugs -import groovy.transform.CompileStatic import org.junit.Test +import static groovy.test.GroovyAssert.assertScript import static groovy.test.GroovyAssert.shouldFail -@CompileStatic final class Groovy8446 { - private GroovyShell shell = new GroovyShell() - @Test void testVoidArray0() { - shell.evaluate ''' + assertScript ''' class C { Void[] m() {} } @@ -40,25 +37,21 @@ final class Groovy8446 { @Test void testVoidArray1() { - def err = shouldFail { - shell.evaluate ''' - class C { - void[] m() {} - } - ''' - } - assert err =~ /void\[\] is an invalid type/ || err =~ /Unexpected input: '\('/ + def err = shouldFail ''' + class C { + void[] m() {} + } + ''' + assert err =~ /void\[\] is an invalid type|Unexpected input: '\('/ } @Test void testVoidArray2() { - def err = shouldFail { - shell.evaluate ''' - class C { - def meth(void... args) {} - } - ''' - } + def err = shouldFail ''' + class C { + def meth(void... args) {} + } + ''' assert err =~ /void\[\] is an invalid type|void is not allowed here/ } } diff --git a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy index a7893321c9f..3d394de6b79 100644 --- a/src/test/groovy/transform/stc/ClosuresSTCTest.groovy +++ b/src/test/groovy/transform/stc/ClosuresSTCTest.groovy @@ -628,30 +628,28 @@ class ClosuresSTCTest extends StaticTypeCheckingTestCase { void testRecurseClosureCallAsMethod() { assertScript ''' Closure cl - cl = { int x -> x == 0 ? x : 1+cl(x-1) } + cl = { int x -> x == 0 ? x : 1 + cl(x-1) } ''' } void testFibClosureCallAsMethod() { assertScript ''' Closure fib - fib = { int x-> x<1?x:fib(x-1)+fib(x-2) } - fib(2) + fib = { int x -> x<2 ? x : fib(x-1) + fib(x-2) } + assert fib(2) == 1 ''' } void testFibClosureCallAsMethodFromWithinClass() { assertScript ''' - class FibUtil { - private Closure fibo - FibUtil() { - fibo = { int x-> x<1?x:fibo(x-1)+fibo(x-2) } + class Fibonacci { + private static final Closure f + static { + f = { int x -> x<2 ? x : f(x-1) + f(x-2) } } - - int fib(int n) { fibo(n) } + static int of(int n) { f(n) } } - FibUtil fib = new FibUtil() - fib.fib(2) + assert Fibonacci.of(2) == 1 ''' }