groovy-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sun...@apache.org
Subject [groovy] branch master updated: GROOVY-9223: Avoid generating common methods for each groovy class
Date Thu, 15 Aug 2019 12:22:34 GMT
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 <sunlan@apache.org>
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')
+            }
         }
     }
 


Mime
View raw message