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 d09ba4e GROOVY-9223: Avoid generating common methods for each groovy
class
d09ba4e is described below
commit d09ba4e9389763fd8de781c5da018b31680a008e
Author: Daniel Sun <[email protected]>
AuthorDate: Tue Aug 13 16:32:50 2019 +0800
GROOVY-9223: Avoid generating common methods for each groovy class
---
src/main/java/groovy/lang/GroovyObject.java | 17 +++-
src/main/java/groovy/lang/GroovyObjectSupport.java | 16 +---
.../groovy/classgen/ClassCompletionVerifier.java | 7 ++
.../org/codehaus/groovy/classgen/Verifier.java | 93 ----------------------
.../groovy/runtime/ProxyGeneratorAdapter.java | 53 ------------
.../invocation/GroovyObjectInheritanceTest.groovy | 8 +-
src/test/groovy/bugs/Groovy3175_Bug.groovy | 2 +-
.../generated/GroovyObjectGeneratedTest.groovy | 2 +
.../transform/GeneratedAnnotationTest.groovy | 4 +-
9 files changed, 33 insertions(+), 169 deletions(-)
diff --git a/src/main/java/groovy/lang/GroovyObject.java
b/src/main/java/groovy/lang/GroovyObject.java
index 2c10e8c..96e85ed 100644
--- a/src/main/java/groovy/lang/GroovyObject.java
+++ b/src/main/java/groovy/lang/GroovyObject.java
@@ -18,6 +18,8 @@
*/
package groovy.lang;
+import groovy.transform.Internal;
+
/**
* The interface implemented by all Groovy objects.
* <p>
@@ -32,7 +34,10 @@ public interface GroovyObject {
* @param args the arguments to use for the method call
* @return the result of invoking the method
*/
- Object invokeMethod(String name, Object args);
+ @Internal // marked as internal just for backward compatibility, e.g.
AbstractCallSite.createGroovyObjectGetPropertySite will check `isMarkedInternal`
+ default Object invokeMethod(String name, Object args) {
+ return getMetaClass().invokeMethod(this, name, args);
+ }
/**
* Retrieves a property value.
@@ -40,7 +45,10 @@ public interface GroovyObject {
* @param propertyName the name of the property of interest
* @return the given property
*/
- Object getProperty(String propertyName);
+ @Internal // marked as internal just for backward compatibility, e.g.
AbstractCallSite.createGroovyObjectGetPropertySite will check `isMarkedInternal`
+ default Object getProperty(String propertyName) {
+ return getMetaClass().getProperty(this, propertyName);
+ }
/**
* Sets the given property to the new value.
@@ -48,7 +56,10 @@ public interface GroovyObject {
* @param propertyName the name of the property of interest
* @param newValue the new value for the property
*/
- void setProperty(String propertyName, Object newValue);
+ @Internal // marked as internal just for backward compatibility, e.g.
AbstractCallSite.createGroovyObjectGetPropertySite will check `isMarkedInternal`
+ default void setProperty(String propertyName, Object newValue) {
+ getMetaClass().setProperty(this, propertyName, newValue);
+ }
/**
* Returns the metaclass for a given class.
diff --git a/src/main/java/groovy/lang/GroovyObjectSupport.java
b/src/main/java/groovy/lang/GroovyObjectSupport.java
index d08ecd6..57b2552 100644
--- a/src/main/java/groovy/lang/GroovyObjectSupport.java
+++ b/src/main/java/groovy/lang/GroovyObjectSupport.java
@@ -32,18 +32,6 @@ public abstract class GroovyObjectSupport implements
GroovyObject {
this.metaClass = getDefaultMetaClass();
}
- public Object getProperty(String property) {
- return getMetaClass().getProperty(this, property);
- }
-
- public void setProperty(String property, Object newValue) {
- getMetaClass().setProperty(this, property, newValue);
- }
-
- public Object invokeMethod(String name, Object args) {
- return getMetaClass().invokeMethod(this, name, args);
- }
-
public MetaClass getMetaClass() {
return this.metaClass;
}
@@ -51,8 +39,8 @@ public abstract class GroovyObjectSupport implements
GroovyObject {
public void setMetaClass(MetaClass metaClass) {
this.metaClass =
null == metaClass
- ? getDefaultMetaClass()
- : metaClass;
+ ? getDefaultMetaClass()
+ : metaClass;
}
private MetaClass getDefaultMetaClass() {
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index f08cbbd..d380d7d 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -205,6 +205,13 @@ public class ClassCompletionVerifier extends
ClassCodeVisitorSupport {
if (abstractMethods == null) return;
for (MethodNode method : abstractMethods) {
MethodNode sameArgsMethod = node.getMethod(method.getName(),
method.getParameters());
+ if (null == sameArgsMethod) {
+ sameArgsMethod =
ClassHelper.GROOVY_OBJECT_TYPE.getMethod(method.getName(),
method.getParameters());
+ if (null != sameArgsMethod && !sameArgsMethod.isAbstract() &&
method.getReturnType().equals(sameArgsMethod.getReturnType())) {
+ return;
+ }
+ }
+
if (sameArgsMethod==null ||
method.getReturnType().equals(sameArgsMethod.getReturnType())) {
addError("Can't have an abstract method in a non-abstract
class." +
" The " + getDescription(node) + " must be declared
abstract or" +
diff --git a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
index 2230085..c379462 100644
--- a/src/main/java/org/codehaus/groovy/classgen/Verifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/Verifier.java
@@ -135,17 +135,6 @@ public class Verifier implements GroovyClassVisitor,
Opcodes {
// for binary compatibility
public static final String __TIMESTAMP = "__timeStamp";
public static final String __TIMESTAMP__ = "__timeStamp__239_neverHappen";
- private static final Parameter[] INVOKE_METHOD_PARAMS = new Parameter[]{
- new Parameter(ClassHelper.STRING_TYPE, "method"),
- new Parameter(ClassHelper.OBJECT_TYPE, "arguments")
- };
- private static final Parameter[] SET_PROPERTY_PARAMS = new Parameter[]{
- new Parameter(ClassHelper.STRING_TYPE, "property"),
- new Parameter(ClassHelper.OBJECT_TYPE, "value")
- };
- private static final Parameter[] GET_PROPERTY_PARAMS = new Parameter[]{
- new Parameter(ClassHelper.STRING_TYPE, "property")
- };
private static final Parameter[] SET_METACLASS_PARAMS = new Parameter[]{
new Parameter(ClassHelper.METACLASS_TYPE, "mc")
};
@@ -503,88 +492,6 @@ public class Verifier implements GroovyClassVisitor,
Opcodes {
methodNode.addAnnotation(internalAnnotation);
}
}
-
- if (!node.hasMethod("invokeMethod", INVOKE_METHOD_PARAMS)) {
- VariableExpression vMethods = new VariableExpression("method");
- VariableExpression vArguments = new
VariableExpression("arguments");
- VariableScope blockScope = new VariableScope();
- blockScope.putReferencedLocalVariable(vMethods);
- blockScope.putReferencedLocalVariable(vArguments);
-
- MethodNode methodNode = addMethod(node, !shouldAnnotate,
- "invokeMethod",
- ACC_PUBLIC,
- ClassHelper.OBJECT_TYPE, INVOKE_METHOD_PARAMS,
- ClassNode.EMPTY_ARRAY,
- new BytecodeSequence(new BytecodeInstruction() {
- @Override
- public void visit(MethodVisitor mv) {
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEVIRTUAL,
classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitVarInsn(ALOAD, 2);
- mv.visitMethodInsn(INVOKEINTERFACE,
"groovy/lang/MetaClass", "invokeMethod",
"(Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;",
true);
- mv.visitInsn(ARETURN);
- }
- })
- );
- if (shouldAnnotate) {
- methodNode.addAnnotation(generatedAnnotation);
- methodNode.addAnnotation(internalAnnotation);
- }
- }
-
- if (!node.hasMethod("getProperty", GET_PROPERTY_PARAMS)) {
- MethodNode methodNode = addMethod(node, !shouldAnnotate,
- "getProperty",
- ACC_PUBLIC,
- ClassHelper.OBJECT_TYPE,
- GET_PROPERTY_PARAMS,
- ClassNode.EMPTY_ARRAY,
- new BytecodeSequence(new BytecodeInstruction() {
- @Override
- public void visit(MethodVisitor mv) {
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEVIRTUAL,
classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitMethodInsn(INVOKEINTERFACE,
"groovy/lang/MetaClass", "getProperty",
"(Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;", true);
- mv.visitInsn(ARETURN);
- }
- })
- );
- if (shouldAnnotate) {
- methodNode.addAnnotation(generatedAnnotation);
- methodNode.addAnnotation(internalAnnotation);
- }
- }
-
- if (!node.hasMethod("setProperty", SET_PROPERTY_PARAMS)) {
- MethodNode methodNode = addMethod(node, !shouldAnnotate,
- "setProperty",
- ACC_PUBLIC,
- ClassHelper.VOID_TYPE,
- SET_PROPERTY_PARAMS,
- ClassNode.EMPTY_ARRAY,
- new BytecodeSequence(new BytecodeInstruction() {
- @Override
- public void visit(MethodVisitor mv) {
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEVIRTUAL,
classInternalName, "getMetaClass", "()Lgroovy/lang/MetaClass;", false);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitVarInsn(ALOAD, 2);
- mv.visitMethodInsn(INVOKEINTERFACE,
"groovy/lang/MetaClass", "setProperty",
"(Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;)V", true);
- mv.visitInsn(RETURN);
- }
- })
- );
- if (shouldAnnotate) {
- methodNode.addAnnotation(generatedAnnotation);
- methodNode.addAnnotation(internalAnnotation);
- }
- }
}
/**
diff --git
a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
index d3ba083..8f688cb 100644
--- a/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
+++ b/src/main/java/org/codehaus/groovy/runtime/ProxyGeneratorAdapter.java
@@ -443,59 +443,6 @@ public class ProxyGeneratorAdapter extends ClassVisitor
implements Opcodes {
mv.visitEnd();
}
- // getProperty
- {
- mv = super.visitMethod(ACC_PUBLIC, "getProperty",
"(Ljava/lang/String;)Ljava/lang/Object;", null, null);
- mv.visitCode();
- mv.visitIntInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEINTERFACE, "groovy/lang/GroovyObject",
"getMetaClass", "()Lgroovy/lang/MetaClass;", true);
- mv.visitIntInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitMethodInsn(INVOKEINTERFACE, "groovy/lang/MetaClass",
"getProperty", "(Ljava/lang/Object;Ljava/lang/String;)Ljava/lang/Object;",
true);
- mv.visitInsn(ARETURN);
- mv.visitMaxs(3, 2);
- mv.visitEnd();
- }
-
- // setProperty
- {
- mv = super.visitMethod(ACC_PUBLIC, "setProperty",
"(Ljava/lang/String;Ljava/lang/Object;)V", null, null);
- mv.visitCode();
- Label l0 = new Label();
- mv.visitLabel(l0);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEVIRTUAL, proxyName, "getMetaClass",
"()Lgroovy/lang/MetaClass;", false);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitVarInsn(ALOAD, 2);
- mv.visitMethodInsn(INVOKEINTERFACE, "groovy/lang/MetaClass",
"setProperty", "(Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;)V",
true);
- Label l1 = new Label();
- mv.visitLabel(l1);
- mv.visitInsn(RETURN);
- Label l2 = new Label();
- mv.visitLabel(l2);
- mv.visitMaxs(4, 3);
- mv.visitEnd();
- }
-
- // invokeMethod
- {
- mv = super.visitMethod(ACC_PUBLIC, "invokeMethod",
"(Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;", null, null);
- Label l0 = new Label();
- mv.visitLabel(l0);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitMethodInsn(INVOKEVIRTUAL, proxyName, "getMetaClass",
"()Lgroovy/lang/MetaClass;", false);
- mv.visitVarInsn(ALOAD, 0);
- mv.visitVarInsn(ALOAD, 1);
- mv.visitVarInsn(ALOAD, 2);
- mv.visitMethodInsn(INVOKEINTERFACE, "groovy/lang/MetaClass",
"invokeMethod",
"(Ljava/lang/Object;Ljava/lang/String;Ljava/lang/Object;)Ljava/lang/Object;",
true);
- mv.visitInsn(ARETURN);
- Label l1 = new Label();
- mv.visitLabel(l1);
- mv.visitMaxs(4, 3);
- mv.visitEnd();
- }
-
// setMetaClass
{
mv = super.visitMethod(ACC_PUBLIC, "setMetaClass",
"(Lgroovy/lang/MetaClass;)V", null, null);
diff --git a/src/test/gls/invocation/GroovyObjectInheritanceTest.groovy
b/src/test/gls/invocation/GroovyObjectInheritanceTest.groovy
index ef52d8b..1d6e998 100644
--- a/src/test/gls/invocation/GroovyObjectInheritanceTest.groovy
+++ b/src/test/gls/invocation/GroovyObjectInheritanceTest.groovy
@@ -91,7 +91,7 @@ class GroovyObjectInheritanceTest extends
CompilableTestSupport {
"""
}
- void testStandardInhgeritance() {
+ void testStandardInheritance() {
assertScript """
class Foo{}
class Bar extends Foo{}
@@ -105,13 +105,13 @@ class GroovyObjectInheritanceTest extends
CompilableTestSupport {
assert Foo.class.declaredMethods.find{it.name=="setMetaClass"}!=null
assert Bar.class.declaredMethods.find{it.name=="setMetaClass"}==null
- assert Foo.class.declaredMethods.find{it.name=="getProperty"}!=null
+ assert Foo.class.declaredMethods.find{it.name=="getProperty"}==null
assert Bar.class.declaredMethods.find{it.name=="getProperty"}==null
- assert Foo.class.declaredMethods.find{it.name=="setProperty"}!=null
+ assert Foo.class.declaredMethods.find{it.name=="setProperty"}==null
assert Bar.class.declaredMethods.find{it.name=="setProperty"}==null
- assert Foo.class.declaredMethods.find{it.name=="invokeMethod"}!=null
+ assert Foo.class.declaredMethods.find{it.name=="invokeMethod"}==null
assert Bar.class.declaredMethods.find{it.name=="invokeMethod"}==null
"""
}
diff --git a/src/test/groovy/bugs/Groovy3175_Bug.groovy
b/src/test/groovy/bugs/Groovy3175_Bug.groovy
index 07e6c7d..1f1023b 100644
--- a/src/test/groovy/bugs/Groovy3175_Bug.groovy
+++ b/src/test/groovy/bugs/Groovy3175_Bug.groovy
@@ -33,7 +33,7 @@ class Groovy3175_Bug extends GroovyTestCase {
def fields = MyService.getDeclaredFields().grep { !it.synthetic }
assert fields.size() == 2
def methods = MyService.getDeclaredMethods().grep { !it.synthetic }
- assert methods.size() == 9
+ assert methods.size() == 6
methods = methods.grep { !it.getAnnotation(Generated) }
assert methods.size() == 2
"""
diff --git a/src/test/groovy/generated/GroovyObjectGeneratedTest.groovy
b/src/test/groovy/generated/GroovyObjectGeneratedTest.groovy
index 5b9322d..fac892f 100644
--- a/src/test/groovy/generated/GroovyObjectGeneratedTest.groovy
+++ b/src/test/groovy/generated/GroovyObjectGeneratedTest.groovy
@@ -25,6 +25,7 @@ import org.junit.Test
class GroovyObjectGeneratedTest extends AbstractGeneratedAstTestCase {
final Class<?> classUnderTest = parseClass('class MyClass { }')
+ /* invokeMethod, getProperty and setProperty are not generated
@Test
void test_invokeMethod_is_annotated() {
assertMethodIsAnnotated(classUnderTest, 'invokeMethod', String, Object)
@@ -39,6 +40,7 @@ class GroovyObjectGeneratedTest extends
AbstractGeneratedAstTestCase {
void test_setProperty_is_annotated() {
assertMethodIsAnnotated(classUnderTest, 'setProperty', String, Object)
}
+ */
@Test
void test_getMetaClass_is_annotated() {
diff --git
a/src/test/org/codehaus/groovy/transform/GeneratedAnnotationTest.groovy
b/src/test/org/codehaus/groovy/transform/GeneratedAnnotationTest.groovy
index df02934..a4c9c10 100644
--- a/src/test/org/codehaus/groovy/transform/GeneratedAnnotationTest.groovy
+++ b/src/test/org/codehaus/groovy/transform/GeneratedAnnotationTest.groovy
@@ -54,7 +54,9 @@ class GeneratedAnnotationTest extends GroovyShellTestCase {
GroovyObject.declaredMethods.each { m ->
def method = person.class.declaredMethods.find { it.name == m.name
}
- assert
method.annotations*.annotationType().name.contains('groovy.transform.Generated')
+ if (method) {
+ assert
method.annotations*.annotationType().name.contains('groovy.transform.Generated')
+ }
}
}
