This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/master by this push:
new 168bb9bc07 cast implicit property and fix types of within-closure
`this` conversion
168bb9bc07 is described below
commit 168bb9bc078ec7f68c47af4571ce0238496235f1
Author: Eric Milles <[email protected]>
AuthorDate: Tue Sep 17 04:22:44 2024 -0500
cast implicit property and fix types of within-closure `this` conversion
---
.../groovy/classgen/AsmClassGenerator.java | 22 ++----
.../classgen/asm/sc/StaticInvocationWriter.java | 88 +++++++++++-----------
.../classgen/asm/sc/StaticTypesCallSiteWriter.java | 4 +-
.../test/builder/ObjectGraphBuilderTest.groovy | 19 +++--
src/test/groovy/bugs/Groovy8446.groovy | 33 ++++----
.../groovy/transform/stc/ClosuresSTCTest.groovy | 20 +++--
6 files changed, 83 insertions(+), 103 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
b/src/main/java/org/codehaus/groovy/classgen/AsmClassGenerator.java
index dba0284fab..b25c542500 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 class AsmClassGenerator extends ClassGenerator {
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 class AsmClassGenerator extends ClassGenerator {
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 @@ public class AsmClassGenerator extends ClassGenerator {
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 4e81f7b08b..8cf40bc7f8 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.isNullConstant;
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.nullX;
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 class StaticInvocationWriter extends
InvocationWriter {
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 @@ public class StaticInvocationWriter extends
InvocationWriter {
lookupClassNode = target.getDeclaringClass().redirect();
}
Map<MethodNode, MethodNode> 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 @@ public class StaticInvocationWriter extends
InvocationWriter {
List<Expression> 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 @@ public class StaticInvocationWriter extends
InvocationWriter {
&& 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 67c6cdaa0d..985e640108 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 @@ public class StaticTypesCallSiteWriter extends
CallSiteWriter {
}
// 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 @@ public class StaticTypesCallSiteWriter extends
CallSiteWriter {
}
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 7191f94051..27028e440c 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 44a78c5a7f..ec9dd3359f 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 a7893321c9..3d394de6b7 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<Integer> 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<Integer> 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<Integer> fibo
- FibUtil() {
- fibo = { int x-> x<1?x:fibo(x-1)+fibo(x-2) }
+ class Fibonacci {
+ private static final Closure<Integer> 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
'''
}
