groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject [groovy] branch master updated: GROOVY-6809 and GROOVY-9168: AIC before delegate constructor call
Date Thu, 04 Jul 2019 23:24:15 GMT
This is an automated email from the ASF dual-hosted git repository.

sunlan 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 d2037b5  GROOVY-6809 and GROOVY-9168: AIC before delegate constructor call
d2037b5 is described below

commit d2037b51d8fa2704ef458633d465307707de3430
Author: Eric Milles <eric.milles@thomsonreuters.com>
AuthorDate: Mon Jul 1 13:16:27 2019 -0500

    GROOVY-6809 and GROOVY-9168: AIC before delegate constructor call
---
 .../classgen/InnerClassCompletionVisitor.java      |   2 +-
 .../groovy/classgen/InnerClassVisitor.java         | 113 +++++++-----
 .../org/codehaus/groovy/classgen/Verifier.java     |  11 ++
 src/test/gls/innerClass/InnerClassTest.groovy      | 193 +++++++++++++++++++--
 4 files changed, 267 insertions(+), 52 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..5416a3f 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
@@ -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,35 @@ public class InnerClassVisitor extends InnerClassVisitorHelper implements
Opcode
         VariableScope scope = innerClass.getVariableScope();
         if (scope == null) return;
 
+        int additionalParamCount = scope.getReferencedLocalVariablesCount();
+        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);
+            Arrays.stream(ctor.getParameters()).filter(Parameter::hasInitialExpression).forEach(p
-> 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 +209,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 +244,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 +258,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 +284,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 +294,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 c9b4f37..f55ba78 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -974,6 +974,17 @@ public class Verifier implements GroovyClassVisitor, Opcodes {
         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 892412b..966313b 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"
@@ -624,8 +624,8 @@ class A {
         '''
     }
 
+    // GROOVY-4896, GROOVY-6810
     void testThisReferenceForAICInOpenBlock() {
-        // GROOVY-6810
         assertScript '''
             import java.security.AccessController
             import java.security.PrivilegedAction
@@ -657,7 +657,6 @@ class A {
             injectVariables(t, ['p': 'q'])
         '''
 
-        //GROOVY-4896
         assertScript '''
             def doSomethingUsingLocal(){
                 logExceptions {
@@ -721,8 +720,8 @@ class A {
         '''
     }
 
+    // GROOVY-5582
     void testAICextendingAbstractInnerClass() {
-        //GROOVY-5582
         assertScript '''
             class Outer {
                 int outer() { 1 }
@@ -740,8 +739,8 @@ class A {
         '''
     }
 
+    // GROOVY-6831
     void testNestedPropertyHandling() {
-        // GROOVY-6831
         assertScript '''
             class Outer {
                 private static List items = []
@@ -763,8 +762,8 @@ class A {
         '''
     }
 
+    // GROOVY-7312
     void testInnerClassOfInterfaceIsStatic() {
-        //GROOVY-7312
         assertScript '''
             import java.lang.reflect.Modifier
             interface Baz {
@@ -775,8 +774,8 @@ class A {
         '''
     }
 
-    void testInnerClassOfInterfaceIsStaticVariant() {
-        //GROOVY-7312
+    // GROOVY-7312
+    void testInnerClassOfInterfaceIsStatic2() {
         assertScript '''
             import java.lang.reflect.Modifier
             import groovy.transform.ASTTest
@@ -793,7 +792,7 @@ class A {
         '''
     }
 
-    //GROOVY-8914
+    // GROOVY-8914
     void testNestedClassInheritingFromNestedClass() {
         // control
         assert new Outer8914.Nested()
@@ -805,6 +804,180 @@ class A {
             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 {


Mime
View raw message