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/parrot 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 + } + } + }
