This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY-11568 in repository https://gitbox.apache.org/repos/asf/groovy.git
commit 2899ea08efce0407b9d6b78e8d838bc7a8c5e4bd Author: Eric Milles <[email protected]> AuthorDate: Sun Feb 16 20:25:26 2025 -0600 GROOVY-11568: try `invokeMethod(Class,Object,Object[],boolean,boolean)` --- src/main/java/groovy/lang/MetaClassImpl.java | 14 +++-- .../groovy/runtime/metaclass/ClosureMetaClass.java | 2 +- .../v8/IndyGuardsFiltersAndSignatures.java | 66 ++++++++++----------- .../org/codehaus/groovy/vmplugin/v8/Selector.java | 26 +++++++-- src/spec/test/builder/NodeBuilderTest.groovy | 7 ++- src/test/groovy/bugs/Groovy11568.groovy | 67 ++++++++++++++++++++++ src/test/groovy/lang/MixinTest.groovy | 3 +- 7 files changed, 139 insertions(+), 46 deletions(-) diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java index 6908e3b6f2..38ac1bc4c1 100644 --- a/src/main/java/groovy/lang/MetaClassImpl.java +++ b/src/main/java/groovy/lang/MetaClassImpl.java @@ -671,7 +671,10 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { private Object getMethods(final Class<?> sender, final String name, final boolean isCallToSuper) { Object answer; - final MetaMethodIndex.Cache entry = metaMethodIndex.getMethods(sender, name); + var entry = metaMethodIndex.getMethods( sender , name); + if (entry == null) { + entry = metaMethodIndex.getMethods(theClass, name); + } if (entry == null) { answer = FastArray.EMPTY_LIST; } else if (isCallToSuper) { @@ -1086,7 +1089,7 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { throw new NullPointerException("Cannot invoke method: " + methodName + " on null object"); } - final Object[] arguments = originalArguments == null ? EMPTY_ARGUMENTS : originalArguments; + final Object[] arguments = Optional.ofNullable(originalArguments).orElse(EMPTY_ARGUMENTS); MetaMethod method = getMetaMethod(sender, object, methodName, isCallToSuper, arguments); @@ -1237,9 +1240,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { method = getMethodWithCaching(sender, methodName, arguments, isCallToSuper); } MetaClassHelper.unwrap(arguments); - - if (method == null) + if (method == null) { method = tryListParamMetaMethod(sender, methodName, isCallToSuper, arguments); + } return method; } @@ -1330,6 +1333,9 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass { : (isCallToSuper && e.methodsForSuper == null)) { // allow "super.name()" to find DGM if class declares method "name" e = metaMethodIndex.getMethods(sender.getSuperclass(), methodName); } + if (e == null) { + e = metaMethodIndex.getMethods(theClass, methodName); // GROOVY-11568 + } if (e == null) { return null; } else if (isCallToSuper) { diff --git a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java index 2d7332d332..479c6b20e9 100644 --- a/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java +++ b/src/main/java/org/codehaus/groovy/runtime/metaclass/ClosureMetaClass.java @@ -365,7 +365,7 @@ public final class ClosureMetaClass extends MetaClassImpl { } Class thisType = sender; while (GeneratedClosure.class.isAssignableFrom(thisType)) thisType = thisType.getEnclosingClass(); - if (thisType != sender && thisType != object.getClass() && thisType.isInstance(object)) { // GROOVY-2433, GROOVY-11128 + if (thisType != object.getClass() && thisType.isInstance(object)) { // GROOVY-2433, GROOVY-11128 try { return ((GroovyObject) object).getMetaClass().invokeMethod(thisType, object, methodName, arguments, false, true); } catch (GroovyRuntimeException gre) { diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java index db2d6307bc..a992f38c21 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java @@ -56,15 +56,14 @@ public class IndyGuardsFiltersAndSignatures { private static final MethodType OBJECT_FILTER = MethodType.methodType(Object.class, Object.class), - OBJECT_GUARD = MethodType.methodType(boolean.class, Object.class), - INVOKER = MethodType.methodType(Object.class, Object.class, String.class, Object[].class); + OBJECT_GUARD = MethodType.methodType(boolean.class, Object.class); protected static final MethodHandle SAME_CLASS, SAME_CLASSES, SAME_MC, IS_NULL, NON_NULL, UNWRAP_METHOD, UNWRAP_EXCEPTION, HAS_CATEGORY_IN_CURRENT_THREAD_GUARD, - META_METHOD_INVOKER, GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY, - META_CLASS_INVOKE_STATIC_METHOD, + GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY,META_METHOD_INVOKER, + META_CLASS_INVOKE_METHOD, META_CLASS_INVOKE_STATIC_METHOD, BEAN_CONSTRUCTOR_PROPERTY_SETTER, META_PROPERTY_GETTER, SLOW_META_CLASS_FIND, @@ -78,35 +77,36 @@ public class IndyGuardsFiltersAndSignatures { static { try { - SAME_CLASS = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClass", MethodType.methodType(boolean.class, Class.class, Object.class)); - SAME_CLASSES = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClasses", MethodType.methodType(boolean.class, Class[].class, Object[].class)); - SAME_MC = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", MethodType.methodType(boolean.class, MetaClass.class, Object.class)); - IS_NULL = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isNull", OBJECT_GUARD); - NON_NULL = LOOKUP.findStatic(Objects.class, "nonNull", MethodType.methodType(boolean.class, Object.class)); - UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER); - UNWRAP_EXCEPTION = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", MethodType.methodType(Object.class, GroovyRuntimeException.class)); - HAS_CATEGORY_IN_CURRENT_THREAD_GUARD = LOOKUP.findStatic(GroovyCategorySupport.class, "hasCategoryInCurrentThread", MethodType.methodType(boolean.class)); - META_METHOD_INVOKER = LOOKUP.findVirtual(MetaMethod.class, "doMethodInvoke", MethodType.methodType(Object.class, Object.class, Object[].class)); - GROOVY_OBJECT_INVOKER = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "invokeGroovyObjectInvoker", INVOKER.insertParameterTypes(0, MissingMethodException.class)); - GROOVY_OBJECT_GET_PROPERTY = LOOKUP.findVirtual(GroovyObject.class, "getProperty", MethodType.methodType(Object.class, String.class)); - META_CLASS_INVOKE_STATIC_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeStaticMethod", INVOKER); - BEAN_CONSTRUCTOR_PROPERTY_SETTER = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "setBeanProperties", MethodType.methodType(Object.class, MetaClass.class, Object.class, Map.class)); - META_PROPERTY_GETTER = LOOKUP.findVirtual(MetaProperty.class, "getProperty", OBJECT_FILTER); - SLOW_META_CLASS_FIND = LOOKUP.findStatic(InvokerHelper.class, "getMetaClass", MethodType.methodType(MetaClass.class, Object.class)); - MOP_GET = LOOKUP.findVirtual(MetaObjectProtocol.class, "getProperty", MethodType.methodType(Object.class, Object.class, String.class)); - MOP_INVOKE_CONSTRUCTOR = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeConstructor", MethodType.methodType(Object.class, Object[].class)); - MOP_INVOKE_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeMethod", INVOKER); - INTERCEPTABLE_INVOKER = LOOKUP.findVirtual(GroovyObject.class, "invokeMethod", MethodType.methodType(Object.class, String.class, Object.class)); - - BOOLEAN_IDENTITY = MethodHandles.identity(Boolean.class); - CLASS_FOR_NAME = LOOKUP.findStatic(Class.class, "forName", MethodType.methodType(Class.class, String.class, boolean.class, ClassLoader.class)); - DTT_CAST_TO_TYPE = LOOKUP.findStatic(DefaultTypeTransformation.class, "castToType", MethodType.methodType(Object.class, Object.class, Class.class)); - SAM_CONVERSION = LOOKUP.findStatic(CachedSAMClass.class, "coerceToSAM", MethodType.methodType(Object.class, Closure.class, Method.class, Class.class)); - HASHSET_CONSTRUCTOR = LOOKUP.findConstructor(HashSet.class, MethodType.methodType(void.class, Collection.class)); - ARRAYLIST_CONSTRUCTOR = LOOKUP.findConstructor(ArrayList.class, MethodType.methodType(void.class, Collection.class)); - GROOVY_CAST_EXCEPTION = LOOKUP.findConstructor(GroovyCastException.class, MethodType.methodType(void.class, Object.class, Class.class)); - - EQUALS = LOOKUP.findVirtual(Object.class, "equals", OBJECT_GUARD); + SAME_CLASS = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "sameClass", MethodType.methodType(boolean.class, Class.class, Object.class)); + SAME_CLASSES = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "sameClasses", MethodType.methodType(boolean.class, Class[].class, Object[].class)); + SAME_MC = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", MethodType.methodType(boolean.class, MetaClass.class, Object.class)); + IS_NULL = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "isNull", OBJECT_GUARD); + NON_NULL = LOOKUP.findStatic (Objects.class, "nonNull", MethodType.methodType(boolean.class, Object.class)); + UNWRAP_METHOD = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER); + UNWRAP_EXCEPTION = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "unwrap", MethodType.methodType(Object.class, GroovyRuntimeException.class)); + HAS_CATEGORY_IN_CURRENT_THREAD_GUARD = LOOKUP.findStatic (GroovyCategorySupport.class, "hasCategoryInCurrentThread", MethodType.methodType(boolean.class)); + GROOVY_OBJECT_INVOKER = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "invokeGroovyObjectInvoker", MethodType.methodType(Object.class, MissingMethodException.class, Object.class, String.class, Object[].class)); + GROOVY_OBJECT_GET_PROPERTY = LOOKUP.findVirtual(GroovyObject.class, "getProperty", MethodType.methodType(Object.class, String.class)); + META_METHOD_INVOKER = LOOKUP.findVirtual(MetaMethod.class, "doMethodInvoke", MethodType.methodType(Object.class, Object.class, Object[].class)); + META_CLASS_INVOKE_METHOD = LOOKUP.findVirtual(MetaClass.class, "invokeMethod", MethodType.methodType(Object.class, Class.class, Object.class, String.class, Object[].class, boolean.class, boolean.class)); + META_CLASS_INVOKE_STATIC_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeStaticMethod", MethodType.methodType(Object.class, Object.class, String.class, Object[].class)); + BEAN_CONSTRUCTOR_PROPERTY_SETTER = LOOKUP.findStatic (IndyGuardsFiltersAndSignatures.class, "setBeanProperties", MethodType.methodType(Object.class, MetaClass.class, Object.class, Map.class)); + META_PROPERTY_GETTER = LOOKUP.findVirtual(MetaProperty.class, "getProperty", OBJECT_FILTER); + SLOW_META_CLASS_FIND = LOOKUP.findStatic (InvokerHelper.class, "getMetaClass", MethodType.methodType(MetaClass.class, Object.class)); + MOP_GET = LOOKUP.findVirtual(MetaObjectProtocol.class, "getProperty", MethodType.methodType(Object.class, Object.class, String.class)); + MOP_INVOKE_CONSTRUCTOR = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeConstructor", MethodType.methodType(Object.class, Object[].class)); + MOP_INVOKE_METHOD = LOOKUP.findVirtual(MetaObjectProtocol.class, "invokeMethod", MethodType.methodType(Object.class, Object.class, String.class, Object[].class)); + INTERCEPTABLE_INVOKER = LOOKUP.findVirtual(GroovyObject.class, "invokeMethod", MethodType.methodType(Object.class, String.class, Object.class)); + + BOOLEAN_IDENTITY = MethodHandles.identity(Boolean.class); + CLASS_FOR_NAME = LOOKUP.findStatic(Class.class, "forName", MethodType.methodType(Class.class, String.class, boolean.class, ClassLoader.class)); + DTT_CAST_TO_TYPE = LOOKUP.findStatic(DefaultTypeTransformation.class, "castToType", MethodType.methodType(Object.class, Object.class, Class.class)); + SAM_CONVERSION = LOOKUP.findStatic(CachedSAMClass.class, "coerceToSAM", MethodType.methodType(Object.class, Closure.class, Method.class, Class.class)); + HASHSET_CONSTRUCTOR = LOOKUP.findConstructor(HashSet.class, MethodType.methodType(void.class, Collection.class)); + ARRAYLIST_CONSTRUCTOR = LOOKUP.findConstructor(ArrayList.class, MethodType.methodType(void.class, Collection.class)); + GROOVY_CAST_EXCEPTION = LOOKUP.findConstructor(GroovyCastException.class, MethodType.methodType(void.class, Object.class, Class.class)); + + EQUALS = LOOKUP.findVirtual(Object.class, "equals", OBJECT_GUARD); } catch (Exception e) { throw new GroovyBugError(e); } diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java index b9da452a7e..be586fdff0 100644 --- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java +++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java @@ -79,6 +79,7 @@ import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.HAS import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.HAS_CATEGORY_IN_CURRENT_THREAD_GUARD; import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.INTERCEPTABLE_INVOKER; import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.IS_NULL; +import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_CLASS_INVOKE_METHOD; import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_CLASS_INVOKE_STATIC_METHOD; import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_METHOD_INVOKER; import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.META_PROPERTY_GETTER; @@ -727,6 +728,13 @@ public abstract class Selector { return targetType; } + private Method metaClassMethod(String name, Class<?>... signature) { + try { return mc.getClass().getMethod(name, signature); + } catch (ReflectiveOperationException e) { + throw new GroovyBugError(e); + } + } + /** * Creates a MethodHandle, which will use the metaclass path. * This method is called only if no handle has been created before. This @@ -741,9 +749,19 @@ public abstract class Selector { handle = META_CLASS_INVOKE_STATIC_METHOD.bindTo(mc); if (LOG_ENABLED) LOG.info("use invokeStaticMethod with bound meta class"); } else { - handle = MOP_INVOKE_METHOD.bindTo(mc); - if (LOG_ENABLED) LOG.info("use invokeMethod with bound meta class"); - + if (standardMetaClass + || metaClassMethod("invokeMethod", Object.class, String.class, Object[].class).getDeclaringClass().isAssignableFrom( + metaClassMethod("invokeMethod", Class.class, Object.class, String.class, Object[].class, boolean.class, boolean.class).getDeclaringClass()) + ) { + // GROOVY-11568: use MetaClass#invokeMethod(Class,Object,Object[],boolean,boolean) + handle = META_CLASS_INVOKE_METHOD; + handle = handle.bindTo(mc).bindTo(sender); + handle = MethodHandles.insertArguments(handle, 3, Boolean.FALSE, Boolean.FALSE); + if (LOG_ENABLED) LOG.info("use invokeMethod with bound meta class and sender class"); + } else { + handle = MOP_INVOKE_METHOD.bindTo(mc); + if (LOG_ENABLED) LOG.info("use invokeMethod with bound meta class"); + } if (receiver instanceof GroovyObject) { // if the metaclass call fails we may still want to fall back to call // GroovyObject#invokeMethod if the receiver is a GroovyObject @@ -1011,7 +1029,7 @@ public abstract class Selector { } /** - * Sets a handle to call {@link GroovyInterceptable#invokeMethod(String, Object)} + * Sets a handle to call {@link GroovyObject#invokeMethod(String,Object)} */ public boolean setInterceptor() { if (!(args[0] instanceof GroovyInterceptable)) return false; diff --git a/src/spec/test/builder/NodeBuilderTest.groovy b/src/spec/test/builder/NodeBuilderTest.groovy index 9871e1e393..82034d0d89 100644 --- a/src/spec/test/builder/NodeBuilderTest.groovy +++ b/src/spec/test/builder/NodeBuilderTest.groovy @@ -18,10 +18,11 @@ */ package builder -import groovy.test.GroovyTestCase +import org.junit.jupiter.api.Test -class NodeBuilderTest extends GroovyTestCase { +final class NodeBuilderTest { + @Test void testNodeBuilder() { // tag::node_builder_example[] def nodeBuilder = new NodeBuilder() @@ -41,6 +42,7 @@ class NodeBuilderTest extends GroovyTestCase { } // GROOVY-7044 + @Test void testNodeCloning() { def node = new NodeBuilder().a { b() @@ -53,6 +55,7 @@ class NodeBuilderTest extends GroovyTestCase { } // GROOVY-7044 + @Test void testNodeCloningWithAttributes() { def node = new NodeBuilder().a(foo: 'bar') { b() diff --git a/src/test/groovy/bugs/Groovy11568.groovy b/src/test/groovy/bugs/Groovy11568.groovy new file mode 100644 index 0000000000..a9b9405c5e --- /dev/null +++ b/src/test/groovy/bugs/Groovy11568.groovy @@ -0,0 +1,67 @@ +/* + * 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 + +import org.junit.jupiter.api.Test + +import static groovy.test.GroovyAssert.assertScript + +final class Groovy11568 { + @Test + void testPrivateMethodWithCustomMetaClass() { + assertScript ''' + class CMC extends MetaClassImpl { + CMC(Class theClass) { + super(theClass) + } + @Override + Object invokeMethod(Object object, String methodName, Object arguments) { + super.invokeMethod(object, methodName, arguments) + } + @Override + Object invokeMethod(Object object, String methodName, Object[] originalArguments) { + super.invokeMethod(object, methodName, originalArguments) + } + @Override + Object invokeMethod(Class sender, Object object, String methodName, Object[] arguments, boolean isCallToSuper, boolean fromInsideClass) { + super.invokeMethod(sender, object, methodName, arguments, isCallToSuper, fromInsideClass) + } + } + + class C { + def publicMethod() { + privateMethod() + } + private privateMethod() { + 'C#privateMethod()' + } + } + + class D extends C { + } + + def cmc = new CMC(D.class) + cmc.initialize() + def obj = new D() + obj.setMetaClass(cmc) + def str = obj.publicMethod() + assert str == 'C#privateMethod()' + ''' + } +} diff --git a/src/test/groovy/lang/MixinTest.groovy b/src/test/groovy/lang/MixinTest.groovy index 8d27dc51d9..3694da2252 100644 --- a/src/test/groovy/lang/MixinTest.groovy +++ b/src/test/groovy/lang/MixinTest.groovy @@ -306,9 +306,8 @@ class MixinTest extends GroovyTestCase { u << 2 assertEquals 6, u.size() - assertEquals 6, ((List) u).size() assertEquals 6, ((Collection) u).size() - assertEquals 2, u.mixedIn[Set].size() + assertEquals 6, ((List) u).size() assertEquals 2, ((Set) u).size() }
