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

4_0_X backport

Author: eric-milles

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

@@ -87,6 +87,7 @@
 import static org.objectweb.asm.Opcodes.ACC_FINAL;
 import static org.objectweb.asm.Opcodes.ACC_INTERFACE;
 import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
+import static org.objectweb.asm.Opcodes.ACC_PROTECTED;
 import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
 import static org.objectweb.asm.Opcodes.ACC_STATIC;
 import static org.objectweb.asm.Opcodes.ACC_SYNTHETIC;
@@ -569,43 +570,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()));
-        mNode.setGenericsTypes(methodNode.getGenericsTypes());
-        if (methodNode.isAbstract()) {
-            mNode.setModifiers(ACC_PUBLIC | ACC_ABSTRACT);
-        } else {
-            methodNode.addAnnotation(new AnnotationNode(Traits.IMPLEMENTED_CLASSNODE));
+        for (AnnotationNode annotation : methodNode.getAnnotations()) {
+            if (!annotation.getClassNode().equals(OVERRIDE_TYPE)) {
+                mNode.addAnnotation(annotation);
+            }
         }
-        methodNode.setCode(null);
+        mNode.setGenericsTypes(methodNode.getGenericsTypes());
+        mNode.setSourcePosition(methodNode);
 
-        if (!methodNode.isPrivate() && !methodNode.isStatic()) {
+        if (!isAbstractMethod) {
+            methodNode.addAnnotation(Traits.IMPLEMENTED_CLASSNODE);
+        }
+        if (!isPrivateMethod && !isStaticMethod) {
             methodNode.setModifiers(ACC_PUBLIC | ACC_ABSTRACT);
         }
-        return mNode;
-    }
+        methodNode.setCode(null);
 
-    private static List<AnnotationNode> filterAnnotations(final List<AnnotationNode> annotations) {
-        List<AnnotationNode> result = new ArrayList<>(annotations.size());
-        for (AnnotationNode annotation : annotations) {
-            if (!annotation.getClassNode().equals(OVERRIDE_TYPE)) {
-                result.add(annotation);
-            }
-        }
-        return result;
+        return mNode;
     }
 
     private static Parameter createSelfParameter(final ClassNode traitClass, boolean isStatic) {

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

@@ -325,17 +325,20 @@ private static void createForwarderMethod(
                 helperMethodArgList
         );
         mce.setImplicitThis(false);
+        if (!helperMethod.isPrivate()) mce.setMethodTarget(helperMethod); // GROOVY-11776
 
         ClassNode[] exceptionTypes = GenericsUtils.correctToGenericsSpecRecurse(genericsSpec, copyExceptions(helperMethod.getExceptions()));
         ClassNode returnType = GenericsUtils.correctToGenericsSpecRecurse(genericsSpec, helperMethod.getReturnType());
         boolean castRequired = !genericsSpec.isEmpty() && !helperMethod.isVoidMethod();
 
         boolean isHelperForStaticMethod = ClassHelper.isClassType(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;
         }

src/test/groovy/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()
+        }
+    }
+}