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 2016/11/10 20:20:16 UTC
groovy git commit: GROOVY-7970: Can't call private method from outer
class when using anonymous inner classes and @CS (closes #452)
Repository: groovy
Updated Branches:
refs/heads/master 6f39e27d7 -> ad566692d
GROOVY-7970: Can't call private method from outer class when using anonymous inner classes and @CS (closes #452)
Project: http://git-wip-us.apache.org/repos/asf/groovy/repo
Commit: http://git-wip-us.apache.org/repos/asf/groovy/commit/ad566692
Tree: http://git-wip-us.apache.org/repos/asf/groovy/tree/ad566692
Diff: http://git-wip-us.apache.org/repos/asf/groovy/diff/ad566692
Branch: refs/heads/master
Commit: ad566692dbc355e1962ba6b61a5b8fe1dad8b966
Parents: 6f39e27
Author: paulk <pa...@asert.com.au>
Authored: Tue Oct 25 14:30:08 2016 +1000
Committer: paulk <pa...@asert.com.au>
Committed: Fri Nov 11 06:06:10 2016 +1000
----------------------------------------------------------------------
.../classgen/asm/sc/StaticInvocationWriter.java | 62 +++++++--
.../stc/StaticTypeCheckingVisitor.java | 7 +-
src/test/groovy/bugs/Groovy7970Bug.groovy | 125 +++++++++++++++++++
3 files changed, 184 insertions(+), 10 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/groovy/blob/ad566692/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 0541d20..09ca8ae 100644
--- a/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
+++ b/src/main/org/codehaus/groovy/classgen/asm/sc/StaticInvocationWriter.java
@@ -22,6 +22,7 @@ import org.codehaus.groovy.ast.ASTNode;
import org.codehaus.groovy.ast.ClassHelper;
import org.codehaus.groovy.ast.ClassNode;
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;
@@ -186,7 +187,16 @@ public class StaticInvocationWriter extends InvocationWriter {
/**
* Attempts to make a direct method call on a bridge method, if it exists.
*/
+ @Deprecated
protected boolean tryBridgeMethod(MethodNode target, Expression receiver, boolean implicitThis, TupleExpression args) {
+ return tryBridgeMethod(target, receiver, implicitThis, args, null);
+ }
+
+ /**
+ * Attempts to make a direct method call on a bridge method, if it exists.
+ */
+ protected boolean tryBridgeMethod(MethodNode target, Expression receiver, boolean implicitThis,
+ TupleExpression args, ClassNode thisClass) {
ClassNode lookupClassNode;
if (target.isProtected()) {
lookupClassNode = controller.getClassNode();
@@ -203,8 +213,22 @@ public class StaticInvocationWriter extends InvocationWriter {
MethodNode bridge = bridges==null?null:bridges.get(target);
if (bridge != null) {
Expression fixedReceiver = receiver;
- if (implicitThis && !controller.isInClosure()) {
- fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
+ if (implicitThis) {
+ if (!controller.isInClosure()) {
+ fixedReceiver = new PropertyExpression(new ClassExpression(lookupClassNode), "this");
+ } else if (thisClass != null) {
+ ClassNode current = thisClass.getOuterClass();
+ fixedReceiver = new VariableExpression("thisObject", current);
+ // adjust for multiple levels of nesting if needed
+ while (current != null && current instanceof InnerClassNode && !lookupClassNode.equals(current)) {
+ FieldNode thisField = current.getField("this$0");
+ current = current.getOuterClass();
+ if (thisField != null) {
+ fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
+ fixedReceiver.setType(current);
+ }
+ }
+ }
}
ArgumentListExpression newArgs = new ArgumentListExpression(target.isStatic()?new ConstantExpression(null):fixedReceiver);
for (Expression expression : args.getExpressions()) {
@@ -261,7 +285,7 @@ public class StaticInvocationWriter extends InvocationWriter {
&& controller.isInClosure()
&& !target.isPublic()
&& target.getDeclaringClass() != classNode) {
- if (!tryBridgeMethod(target, receiver, implicitThis, args)) {
+ if (!tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
// replace call with an invoker helper call
ArrayExpression arr = new ArrayExpression(ClassHelper.OBJECT_TYPE, args.getExpressions());
MethodCallExpression mce = new MethodCallExpression(
@@ -281,6 +305,8 @@ public class StaticInvocationWriter extends InvocationWriter {
}
return true;
}
+ Expression fixedReceiver = null;
+ boolean fixedImplicitThis = implicitThis;
if (target.isPrivate()) {
if (tryPrivateMethod(target, implicitThis, receiver, args, classNode)) return true;
} else if (target.isProtected()) {
@@ -295,16 +321,36 @@ public class StaticInvocationWriter extends InvocationWriter {
controller.getSourceUnit().addError(
new SyntaxException("Method " + target.getName() + " is protected in " + target.getDeclaringClass().toString(false),
src.getLineNumber(), src.getColumnNumber(), src.getLastLineNumber(), src.getLastColumnNumber()));
- } else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args)) {
+ } else if (!node.isDerivedFrom(target.getDeclaringClass()) && tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
}
+ } else if (target.isPublic() && receiver != null) {
+ if (implicitThis
+ && !classNode.isDerivedFrom(target.getDeclaringClass())
+ && !classNode.implementsInterface(target.getDeclaringClass())
+ && classNode instanceof InnerClassNode && controller.isInClosure()) {
+ ClassNode current = classNode.getOuterClass();
+ fixedReceiver = new VariableExpression("thisObject", current);
+ // adjust for multiple levels of nesting if needed
+ while (current != null && current instanceof InnerClassNode && !classNode.equals(current)) {
+ FieldNode thisField = current.getField("this$0");
+ current = current.getOuterClass();
+ if (thisField != null) {
+ fixedReceiver = new PropertyExpression(fixedReceiver, "this$0");
+ fixedReceiver.setType(current);
+ fixedImplicitThis = false;
+ }
+ }
+ }
}
if (receiver != null) {
- if (!(receiver instanceof VariableExpression) || !((VariableExpression) receiver).isSuperExpression()) {
+ boolean callToSuper = receiver instanceof VariableExpression && ((VariableExpression) receiver).isSuperExpression();
+ if (!callToSuper) {
+ fixedReceiver = fixedReceiver == null ? receiver : fixedReceiver;
// in order to avoid calls to castToType, which is the dynamic behaviour, we make sure that we call CHECKCAST instead
// then replace the top operand type
- Expression checkCastReceiver = new CheckcastReceiverExpression(receiver, target);
- return super.writeDirectMethodCall(target, implicitThis, checkCastReceiver, args);
+ Expression checkCastReceiver = new CheckcastReceiverExpression(fixedReceiver, target);
+ return super.writeDirectMethodCall(target, fixedImplicitThis, checkCastReceiver, args);
}
}
return super.writeDirectMethodCall(target, implicitThis, receiver, args);
@@ -316,7 +362,7 @@ public class StaticInvocationWriter extends InvocationWriter {
if ((isPrivateBridgeMethodsCallAllowed(declaringClass, classNode) || isPrivateBridgeMethodsCallAllowed(classNode, declaringClass))
&& declaringClass.getNodeMetaData(PRIVATE_BRIDGE_METHODS) != null
&& !declaringClass.equals(classNode)) {
- if (tryBridgeMethod(target, receiver, implicitThis, args)) {
+ if (tryBridgeMethod(target, receiver, implicitThis, args, classNode)) {
return true;
} else if (declaringClass != classNode) {
controller.getSourceUnit().addError(new SyntaxException("Cannot call private method " + (target.isStatic() ? "static " : "") +
http://git-wip-us.apache.org/repos/asf/groovy/blob/ad566692/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
----------------------------------------------------------------------
diff --git a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
index c21166b..f16ff82 100644
--- a/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
+++ b/src/main/org/codehaus/groovy/transform/stc/StaticTypeCheckingVisitor.java
@@ -3849,8 +3849,11 @@ public class StaticTypeCheckingVisitor extends ClassCodeVisitorSupport {
collectAllInterfaceMethodsByName(receiver, name, methods);
methods.addAll(OBJECT_TYPE.getMethods(name));
}
- if (typeCheckingContext.getEnclosingClosure() == null) {
- // not in a closure
+ // TODO: investigate the trait exclusion a bit further, needed otherwise
+ // CallMethodOfTraitInsideClosureAndClosureParamTypeInference fails saying
+ // not static method can't be called from a static context
+ if (typeCheckingContext.getEnclosingClosure() == null || (receiver instanceof InnerClassNode && !receiver.getName().endsWith("$Trait$Helper"))) {
+ // not in a closure or within an inner class
ClassNode parent = receiver;
while (parent instanceof InnerClassNode && !parent.isStaticClass()) {
parent = parent.getOuterClass();
http://git-wip-us.apache.org/repos/asf/groovy/blob/ad566692/src/test/groovy/bugs/Groovy7970Bug.groovy
----------------------------------------------------------------------
diff --git a/src/test/groovy/bugs/Groovy7970Bug.groovy b/src/test/groovy/bugs/Groovy7970Bug.groovy
new file mode 100644
index 0000000..9bda2a3
--- /dev/null
+++ b/src/test/groovy/bugs/Groovy7970Bug.groovy
@@ -0,0 +1,125 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package groovy.bugs
+
+class Groovy7970Bug extends GroovyTestCase {
+
+ private static final String getScriptAIC(String visibility, boolean cs) {
+ """
+ ${ cs ? '@groovy.transform.CompileStatic' : ''}
+ class Bar {
+ $visibility String renderTemplate(String arg) { "dummy\$arg" }
+ def foo() {
+ new Object() {
+ String bar() {
+ 'A'.with{ renderTemplate(it) }
+ }
+ }
+ }
+ }
+ assert new Bar().foo().bar() == 'dummyA'
+ """
+ }
+
+ private static final String getScriptNestedAIC(String visibility, boolean cs) {
+ """
+ ${ cs ? '@groovy.transform.CompileStatic' : ''}
+ class Bar {
+ $visibility String renderTemplate(String arg) { "dummy\$arg" }
+ def foo() {
+ new Object() {
+ def bar() {
+ new Object() { String xxx() { 'B'.with{ renderTemplate(it) } }}
+ }
+ }
+ }
+ }
+ assert new Bar().foo().bar().xxx() == 'dummyB'
+ """
+ }
+
+ private static final String getScriptInner(String visibility, boolean cs) {
+ """
+ ${ cs ? '@groovy.transform.CompileStatic' : ''}
+ class Bar {
+ $visibility String renderTemplate(String arg) { "dummy\$arg" }
+ class Inner {
+ String baz() {
+ 'C'.with{ renderTemplate(it) }
+ }
+ }
+ }
+ def b = new Bar()
+ assert new Bar.Inner(b).baz() == 'dummyC'
+ """
+ }
+
+ private static final String getScriptNestedInner(String visibility, boolean cs) {
+ """
+ ${ cs ? '@groovy.transform.CompileStatic' : ''}
+ class Bar {
+ $visibility String renderTemplate(String arg) { "dummy\$arg" }
+ class Inner {
+ class InnerInner {
+ String xxx() { 'D'.with{ renderTemplate(it) } }
+ }
+ }
+ }
+ def b = new Bar()
+ def bi = new Bar.Inner(b)
+ assert new Bar.Inner.InnerInner(bi).xxx() == 'dummyD'
+ """
+ }
+
+ void testMethodAccessFromClosureWithinInnerClass() {
+ assertScript getScriptInner('public', false)
+ assertScript getScriptInner('protected', false)
+ assertScript getScriptInner('private', false)
+ assertScript getScriptNestedInner('public', false)
+ assertScript getScriptNestedInner('protected', false)
+ assertScript getScriptNestedInner('private', false)
+ }
+
+ void testMethodAccessFromClosureWithinInnerClass_CS() {
+ assertScript getScriptInner('public', true)
+ assertScript getScriptInner('protected', true)
+ assertScript getScriptInner('private', true)
+ assertScript getScriptNestedInner('public', true)
+ assertScript getScriptNestedInner('protected', true)
+ assertScript getScriptNestedInner('private', true)
+ }
+
+ void testMethodAccessFromClosureWithinAIC() {
+ assertScript getScriptAIC('public', false)
+ assertScript getScriptAIC('protected', false)
+ assertScript getScriptAIC('private', false)
+ assertScript getScriptNestedAIC('public', false)
+ assertScript getScriptNestedAIC('protected', false)
+ assertScript getScriptNestedAIC('private', false)
+ }
+
+ void testMethodAccessFromClosureWithinAIC_CS() {
+ assertScript getScriptAIC('public', true)
+ assertScript getScriptAIC('protected', true)
+ assertScript getScriptAIC('private', true)
+ assertScript getScriptNestedAIC('public', true)
+ assertScript getScriptNestedAIC('protected', true)
+ assertScript getScriptNestedAIC('private', true)
+ }
+}