This is an automated email from the ASF dual-hosted git repository. emilles pushed a commit to branch GROOVY_3_0_X in repository https://gitbox.apache.org/repos/asf/groovy.git
commit f3f686dff11b8dc2cb12a246b1c8908747a633fa Author: Eric Milles <[email protected]> AuthorDate: Fri Oct 10 10:50:01 2025 -0500 GROOVY-11776: set method target to trait helper for static dispatch 3_0_X backport --- .../transform/trait/TraitASTTransformation.java | 65 +++++++++------ .../groovy/transform/trait/TraitComposer.java | 30 ++++--- .../groovy/transform/traitx/Groovy11776.groovy | 96 ++++++++++++++++++++++ 3 files changed, 151 insertions(+), 40 deletions(-) diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java index 2184be1bfc..c32db8505f 100644 --- a/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java +++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java @@ -560,44 +560,55 @@ public class TraitASTTransformation extends AbstractASTTransformation implements } private MethodNode processMethod(final ClassNode traitClass, final ClassNode traitHelperClass, final MethodNode methodNode, final ClassNode fieldHelper, final Collection<String> knownFields) { - Parameter[] initialParams = methodNode.getParameters(); - Parameter[] newParams = new Parameter[initialParams.length + 1]; - newParams[0] = createSelfParameter(traitClass, methodNode.isStatic()); - System.arraycopy(initialParams, 0, newParams, 1, initialParams.length); - final int mod = methodNode.isPrivate() ? ACC_PRIVATE : ACC_PUBLIC | (methodNode.isFinal() ? ACC_FINAL : 0); + boolean isAbstractMethod = methodNode.isAbstract(); + boolean isPrivateMethod = methodNode.isPrivate(); + boolean isStaticMethod = methodNode.isStatic(); + + int modifiers = ACC_PUBLIC; + if (isAbstractMethod) { + modifiers |= ACC_ABSTRACT; + } else { + // public or private + if (isPrivateMethod) { + modifiers ^= ACC_PUBLIC | (!isStaticMethod ? ACC_PRIVATE : ACC_PROTECTED); // GROOVY-11776 + } + // static or final (maybe) + if (!isStaticMethod && methodNode.isFinal()) { + modifiers |= ACC_FINAL; + } + modifiers |= ACC_STATIC; + } + + Parameter[] methodParams = methodNode.getParameters(); + Parameter[] helperParams = new Parameter[methodParams.length + 1]; + helperParams[0] = createSelfParameter(traitClass, isStaticMethod); + System.arraycopy(methodParams, 0, helperParams, 1, methodParams.length); + MethodNode mNode = new MethodNode( methodNode.getName(), - mod | ACC_STATIC, + modifiers, methodNode.getReturnType(), - newParams, + helperParams, methodNode.getExceptions(), - processBody(varX(newParams[0]), methodNode.getCode(), traitClass, traitHelperClass, fieldHelper, knownFields) + processBody(varX(helperParams[0]), methodNode.getCode(), traitClass, traitHelperClass, fieldHelper, knownFields) ); - mNode.setSourcePosition(methodNode); - mNode.addAnnotations(filterAnnotations(methodNode.getAnnotations())); + for (AnnotationNode annotation : methodNode.getAnnotations()) { + if (!annotation.getClassNode().equals(OVERRIDE_CLASSNODE)) { + mNode.addAnnotation(annotation); + } + } mNode.setGenericsTypes(methodNode.getGenericsTypes()); - if (methodNode.isAbstract()) { - mNode.setModifiers(ACC_PUBLIC | ACC_ABSTRACT); - } else { + mNode.setSourcePosition(methodNode); + + if (!isAbstractMethod) { methodNode.addAnnotation(new AnnotationNode(Traits.IMPLEMENTED_CLASSNODE)); } - methodNode.setCode(null); - - if (!methodNode.isPrivate() && !methodNode.isStatic()) { + if (!isPrivateMethod && !isStaticMethod) { methodNode.setModifiers(ACC_PUBLIC | ACC_ABSTRACT); } - return mNode; - } - - private static List<AnnotationNode> filterAnnotations(final List<AnnotationNode> annotations) { - List<AnnotationNode> result = new ArrayList<>(annotations.size()); - for (AnnotationNode annotation : annotations) { - if (!OVERRIDE_CLASSNODE.equals(annotation.getClassNode())) { - result.add(annotation); - } - } + methodNode.setCode(null); - return result; + return mNode; } private static Parameter createSelfParameter(final ClassNode traitClass, boolean isStatic) { diff --git a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java index a396ab2ecc..f4aeaedf69 100644 --- a/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java +++ b/src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java @@ -335,28 +335,32 @@ public abstract class TraitComposer { helperMethodArgList ); mce.setImplicitThis(false); + if (!helperMethod.isPrivate()) mce.setMethodTarget(helperMethod); // GROOVY-11776 - ClassNode[] exceptionNodes = correctToGenericsSpecRecurse(genericsSpec, copyExceptions(helperMethod.getExceptions())); - ClassNode fixedReturnType = correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType()); - boolean noCastRequired = genericsSpec.isEmpty() || fixedReturnType.getName().equals(ClassHelper.VOID_TYPE.getName()); - Expression forwardExpression = noCastRequired ? mce : new CastExpression(fixedReturnType,mce); - // we could rely on the first parameter name ($static$self) but that information is not - // guaranteed to be always present - boolean isHelperForStaticMethod = helperMethodParams[0].getOriginType().equals(ClassHelper.CLASS_Type); + ClassNode[] exceptionTypes = correctToGenericsSpecRecurse(genericsSpec, copyExceptions(helperMethod.getExceptions())); + ClassNode returnType = correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType()); + // we could rely on the first parameter name ($static$self) but that information is not guaranteed to be always present + boolean noCastRequired = genericsSpec.isEmpty() || ClassHelper.VOID_TYPE.equals(returnType); + Expression forwardExpression = noCastRequired ? mce : new CastExpression(returnType, mce); + + boolean isHelperForStaticMethod = ClassHelper.CLASS_Type.equals(helperMethodParams[0].getOriginType()); if (helperMethod.isPrivate() && !isHelperForStaticMethod) { - // GROOVY-7213: do not create forwarder for private methods - return; + return; // GROOVY-7213: don't create forwarder for private method + } + int modifiers = helperMethod.getModifiers() & ~Opcodes.ACC_PROTECTED; + if (!helperMethod.isPublic()) { + modifiers |= Opcodes.ACC_PRIVATE; } - int modifiers = helperMethod.getModifiers(); if (!isHelperForStaticMethod) { - modifiers ^= Opcodes.ACC_STATIC; + modifiers &= ~Opcodes.ACC_STATIC; } + MethodNode forwarder = new MethodNode( helperMethod.getName(), modifiers, - fixedReturnType, + returnType, forwarderParams, - exceptionNodes, + exceptionTypes, new ExpressionStatement(forwardExpression) ); List<AnnotationNode> copied = new LinkedList<>(); diff --git a/src/test/org/codehaus/groovy/transform/traitx/Groovy11776.groovy b/src/test/org/codehaus/groovy/transform/traitx/Groovy11776.groovy new file mode 100644 index 0000000000..f26bbdc7b1 --- /dev/null +++ b/src/test/org/codehaus/groovy/transform/traitx/Groovy11776.groovy @@ -0,0 +1,96 @@ +/* + * 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 org.codehaus.groovy.transform.traitx + +import org.codehaus.groovy.control.CompilationUnit +import org.codehaus.groovy.control.CompilerConfiguration +import org.junit.Test +import org.objectweb.asm.ClassReader +import org.objectweb.asm.util.CheckClassAdapter + +final class Groovy11776 { + + @Test + void testTraitMethodOverloads() { + File sourceDir = File.createTempDir() + File targetDir = File.createTempDir() + try { + def a = new File(sourceDir, 'A.groovy') + a.write ''' + trait A { + def foo(Object o) { + return 'foo(o)' + } + def foo(Map<String,Object> m) { + return 'foo(m)' + } + } + ''' + def b = new File(sourceDir, 'B.groovy') + b.write ''' + class B implements A { + def bar(Object o) { + return 'bar(o)' + } + def bar(Map<String,Object> m) { + return 'bar(m)' + } + } + ''' + def c = new File(sourceDir, 'C.groovy') + c.write ''' + new B().with { + assert bar( (Object) null) == 'bar(o)' + assert bar(null as Object) == 'bar(o)' + assert foo( (Object) null) == 'foo(o)' + assert foo(null as Object) == 'foo(o)' + } + (new Object() as A).with { + assert foo( (Object) null) == 'foo(o)' + assert foo(null as Object) == 'foo(o)' + } + ''' + + def config = new CompilerConfiguration(targetDirectory: targetDir) + def loader = new GroovyClassLoader(this.class.classLoader) + def unit = new CompilationUnit(config, null, loader) + unit.addSources(a, b, c) + unit.compile() + + loader.addClasspath(targetDir.absolutePath) + loader.loadClass('C', true).main() + + // produce bytecode for class B + def writer = new StringWriter() + def reader = new ClassReader(unit.classes.find{ it.name == 'B' }.bytes) + CheckClassAdapter.verify(reader, loader, true, new PrintWriter(writer)) + + def string = writer.toString().with { + int start = indexOf('foo(Ljava/lang/Object;)') + int until = indexOf('ARETURN', start) + 8 + substring(start, until) + } + assert !string.contains('INVOKEDYNAMIC invoke(Ljava/lang/Class;') + assert string.contains('INVOKESTATIC A$Trait$Helper.foo') + } finally { + sourceDir.deleteDir() + targetDir.deleteDir() + } + } +}
