GROOVY-11776: set method target to trait helper for static dispatch

3_0_X backport

Author: eric-milles

src/main/java/org/codehaus/groovy/transform/trait/TraitASTTransformation.java

@@ -560,44 +560,55 @@ private void processField(final FieldNode field, final MethodNode initializer, f
     }
 
     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) {

src/main/java/org/codehaus/groovy/transform/trait/TraitComposer.java

@@ -335,28 +335,32 @@ private static void createForwarderMethod(
                 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<>();

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()
+        }
+    }
+}