You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by sh...@apache.org on 2016/10/22 21:13:29 UTC

groovy git commit: GROOVY-7932: generate bridge methods for private constructors during static compilation (closes #449)

Repository: groovy
Updated Branches:
  refs/heads/master fe8914e07 -> 8213bd079


GROOVY-7932: generate bridge methods for private constructors during static compilation (closes #449)


Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/8213bd07
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/8213bd07
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/8213bd07

Branch: refs/heads/master
Commit: 8213bd0792d2741d0c743b4269570a1fd390b2bd
Parents: fe8914e
Author: Shil Sinha <sh...@apache.org>
Authored: Sun Oct 9 16:57:47 2016 -0400
Committer: Shil Sinha <sh...@apache.org>
Committed: Sat Oct 22 16:06:16 2016 -0400

----------------------------------------------------------------------
 .../classgen/asm/sc/StaticInvocationWriter.java | 19 ++++++-
 .../transform/sc/StaticCompilationVisitor.java  | 45 +++++++++++----
 .../asm/sc/StaticCompileConstructorsTest.groovy | 58 ++++++++++++++++++++
 3 files changed, 107 insertions(+), 15 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/groovy/blob/8213bd07/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
index 18b91c7..0541d20 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -126,21 +126,34 @@ public class StaticInvocationWriter extends InvocationWriter {
             cn = new ConstructorNode(mn.getModifiers(), mn.getParameters(), mn.getExceptions(), mn.getCode());
             cn.setDeclaringClass(mn.getDeclaringClass());
         }
+        TupleExpression args = makeArgumentList(call.getArguments());
         if (cn.isPrivate()) {
             ClassNode classNode = controller.getClassNode();
             ClassNode declaringClass = cn.getDeclaringClass();
             if (declaringClass != classNode) {
-                controller.getSourceUnit().addError(new SyntaxException("Cannot call private constructor for " + declaringClass.toString(false) +
+                MethodNode bridge = null;
+                if (call.getNodeMetaData(StaticTypesMarker.PV_METHODS_ACCESS) != null) {
+                    Map<MethodNode, MethodNode> bridgeMethods = declaringClass.getNodeMetaData(StaticCompilationMetadataKeys.PRIVATE_BRIDGE_METHODS);
+                    bridge = bridgeMethods != null ? bridgeMethods.get(cn) : null;
+                }
+                if (bridge != null && bridge instanceof ConstructorNode) {
+                    ArgumentListExpression newArgs = new ArgumentListExpression(new ConstantExpression(null));
+                    for (Expression arg: args) {
+                        newArgs.addExpression(arg);
+                    }
+                    cn = (ConstructorNode) bridge;
+                    args = newArgs;
+                } else {
+                    controller.getSourceUnit().addError(new SyntaxException("Cannot call private constructor for " + declaringClass.toString(false) +
                             " from class " + classNode.toString(false), call.getLineNumber(), call.getColumnNumber(), mn.getLastLineNumber(), call.getLastColumnNumber()));
+                }
             }
         }
 
         String ownerDescriptor = prepareConstructorCall(cn);
-        TupleExpression args = makeArgumentList(call.getArguments());
         int before = controller.getOperandStack().getStackLength();
         loadArguments(args.getExpressions(), cn.getParameters());
         finnishConstructorCall(cn, ownerDescriptor, controller.getOperandStack().getStackLength() - before);
-
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/groovy/blob/8213bd07/src/main/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java b/src/main/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
index f385c49..1e00c50 100644
--- a/src/main/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/sc/StaticCompilationVisitor.java
@@ -46,6 +46,8 @@ import static org.codehaus.groovy.ast.tools.GenericsUtils.*;
 import static org.codehaus.groovy.transform.sc.StaticCompilationMetadataKeys.*;
 import static org.codehaus.groovy.transform.stc.StaticTypesMarker.DIRECT_METHOD_CALL_TARGET;
 import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
+import static org.objectweb.asm.Opcodes.ACC_STATIC;
+import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
 
 /**
  * This visitor is responsible for amending the AST with static compilation metadata or transform the AST so that
@@ -248,6 +250,7 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
         Set<ASTNode> accessedMethods = (Set<ASTNode>) node.getNodeMetaData(StaticTypesMarker.PV_METHODS_ACCESS);
         if (accessedMethods==null) return;
         List<MethodNode> methods = new ArrayList<MethodNode>(node.getAllDeclaredMethods());
+        methods.addAll(node.getDeclaredConstructors());
         Map<MethodNode, MethodNode> privateBridgeMethods = (Map<MethodNode, MethodNode>) node.getNodeMetaData(PRIVATE_BRIDGE_METHODS);
         if (privateBridgeMethods!=null) {
             // private bridge methods already added
@@ -273,7 +276,6 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
                             orig.getName()
                     );
                 }
-                newParams[0] = new Parameter(node.getPlainNodeReference(), "$that");
                 Expression arguments;
                 if (method.getParameters()==null || method.getParameters().length==0) {
                     arguments = ArgumentListExpression.EMPTY_ARGUMENTS;
@@ -284,17 +286,36 @@ public class StaticCompilationVisitor extends StaticTypeCheckingVisitor {
                     }
                     arguments = new ArgumentListExpression(args);
                 }
-                Expression receiver = method.isStatic()?new ClassExpression(node):new VariableExpression(newParams[0]);
-                MethodCallExpression mce = new MethodCallExpression(receiver, method.getName(), arguments);
-                mce.setMethodTarget(method);
-
-                ExpressionStatement returnStatement = new ExpressionStatement(mce);
-                MethodNode bridge = node.addMethod(
-                        "access$"+i, access,
-                        correctToGenericsSpecRecurse(genericsSpec, method.getReturnType(), methodSpecificGenerics),
-                        newParams,
-                        method.getExceptions(),
-                        returnStatement);
+
+                MethodNode bridge;
+                if (method instanceof ConstructorNode) {
+                    // create constructor with a nested class as the first parameter, creating one if necessary
+                    ClassNode thatType = null;
+                    Iterator<InnerClassNode> innerClasses = node.getInnerClasses();
+                    if (innerClasses.hasNext()) {
+                        thatType = innerClasses.next();
+                    } else {
+                        thatType = new InnerClassNode(node.redirect(), node.getName() + "$1", ACC_STATIC | ACC_SYNTHETIC, ClassHelper.OBJECT_TYPE);
+                        node.getModule().addClass(thatType);
+                    }
+                    newParams[0] = new Parameter(thatType.getPlainNodeReference(), "$that");
+                    Expression cce = new ConstructorCallExpression(ClassNode.THIS, arguments);
+                    Statement body = new ExpressionStatement(cce);
+                    bridge = node.addConstructor(ACC_SYNTHETIC, newParams, ClassNode.EMPTY_ARRAY, body);
+                } else {
+                    newParams[0] = new Parameter(node.getPlainNodeReference(), "$that");
+                    Expression receiver = method.isStatic()?new ClassExpression(node):new VariableExpression(newParams[0]);
+                    MethodCallExpression mce = new MethodCallExpression(receiver, method.getName(), arguments);
+                    mce.setMethodTarget(method);
+
+                    ExpressionStatement returnStatement = new ExpressionStatement(mce);
+                    bridge = node.addMethod(
+                            "access$"+i, access,
+                            correctToGenericsSpecRecurse(genericsSpec, method.getReturnType(), methodSpecificGenerics),
+                            newParams,
+                            method.getExceptions(),
+                            returnStatement);
+                }
                 GenericsType[] origGenericsTypes = method.getGenericsTypes();
                 if (origGenericsTypes !=null) {
                     bridge.setGenericsTypes(applyGenericsContextToPlaceHolders(genericsSpec,origGenericsTypes));

http://git-wip-us.apache.org/repos/asf/groovy/blob/8213bd07/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
----------------------------------------------------------------------
diff --git a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
index ac56657..c20fe76 100644
--- a/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
+++ b/src/test/org/codehaus/groovy/classgen/asm/sc/StaticCompileConstructorsTest.groovy
@@ -60,5 +60,63 @@ class StaticCompileConstructorsTest extends ConstructorsSTCTest implements Stati
             }
         """, "Cannot statically compile constructor implicitly including non static elements from object initializers, properties or fields")
     }
+
+    void testPrivateConstructorFromClosure() {
+        try {
+            assertScript '''
+                class Foo {
+                    String s
+                    private Foo(String s) { this.s = s }
+                    static Foo makeFoo(String s) {
+                        def cl = { new Foo(s) }
+                        cl()
+                    }
+                }
+                assert Foo.makeFoo('pls').s == 'pls'
+            '''
+        } finally {
+            //println astTrees
+        }
+    }
+
+    void testPrivateConstructorFromNestedClass() {
+        try {
+            assertScript '''
+                class Foo {
+                    String s
+                    private Foo(String s) { this.s = s }
+                    static class Bar {
+                        static Foo makeFoo(String s) { new Foo(s) }
+                    }
+
+                }
+                assert Foo.Bar.makeFoo('pls').s == 'pls'
+            '''
+        } finally {
+            //println astTrees
+        }
+    }
+
+    void testPrivateConstructorFromAIC() {
+        try {
+            assertScript '''
+                class Foo {
+                    String s
+                    private Foo(String s) { this.s = s }
+                    static Foo makeFoo(String s) {
+                        return new Object() {
+                            Foo makeFoo(String x) {
+                                new Foo(x)
+                            }
+                        }.makeFoo(s)
+                    }
+                }
+                assert Foo.makeFoo('pls').s == 'pls'
+            '''
+        } finally {
+            //println astTrees
+        }
+    }
+
 }