You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by pa...@apache.org on 2020/05/29 07:16:32 UTC
[groovy] branch GROOVY_2_5_X updated: GROOVY-6809 and GROOVY-9168:
AIC before delegate constructor call (port to 2_5_X)
This is an automated email from the ASF dual-hosted git repository.
paulk pushed a commit to branch GROOVY_2_5_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_2_5_X by this push:
new 52fa307 GROOVY-6809 and GROOVY-9168: AIC before delegate constructor call (port to 2_5_X)
52fa307 is described below
commit 52fa307d9d4a924a9593b49d43218f478249a021
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Mon Jul 1 13:16:27 2019 -0500
GROOVY-6809 and GROOVY-9168: AIC before delegate constructor call (port to 2_5_X)
---
.../classgen/InnerClassCompletionVisitor.java | 2 +-
.../groovy/classgen/InnerClassVisitor.java | 119 ++++++++-----
.../org/codehaus/groovy/classgen/Verifier.java | 19 +-
src/test/gls/innerClass/InnerClassTest.groovy | 193 +++++++++++++++++++--
4 files changed, 277 insertions(+), 56 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
index 452ac65..1e52435 100644
--- a/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassCompletionVisitor.java
@@ -160,7 +160,7 @@ public class InnerClassCompletionVisitor extends InnerClassVisitorHelper impleme
private void getThis(MethodVisitor mv, String classInternalName, String outerClassDescriptor, String innerClassInternalName) {
mv.visitVarInsn(ALOAD, 0);
- if (CLOSURE_TYPE.equals(thisField.getType())) {
+ if (thisField != null && CLOSURE_TYPE.equals(thisField.getType())) {
mv.visitFieldInsn(GETFIELD, classInternalName, "this$0", CLOSURE_DESCRIPTOR);
mv.visitMethodInsn(INVOKEVIRTUAL, CLOSURE_INTERNAL_NAME, "getThisObject", "()Ljava/lang/Object;", false);
mv.visitTypeInsn(CHECKCAST, innerClassInternalName);
diff --git a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
index d2d570f..90dd015 100644
--- a/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
+++ b/src/main/java/org/codehaus/groovy/classgen/InnerClassVisitor.java
@@ -20,11 +20,15 @@ package org.codehaus.groovy.classgen;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
+import org.codehaus.groovy.ast.CodeVisitorSupport;
+import org.codehaus.groovy.ast.ConstructorNode;
import org.codehaus.groovy.ast.FieldNode;
+import org.codehaus.groovy.ast.GroovyCodeVisitor;
import org.codehaus.groovy.ast.InnerClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.Parameter;
import org.codehaus.groovy.ast.PropertyNode;
+import org.codehaus.groovy.ast.Variable;
import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.ClosureExpression;
import org.codehaus.groovy.ast.expr.ConstructorCallExpression;
@@ -40,19 +44,22 @@ import org.codehaus.groovy.transform.trait.Traits;
import org.objectweb.asm.Opcodes;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Iterator;
import java.util.List;
+import static org.codehaus.groovy.ast.tools.GeneralUtils.classX;
+import static org.codehaus.groovy.ast.tools.GenericsUtils.nonGeneric;
+
public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcodes {
private final SourceUnit sourceUnit;
private ClassNode classNode;
- private static final int PUBLIC_SYNTHETIC = Opcodes.ACC_PUBLIC + Opcodes.ACC_SYNTHETIC;
- private FieldNode thisField = null;
+ private FieldNode thisField;
private MethodNode currentMethod;
private FieldNode currentField;
- private boolean processingObjInitStatements = false;
- private boolean inClosure = false;
+ private boolean processingObjInitStatements;
+ private boolean inClosure;
public InnerClassVisitor(CompilationUnit cu, SourceUnit su) {
sourceUnit = su;
@@ -65,13 +72,13 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
@Override
public void visitClass(ClassNode node) {
- this.classNode = node;
+ classNode = node;
thisField = null;
InnerClassNode innerClass = null;
if (!node.isEnum() && !node.isInterface() && node instanceof InnerClassNode) {
innerClass = (InnerClassNode) node;
if (!isStatic(innerClass) && innerClass.getVariableScope() == null) {
- thisField = innerClass.addField("this$0", PUBLIC_SYNTHETIC, node.getOuterClass().getPlainNodeReference(), null);
+ thisField = innerClass.addField("this$0", ACC_FINAL | ACC_SYNTHETIC, node.getOuterClass().getPlainNodeReference(), null);
}
}
@@ -85,7 +92,7 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
node.setUnresolvedSuperClass(ClassHelper.OBJECT_TYPE);
}
}
-
+
@Override
public void visitClosureExpression(ClosureExpression expression) {
boolean inClosureOld = inClosure;
@@ -103,7 +110,7 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
@Override
protected void visitConstructorOrMethod(MethodNode node, boolean isConstructor) {
- this.currentMethod = node;
+ currentMethod = node;
visitAnnotations(node);
visitClassCodeContainer(node.getCode());
// GROOVY-5681: initial expressions should be visited too!
@@ -113,14 +120,14 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
}
visitAnnotations(param);
}
- this.currentMethod = null;
+ currentMethod = null;
}
@Override
public void visitField(FieldNode node) {
- this.currentField = node;
+ currentField = node;
super.visitField(node);
- this.currentField = null;
+ currentField = null;
}
@Override
@@ -133,7 +140,7 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
}
@Override
- public void visitConstructorCallExpression(ConstructorCallExpression call) {
+ public void visitConstructorCallExpression(final ConstructorCallExpression call) {
super.visitConstructorCallExpression(call);
if (!call.isUsingAnonymousInnerClass()) {
passThisReference(call);
@@ -143,9 +150,8 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
InnerClassNode innerClass = (InnerClassNode) call.getType();
ClassNode outerClass = innerClass.getOuterClass();
ClassNode superClass = innerClass.getSuperClass();
- if (superClass instanceof InnerClassNode
- && !superClass.isInterface()
- && !(superClass.isStaticClass()||((superClass.getModifiers()&ACC_STATIC)==ACC_STATIC))) {
+ if (!superClass.isInterface() && superClass.getOuterClass() != null
+ && !(superClass.isStaticClass() || (superClass.getModifiers() & ACC_STATIC) != 0)) {
insertThis0ToSuperCall(call, innerClass);
}
if (!innerClass.getDeclaredConstructors().isEmpty()) return;
@@ -154,17 +160,39 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
VariableScope scope = innerClass.getVariableScope();
if (scope == null) return;
+ int additionalParamCount = scope.getReferencedLocalVariablesCount();
+ final boolean[] precedesSuperOrThisCall = new boolean[1];
+ if (currentMethod instanceof ConstructorNode) {
+ ConstructorNode ctor = (ConstructorNode) currentMethod;
+ GroovyCodeVisitor visitor = new CodeVisitorSupport() {
+ @Override
+ public void visitConstructorCallExpression(ConstructorCallExpression cce) {
+ if (cce == call) {
+ precedesSuperOrThisCall[0] = true;
+ } else {
+ super.visitConstructorCallExpression(cce);
+ }
+ }
+ };
+ if (ctor.firstStatementIsSpecialConstructorCall()) currentMethod.getFirstStatement().visit(visitor);
+ for (Parameter p : ctor.getParameters()) {
+ if (p.hasInitialExpression()) {
+ p.getInitialExpression().visit(visitor);
+ }
+ }
+ }
+ if (!precedesSuperOrThisCall[0]) additionalParamCount += 1;
+
// expressions = constructor call arguments
List<Expression> expressions = ((TupleExpression) call.getArguments()).getExpressions();
// block = init code for the constructor we produce
BlockStatement block = new BlockStatement();
// parameters = parameters of the constructor
- final int additionalParamCount = 1 + scope.getReferencedLocalVariablesCount();
- List<Parameter> parameters = new ArrayList<Parameter>(expressions.size() + additionalParamCount);
+ List<Parameter> parameters = new ArrayList<>(expressions.size() + additionalParamCount);
// superCallArguments = arguments for the super call == the constructor call arguments
- List<Expression> superCallArguments = new ArrayList<Expression>(expressions.size());
+ List<Expression> superCallArguments = new ArrayList<>(expressions.size());
- // first we add a super() call for all expressions given in the
+ // first we add a super() call for all expressions given in the
// constructor call expression
int pCount = additionalParamCount;
for (Expression expr : expressions) {
@@ -185,24 +213,32 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
block.addStatement(new ExpressionStatement(cce));
- // we need to add "this" to access unknown methods/properties
- // this is saved in a field named this$0
pCount = 0;
- expressions.add(pCount, VariableExpression.THIS_EXPRESSION);
- boolean isStatic = isStaticThis(innerClass,scope);
- ClassNode outerClassType = getClassNode(outerClass, isStatic);
- if (!isStatic && inClosure) outerClassType = ClassHelper.CLOSURE_TYPE;
- outerClassType = outerClassType.getPlainNodeReference();
- Parameter thisParameter = new Parameter(outerClassType, "p" + pCount);
- parameters.add(pCount, thisParameter);
-
- thisField = innerClass.addField("this$0", PUBLIC_SYNTHETIC, outerClassType, null);
- addFieldInit(thisParameter, thisField, block);
-
- // for each shared variable we add a reference and save it as field
- for (Iterator it = scope.getReferencedLocalVariablesIterator(); it.hasNext();) {
+ boolean isStatic = isStaticThis(innerClass, scope);
+ ClassNode outerClassType;
+ if (!isStatic && inClosure) {
+ outerClassType = ClassHelper.CLOSURE_TYPE.getPlainNodeReference();
+ } else {
+ outerClassType = getClassNode(outerClass, isStatic).getPlainNodeReference();
+ }
+ if (!precedesSuperOrThisCall[0]) {
+ // need to pass "this" to access unknown methods/properties
+ expressions.add(pCount, VariableExpression.THIS_EXPRESSION);
+
+ Parameter thisParameter = new Parameter(outerClassType, "p" + pCount);
+ parameters.add(pCount, thisParameter);
+
+ // "this" reference is saved in a field named "this$0"
+ thisField = innerClass.addField("this$0", ACC_FINAL | ACC_SYNTHETIC, outerClassType, null);
+ addFieldInit(thisParameter, thisField, block);
+ }/* else {
+ thisField = innerClass.addField("this$0", ACC_FINAL | ACC_SYNTHETIC, nonGeneric(ClassHelper.CLASS_Type), classX(outerClassType));
+ }*/
+
+ // for each shared variable, add a Reference field
+ for (Iterator<Variable> it = scope.getReferencedLocalVariablesIterator(); it.hasNext();) {
pCount++;
- org.codehaus.groovy.ast.Variable var = (org.codehaus.groovy.ast.Variable) it.next();
+ Variable var = it.next();
VariableExpression ve = new VariableExpression(var);
ve.setClosureSharedVariable(true);
ve.setUseReferenceDirectly(true);
@@ -212,10 +248,10 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
Parameter p = new Parameter(rawReferenceType, "p" + pCount);
parameters.add(pCount, p);
p.setOriginType(var.getOriginType());
- final VariableExpression initial = new VariableExpression(p);
+ VariableExpression initial = new VariableExpression(p);
initial.setSynthetic(true);
initial.setUseReferenceDirectly(true);
- final FieldNode pField = innerClass.addFieldFirst(ve.getName(), PUBLIC_SYNTHETIC,rawReferenceType, initial);
+ FieldNode pField = innerClass.addFieldFirst(ve.getName(), ACC_PUBLIC | ACC_SYNTHETIC, rawReferenceType, initial);
pField.setHolder(true);
pField.setOriginType(ClassHelper.getWrapper(var.getOriginType()));
}
@@ -226,11 +262,11 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
private boolean isStaticThis(InnerClassNode innerClass, VariableScope scope) {
if (inClosure) return false;
boolean ret = innerClass.isStaticClass();
- if ( innerClass.getEnclosingMethod()!=null) {
+ if (innerClass.getEnclosingMethod() != null) {
ret = ret || innerClass.getEnclosingMethod().isStatic();
- } else if (currentField!=null) {
+ } else if (currentField != null) {
ret = ret || currentField.isStatic();
- } else if (currentMethod!=null && "<clinit>".equals(currentMethod.getName())) {
+ } else if (currentMethod != null && "<clinit>".equals(currentMethod.getName())) {
ret = true;
}
return ret;
@@ -252,7 +288,7 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
// if constructor call is not in static context, return
if (isInStaticContext) {
- // constructor call is in static context and the inner class is non-static - 1st arg is supposed to be
+ // constructor call is in static context and the inner class is non-static - 1st arg is supposed to be
// passed as enclosing "this" instance
//
Expression args = call.getArguments();
@@ -262,7 +298,6 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements Opcode
return;
}
insertThis0ToSuperCall(call, cn);
-
}
private void insertThis0ToSuperCall(final ConstructorCallExpression call, final ClassNode cn) {
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 0ed2678..4a7f394 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -893,9 +893,22 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
});
}
- protected void addConstructor(Parameter[] newParams, ConstructorNode ctor, Statement code, ClassNode node) {
- ConstructorNode genConstructor = node.addConstructor(ctor.getModifiers(), newParams, ctor.getExceptions(), code);
- markAsGenerated(node, genConstructor);
+ protected void addConstructor(Parameter[] newParams, ConstructorNode ctor, Statement code, ClassNode type) {
+ final ConstructorNode newConstructor = type.addConstructor(ctor.getModifiers(), newParams, ctor.getExceptions(), code);
+ newConstructor.putNodeMetaData(DEFAULT_PARAMETER_GENERATED, Boolean.TRUE);
+ markAsGenerated(type, newConstructor);
+ // TODO: Copy annotations, etc.?
+
+ // set anon. inner enclosing method reference
+ code.visit(new CodeVisitorSupport() {
+ @Override
+ public void visitConstructorCallExpression(ConstructorCallExpression call) {
+ if (call.isUsingAnonymousInnerClass()) {
+ call.getType().setEnclosingMethod(newConstructor);
+ }
+ super.visitConstructorCallExpression(call);
+ }
+ });
}
/**
diff --git a/src/test/gls/innerClass/InnerClassTest.groovy b/src/test/gls/innerClass/InnerClassTest.groovy
index 8e622ee..ab548b6 100644
--- a/src/test/gls/innerClass/InnerClassTest.groovy
+++ b/src/test/gls/innerClass/InnerClassTest.groovy
@@ -68,9 +68,9 @@ class InnerClassTest extends CompilableTestSupport {
void testExtendsObjectAndReferenceAMethodParameterWithinAGString() {
assertScript """
Object makeObj0(String name) {
- new Object() {
+ new Object() {
String toString() { "My name is \${name}" }
- }
+ }
}
assert makeObj0("Guillaume").toString() == "My name is Guillaume"
@@ -731,8 +731,8 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
+ // GROOVY-4896, GROOVY-6810
void testThisReferenceForAICInOpenBlock() {
- // GROOVY-6810
assertScript '''
import java.security.AccessController
import java.security.PrivilegedAction
@@ -764,7 +764,6 @@ import org.codehaus.groovy.classgen.Verifier
injectVariables(t, ['p': 'q'])
'''
- //GROOVY-4896
assertScript '''
def doSomethingUsingLocal(){
logExceptions {
@@ -828,8 +827,8 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
+ // GROOVY-5582
void testAICextendingAbstractInnerClass() {
- //GROOVY-5582
assertScript '''
class Outer {
int outer() { 1 }
@@ -847,8 +846,8 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
+ // GROOVY-6831
void testNestedPropertyHandling() {
- // GROOVY-6831
assertScript '''
class Outer {
private static List items = []
@@ -870,8 +869,8 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
+ // GROOVY-7312
void testInnerClassOfInterfaceIsStatic() {
- //GROOVY-7312
assertScript '''
import java.lang.reflect.Modifier
interface Baz {
@@ -882,8 +881,8 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
- void testInnerClassOfInterfaceIsStaticVariant() {
- //GROOVY-7312
+ // GROOVY-7312
+ void testInnerClassOfInterfaceIsStatic2() {
assertScript '''
import java.lang.reflect.Modifier
import groovy.transform.ASTTest
@@ -900,7 +899,7 @@ import org.codehaus.groovy.classgen.Verifier
'''
}
- //GROOVY-8914
+ // GROOVY-8914
void testNestedClassInheritingFromNestedClass() {
// control
assert new Outer8914.Nested()
@@ -912,6 +911,180 @@ import org.codehaus.groovy.classgen.Verifier
assert new OuterReferencingPrecompiled.Nested()
'''
}
+
+ // GROOVY-6809
+ void _FIXME_testReferenceToUninitializedThis() {
+ assertScript '''
+ class Test {
+ static main(args) {
+ def a = new A()
+ }
+
+ static class A {
+ A() {
+ def b = new B()
+ }
+
+ void sayA() {
+ println 'saying A'
+ }
+
+ class B extends A {
+ B() {
+ super(A.this) // does not exist
+ sayA()
+ }
+ }
+ }
+ }
+ '''
+ }
+
+ // GROOVY-6809
+ void testReferenceToUninitializedThis2() {
+ assertScript '''
+ class A {
+ A() {
+ this(new Runnable() {
+ @Override
+ void run() {
+ }
+ })
+ }
+
+ private A(Runnable action) {
+ }
+ }
+
+ new A()
+ '''
+ }
+
+ // GROOVY-6809
+ void testReferenceToUninitializedThis3() {
+ assertScript '''
+ class A {
+ A(x) {
+ }
+ }
+ class B extends A {
+ B() {
+ super(new Object() {})
+ }
+ }
+
+ new B()
+ '''
+ }
+
+ // GROOVY-7609
+ void _FIXME_testReferenceToUninitializedThis4() {
+ assertScript '''
+ class Login {
+ Login() {
+ def navBar = new LoginNavigationBar()
+ }
+
+ class LoginNavigationBar {
+ ExploreDestinationsDropdown exploreDestinationsDropdown
+
+ LoginNavigationBar() {
+ exploreDestinationsDropdown = new ExploreDestinationsDropdown()
+ }
+
+ class ExploreDestinationsDropdown /*extends NavigationBarDropdown<ExploreDestinationsDropdown>*/ {
+ ExploreDestinationsDropdown() {
+ //super(Login.this.sw, 0)
+ Login.this.sw
+ }
+ }
+ }
+
+ static main(args) {
+ new Login()
+ }
+ }
+ '''
+ }
+
+ // GROOVY-9168
+ void _FIXME_testReferenceToUninitializedThis5() {
+ assertScript '''
+ class Outer {
+ class Inner {
+ }
+ Outer(Inner inner = new Inner()) {
+ }
+ }
+ new Outer()
+ '''
+ }
+
+ // GROOVY-9168
+ void testReferenceToUninitializedThis6() {
+ assertScript '''
+ import groovy.transform.ASTTest
+ import java.util.concurrent.Callable
+ import org.codehaus.groovy.ast.expr.*
+ import static org.codehaus.groovy.classgen.Verifier.*
+ import static org.codehaus.groovy.control.CompilePhase.*
+
+ class A {
+ @ASTTest(phase=CLASS_GENERATION, value={
+ def init = node.parameters[0].getNodeMetaData(INITIAL_EXPRESSION)
+ assert init instanceof ConstructorCallExpression
+ assert init.isUsingAnonymousInnerClass()
+ assert init.type.enclosingMethod != null
+ assert init.type.enclosingMethod.name == '<init>'
+ assert init.type.enclosingMethod.parameters.length == 0 // ensure the enclosing method is A(), not A(Runnable)
+ })
+ A(Callable action = new Callable() { def call() { return 42 }}) {
+ this.action = action
+ }
+ Callable action
+ }
+
+ def a = new A()
+ assert a.action.call() == 42
+ '''
+ }
+
+ // GROOVY-9168
+ void _FIXME_testReferenceToUninitializedThis7() {
+ assertScript '''
+ class A {
+ // AIC in this position can use static properties:
+ A(Runnable action = new Runnable() { void run() { answer = 42 }}) {
+ this.action = action
+ }
+ Runnable action
+ static int answer
+ }
+
+ def a = new A()
+ a.action.run();
+ assert a.answer == 42
+ '''
+ }
+
+ // GROOVY-9168
+ void _FIXME_testReferenceToUninitializedThis8() {
+ assertScript '''
+ class A {
+ // AIC in this position can use static methods:
+ A(Runnable action = new Runnable() { void run() { setAnswer(42) }}) {
+ this.action = action
+ }
+ Runnable action
+ protected static int answer
+ static void setAnswer(int value) { answer = value }
+ }
+
+ def a = new A()
+ a.action.run();
+ assert a.answer == 42
+ '''
+ }
}
class Parent8914 {