GROOVY-7932: generate bridge methods for private constructors during …

…static compilation (closes groovy#449)
traneHead · Oct 22, 2016 · 8213bd0 · 8213bd0
1 parent fe8914e
commit 8213bd0
Show file tree

Hide file tree

Showing 3 changed files with 107 additions and 15 deletions.
diff --git 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 void writeInvokeConstructor(final ConstructorCallExpression call) {
             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

diff --git 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.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 @@ private static void addPrivateBridgeMethods(final ClassNode node) {
         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 @@ private static void addPrivateBridgeMethods(final ClassNode node) {
                             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 @@ private static void addPrivateBridgeMethods(final ClassNode node) {
                     }
                     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));

diff --git 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
+        }
+    }
+
 }