You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@groovy.apache.org by em...@apache.org on 2021/12/01 21:37:59 UTC
[groovy] branch master updated: GROOVY-4922, GROOVY-6001: apply check to method overload situation
This is an automated email from the ASF dual-hosted git repository.
emilles 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 b5c7e1c GROOVY-4922, GROOVY-6001: apply check to method overload situation
b5c7e1c is described below
commit b5c7e1ce9e1838eef5a7eb9b6373587afa9ca24e
Author: Eric Milles <er...@thomsonreuters.com>
AuthorDate: Wed Dec 1 15:37:47 2021 -0600
GROOVY-4922, GROOVY-6001: apply check to method overload situation
---
src/main/java/groovy/lang/MetaClassImpl.java | 208 ++++++++-------------
.../groovy/classgen/asm/InvocationWriter.java | 68 +++----
2 files changed, 106 insertions(+), 170 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java b/src/main/java/groovy/lang/MetaClassImpl.java
index fcc55c5..b16fe8c 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -355,26 +355,27 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return isGroovyObject;
}
- /**
- * Fills the method index
- */
private void fillMethodIndex() {
mainClassMethodHeader = metaMethodIndex.getHeader(theClass);
- LinkedList<CachedClass> superClasses = getSuperClasses();
- CachedClass firstGroovySuper = calcFirstGroovySuperClass(superClasses);
- Set<CachedClass> interfaces = theCachedClass.getInterfaces();
- addInterfaceMethods(interfaces);
+ Set<CachedClass> interfaces = theCachedClass.getInterfaces();
+ List<CachedClass> superClasses = getSuperClasses(); // in reverse order
+ CachedClass firstGroovySuper = calcFirstGroovySuperClass(superClasses);
+
+ for (CachedClass c : interfaces) {
+ for (CachedMethod m : c.getMethods()) {
+ addMetaMethodToIndex(m, mainClassMethodHeader);
+ }
+ }
populateMethods(superClasses, firstGroovySuper);
inheritInterfaceNewMetaMethods(interfaces);
- if (isGroovyObject) {
- metaMethodIndex.copyMethodsToSuper();
+ if (isGroovyObject) {
+ metaMethodIndex.copyMethodsToSuper(); // methods --> methodsForSuper
connectMultimethods(superClasses, firstGroovySuper);
removeMultimethodsOverloadedWithPrivateMethods();
-
replaceWithMOPCalls(theCachedClass.mopMethods);
}
}
@@ -435,20 +436,11 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return myNewMetaMethods;
}
- private void addInterfaceMethods(final Set<CachedClass> interfaces) {
- MetaMethodIndex.Header header = metaMethodIndex.getHeader(theClass);
- for (CachedClass c : interfaces) {
- for (CachedMethod m : c.getMethods()) {
- addMetaMethodToIndex(m, header);
- }
- }
- }
-
protected LinkedList<CachedClass> getSuperClasses() {
LinkedList<CachedClass> superClasses = new LinkedList<>();
if (theClass.isInterface()) {
- superClasses.addFirst(ReflectionCache.OBJECT_CLASS);
+ superClasses.add(ReflectionCache.OBJECT_CLASS);
} else {
for (CachedClass c = theCachedClass; c != null; c = c.getCachedSuperClass()) {
superClasses.addFirst(c);
@@ -511,68 +503,57 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
private void replaceWithMOPCalls(final CachedMethod[] mopMethods) {
- // no MOP methods if not a child of GroovyObject
- if (!isGroovyObject) return;
+ if (mopMethods == null || mopMethods.length == 0) return;
class MOPIter extends MethodIndexAction {
boolean useThis;
@Override
public void methodNameAction(final Class<?> clazz, final MetaMethodIndex.Entry e) {
- if (useThis) {
- if (e.methods == null)
- return;
-
- if (e.methods instanceof FastArray) {
- FastArray methods = (FastArray) e.methods;
- processFastArray(methods);
- } else {
- MetaMethod method = (MetaMethod) e.methods;
- if (method instanceof NewMetaMethod)
- return;
- if (useThis ^ Modifier.isPrivate(method.getModifiers())) return;
- String mopName = method.getMopName();
- int index = Arrays.binarySearch(mopMethods, mopName, CachedClass.CachedMethodComparatorWithString.INSTANCE);
- if (index >= 0) {
- int matchingMethod = findMatchingMethod(method, mopName, index, mopMethods);
- if (matchingMethod != -1) {
- e.methods = mopMethods[matchingMethod];
- }
+ Object arrayOrMethod = (useThis ? e.methods : e.methodsForSuper);
+ if (arrayOrMethod instanceof FastArray) {
+ FastArray methods = (FastArray) arrayOrMethod;
+ for (int i = 0, n = methods.size(); i < n; i += 1) {
+ int matchingMethod = mopArrayIndex((MetaMethod) methods.get(i));
+ if (matchingMethod >= 0) {
+ methods.set(i, mopMethods[matchingMethod]);
}
}
- } else {
- if (e.methodsForSuper == null)
- return;
-
- if (e.methodsForSuper instanceof FastArray) {
- FastArray methods = (FastArray) e.methodsForSuper;
- processFastArray(methods);
- } else {
- MetaMethod method = (MetaMethod) e.methodsForSuper;
- if (method instanceof NewMetaMethod)
- return;
- if (useThis ^ Modifier.isPrivate(method.getModifiers())) return;
- String mopName = method.getMopName();
- // GROOVY-4922: Due to a numbering scheme change, we must find the super$X$method which exists
- // with the highest number. If we don't, no method may be found, leading to a stack overflow
- String[] decomposedMopName = decomposeMopName(mopName);
- int distance = Integer.parseInt(decomposedMopName[1]);
- while (distance > 0) {
- String fixedMopName = decomposedMopName[0] + distance + decomposedMopName[2];
- int index = Arrays.binarySearch(mopMethods, fixedMopName, CachedClass.CachedMethodComparatorWithString.INSTANCE);
- if (index >= 0) {
- int matchingMethod = findMatchingMethod(method, fixedMopName, index, mopMethods);
- if (matchingMethod != -1) {
- e.methodsForSuper = mopMethods[matchingMethod];
- distance = 0;
- }
- }
- distance -= 1;
- }
+ } else if (arrayOrMethod != null) {
+ int matchingMethod = mopArrayIndex((MetaMethod) arrayOrMethod);
+ if (matchingMethod >= 0) {
+ if (useThis) e.methods = mopMethods[matchingMethod];
+ else e.methodsForSuper = mopMethods[matchingMethod];
}
}
}
+ private int mopArrayIndex(final MetaMethod method) {
+ if (method instanceof NewMetaMethod) return -1;
+ if (useThis ^ Modifier.isPrivate(method.getModifiers())) return -1;
+
+ String mopName = method.getMopName();
+ if (useThis) {
+ return mopArrayIndex(method, mopName);
+ }
+ // GROOVY-4922: Due to a numbering scheme change, find the super$number$methodName with
+ // the highest value. If we don't, no method may be found, leading to a stack overflow!
+ String[] tokens = decomposeMopName(mopName);
+ int distance = Integer.parseInt(tokens[1]);
+ while (distance > 0) {
+ mopName = tokens[0] + distance + tokens[2];
+ int index = mopArrayIndex(method, mopName);
+ if (index >= 0) return index;
+ distance -= 1;
+ }
+ return -1;
+ }
+
+ private int mopArrayIndex(final MetaMethod method, final String mopName) {
+ int index = Arrays.binarySearch(mopMethods, mopName, CachedClass.CachedMethodComparatorWithString.INSTANCE);
+ return index < 0 ? -1 : findMatchingMethod(method, mopName, index, mopMethods);
+ }
+
private String[] decomposeMopName(final String mopName) {
int idx = mopName.indexOf('$');
if (idx > 0) {
@@ -587,25 +568,6 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
return new String[]{"", "0", mopName};
}
-
- private void processFastArray(final FastArray methods) {
- final int len = methods.size();
- final Object[] data = methods.getArray();
- for (int i = 0; i != len; i += 1) {
- MetaMethod method = (MetaMethod) data[i];
- if (method instanceof NewMetaMethod) continue;
- boolean isPrivate = Modifier.isPrivate(method.getModifiers());
- if (useThis ^ isPrivate) continue;
- String mopName = method.getMopName();
- int index = Arrays.binarySearch(mopMethods, mopName, CachedClass.CachedMethodComparatorWithString.INSTANCE);
- if (index >= 0) {
- int matchingMethod = findMatchingMethod(method, mopName, index, mopMethods);
- if (matchingMethod != -1) {
- methods.set(i, mopMethods[matchingMethod]);
- }
- }
- }
- }
}
MOPIter iter = new MOPIter();
@@ -1532,14 +1494,12 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
return isCallToSuper ? getSuperMethodWithCaching(arguments, e) : getNormalMethodWithCaching(arguments, e);
}
- private static boolean sameClasses(Class[] params, Class[] arguments) {
+ private static boolean sameClasses(final Class[] params, final Class[] arguments) {
// we do here a null check because the params field might not have been set yet
- if (params == null) return false;
-
- if (params.length != arguments.length)
+ if (params == null || params.length != arguments.length)
return false;
- for (int i = params.length - 1; i >= 0; i--) {
+ for (int i = params.length - 1; i >= 0; i -= 1) {
Object arg = arguments[i];
if (arg != null) {
if (params[i] != arguments[i]) return false;
@@ -1552,70 +1512,56 @@ public class MetaClassImpl implements MetaClass, MutableMetaClass {
}
// This method should be called by CallSite only
- private MetaMethod getMethodWithCachingInternal(Class sender, CallSite site, Class[] params) {
+ private MetaMethod getMethodWithCachingInternal(final Class sender, final CallSite site, final Class<?>[] params) {
if (GroovyCategorySupport.hasCategoryInCurrentThread())
return getMethodWithoutCaching(sender, site.getName(), params, false);
- final MetaMethodIndex.Entry e = metaMethodIndex.getMethods(sender, site.getName());
- if (e == null) {
+ MetaMethodIndex.Entry e = metaMethodIndex.getMethods(sender, site.getName());
+ if (e == null || e.methods == null)
return null;
- }
- MetaMethodIndex.CacheEntry cacheEntry;
- final Object methods = e.methods;
- if (methods == null)
- return null;
-
- cacheEntry = e.cachedMethod;
+ MetaMethodIndex.CacheEntry cacheEntry = e.cachedMethod;
if (cacheEntry != null && (sameClasses(cacheEntry.params, params))) {
return cacheEntry.method;
}
- cacheEntry = new MetaMethodIndex.CacheEntry(params, (MetaMethod) chooseMethod(e.name, methods, params));
- e.cachedMethod = cacheEntry;
+ cacheEntry = e.cachedMethod = new MetaMethodIndex.CacheEntry(params, (MetaMethod) chooseMethod(e.name, e.methods, params));
+
return cacheEntry.method;
}
- private MetaMethod getSuperMethodWithCaching(Object[] arguments, MetaMethodIndex.Entry e) {
- MetaMethodIndex.CacheEntry cacheEntry;
+ private MetaMethod getSuperMethodWithCaching(final Object[] arguments, final MetaMethodIndex.Entry e) {theClass.getSuperclass();
if (e.methodsForSuper == null)
return null;
- cacheEntry = e.cachedMethodForSuper;
+ MetaMethodIndex.CacheEntry cacheEntry = e.cachedMethodForSuper;
- if (cacheEntry != null &&
- MetaClassHelper.sameClasses(cacheEntry.params, arguments, e.methodsForSuper instanceof MetaMethod)) {
- MetaMethod method = cacheEntry.method;
- if (method != null) return method;
+ if (cacheEntry != null && cacheEntry.method != null
+ && MetaClassHelper.sameClasses(cacheEntry.params, arguments, e.methodsForSuper instanceof MetaMethod)) {
+ return cacheEntry.method;
}
- final Class[] classes = MetaClassHelper.convertToTypeArray(arguments);
- MetaMethod method = (MetaMethod) chooseMethod(e.name, e.methodsForSuper, classes);
- cacheEntry = new MetaMethodIndex.CacheEntry(classes, method.isAbstract() ? null : method);
-
- e.cachedMethodForSuper = cacheEntry;
+ Class<?>[] types = MetaClassHelper.convertToTypeArray(arguments);
+ MetaMethod method = (MetaMethod) chooseMethod(e.name, e.methodsForSuper, types);
+ cacheEntry = e.cachedMethodForSuper = new MetaMethodIndex.CacheEntry(types, method.isAbstract() ? null : method);
return cacheEntry.method;
}
- private MetaMethod getNormalMethodWithCaching(Object[] arguments, MetaMethodIndex.Entry e) {
- MetaMethodIndex.CacheEntry cacheEntry;
- final Object methods = e.methods;
- if (methods == null)
+ private MetaMethod getNormalMethodWithCaching(final Object[] arguments, final MetaMethodIndex.Entry e) {
+ if (e.methods == null)
return null;
- cacheEntry = e.cachedMethod;
+ MetaMethodIndex.CacheEntry cacheEntry = e.cachedMethod;
- if (cacheEntry != null &&
- MetaClassHelper.sameClasses(cacheEntry.params, arguments, methods instanceof MetaMethod)) {
- MetaMethod method = cacheEntry.method;
- if (method != null) return method;
+ if (cacheEntry != null && cacheEntry.method != null
+ && MetaClassHelper.sameClasses(cacheEntry.params, arguments, e.methods instanceof MetaMethod)) {
+ return cacheEntry.method;
}
- final Class[] classes = MetaClassHelper.convertToTypeArray(arguments);
- cacheEntry = new MetaMethodIndex.CacheEntry(classes, (MetaMethod) chooseMethod(e.name, methods, classes));
-
- e.cachedMethod = cacheEntry;
+ Class<?>[] types = MetaClassHelper.convertToTypeArray(arguments);
+ MetaMethod method = (MetaMethod) chooseMethod(e.name, e.methods, types);
+ cacheEntry = e.cachedMethod = new MetaMethodIndex.CacheEntry(types, method);
return cacheEntry.method;
}
diff --git a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
index c8af4b5..4757f8f 100644
--- a/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
+++ b/src/main/java/org/codehaus/groovy/classgen/asm/InvocationWriter.java
@@ -124,6 +124,16 @@ public class InvocationWriter {
makeCall(origin, new ClassExpression(sender), receiver, message, arguments, adapter, safe, spreadSafe, implicitThis);
}
+ protected void makeCall(final Expression origin, final ClassExpression sender, final Expression receiver, final Expression message, final Expression arguments, final MethodCallerMultiAdapter adapter, final boolean safe, final boolean spreadSafe, final boolean implicitThis) {
+ boolean containsSpreadExpression = AsmClassGenerator.containsSpreadExpression(arguments);
+ // direct invocation path
+ if (!makeDirectCall(origin, receiver, message, arguments, adapter, implicitThis, containsSpreadExpression))
+ // call site or indy path
+ if (!makeCachedCall(origin, sender, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis, containsSpreadExpression))
+ // ScriptBytecodeAdapter path
+ makeUncachedCall(origin, sender, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis, containsSpreadExpression);
+ }
+
protected boolean writeDirectMethodCall(final MethodNode target, final boolean implicitThis, final Expression receiver, final TupleExpression args) {
if (target == null) return false;
ClassNode declaringClass = target.getDeclaringClass();
@@ -279,33 +289,27 @@ public class InvocationWriter {
protected boolean makeDirectCall(Expression origin, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean implicitThis, boolean containsSpreadExpression) {
if (makeClassForNameCall(origin, receiver, message, arguments)) return true;
-
- // optimization path
- boolean fittingAdapter = adapter == invokeMethodOnCurrent || adapter == invokeStaticMethod;
- if (fittingAdapter && controller.optimizeForInt && controller.isFastPath()) {
+ if (controller.optimizeForInt && controller.isFastPath() // optimization path
+ && adapter == invokeMethodOnCurrent || adapter == invokeStaticMethod) {
String methodName = getMethodName(message);
if (methodName != null) {
- TupleExpression args;
- if (arguments instanceof TupleExpression) {
- args = (TupleExpression) arguments;
- } else {
- args = new TupleExpression(receiver);
+ OptimizingStatementWriter.StatementMeta meta = origin.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
+ if (meta != null && meta.target != null) {
+ TupleExpression args;
+ if (arguments instanceof TupleExpression) {
+ args = (TupleExpression) arguments;
+ } else {
+ args = new TupleExpression(receiver);
+ }
+ if (writeDirectMethodCall(meta.target, true, null, args)) return true;
}
-
- OptimizingStatementWriter.StatementMeta meta = null;
- if (origin != null) meta = origin.getNodeMetaData(OptimizingStatementWriter.StatementMeta.class);
- MethodNode mn = null;
- if (meta != null) mn = meta.target;
-
- if (writeDirectMethodCall(mn, true, null, args)) return true;
}
}
-
if (containsSpreadExpression) return false;
if (origin instanceof MethodCallExpression) {
MethodCallExpression mce = (MethodCallExpression) origin;
- MethodNode target = mce.getMethodTarget();
- return writeDirectMethodCall(target, implicitThis, receiver, makeArgumentList(arguments));
+ if (mce.getMethodTarget() != null)
+ return writeDirectMethodCall(mce.getMethodTarget(), implicitThis, receiver, makeArgumentList(arguments));
}
return false;
}
@@ -323,8 +327,8 @@ public class InvocationWriter {
}
protected void makeUncachedCall(Expression origin, ClassExpression sender, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean safe, boolean spreadSafe, boolean implicitThis, boolean containsSpreadExpression) {
- OperandStack operandStack = controller.getOperandStack();
CompileStack compileStack = controller.getCompileStack();
+ OperandStack operandStack = controller.getOperandStack();
AsmClassGenerator acg = controller.getAcg();
// ensure VariableArguments are read, not stored
@@ -339,7 +343,7 @@ public class InvocationWriter {
String methodName = getMethodName(message);
if (adapter == invokeMethodOnSuper && methodName != null) {
- controller.getSuperMethodNames().add(methodName);
+ controller.getSuperMethodNames().add(methodName); // for MOP method
}
// receiver
@@ -383,19 +387,6 @@ public class InvocationWriter {
operandStack.replace(ClassHelper.OBJECT_TYPE, operandsToRemove);
}
- protected void makeCall(Expression origin, ClassExpression sender, Expression receiver, Expression message, Expression arguments, MethodCallerMultiAdapter adapter, boolean safe, boolean spreadSafe, boolean implicitThis) {
- // direct method call paths
- boolean containsSpreadExpression = AsmClassGenerator.containsSpreadExpression(arguments);
-
- if (makeDirectCall(origin, receiver, message, arguments, adapter, implicitThis, containsSpreadExpression)) return;
-
- // normal path
- if (makeCachedCall(origin, sender, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis, containsSpreadExpression)) return;
-
- // path through ScriptBytecodeAdapter
- makeUncachedCall(origin, sender, receiver, message, arguments, adapter, safe, spreadSafe, implicitThis, containsSpreadExpression);
- }
-
/**
* if Class.forName(x) is recognized, make a direct method call
*/
@@ -445,24 +436,23 @@ public class InvocationWriter {
public void writeInvokeMethod(MethodCallExpression call) {
if (isClosureCall(call)) {
- // let's invoke the closure method
invokeClosure(call.getArguments(), call.getMethodAsString());
} else {
if (isFunctionInterfaceCall(call)) {
call = transformToRealMethodCall(call);
}
+ Expression receiver = call.getObjectExpression();
MethodCallerMultiAdapter adapter = invokeMethod;
- Expression objectExpression = call.getObjectExpression();
- if (isSuperExpression(objectExpression)) {
+ if (isSuperExpression(receiver)) {
adapter = invokeMethodOnSuper;
- } else if (isThisExpression(objectExpression)) {
+ } else if (isThisExpression(receiver)) {
adapter = invokeMethodOnCurrent;
}
if (isStaticInvocation(call)) {
adapter = invokeStaticMethod;
}
Expression messageName = new CastExpression(ClassHelper.STRING_TYPE, call.getMethod());
- makeCall(call, objectExpression, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis());
+ makeCall(call, receiver, messageName, call.getArguments(), adapter, call.isSafe(), call.isSpreadSafe(), call.isImplicitThis());
}
}