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 9299d69e9f GROOVY-11627: SC: disable mixed public and private
overloads checking
9299d69e9f is described below
commit 9299d69e9f420e5a76fa94d40d17afcdcd4d73cf
Author: Eric Milles <[email protected]>
AuthorDate: Wed May 14 15:21:07 2025 -0500
GROOVY-11627: SC: disable mixed public and private overloads checking
---
src/main/java/groovy/lang/MetaClassImpl.java | 64 +++++++++++-----------
.../groovy/classgen/ClassCompletionVerifier.java | 4 +-
src/test/groovy/bugs/Groovy5193.groovy | 26 +++++++++
.../traitx/TraitASTTransformationTest.groovy | 17 +++---
4 files changed, 70 insertions(+), 41 deletions(-)
diff --git a/src/main/java/groovy/lang/MetaClassImpl.java
b/src/main/java/groovy/lang/MetaClassImpl.java
index 6f5c181fd6..445c018675 100644
--- a/src/main/java/groovy/lang/MetaClassImpl.java
+++ b/src/main/java/groovy/lang/MetaClassImpl.java
@@ -446,50 +446,48 @@ public class MetaClassImpl implements MetaClass,
MutableMetaClass {
}
private void removeMultimethodsOverloadedWithPrivateMethods() {
- MethodIndexAction mia = new MethodIndexAction() {
+ var mia = new MethodIndexAction() {
@Override
public boolean skipClass(final Class<?> clazz) {
return clazz == theClass;
}
@Override
- public void methodNameAction(final Class<?> clazz, final
MetaMethodIndex.Cache e) {
- if (e.methods == null)
- return;
-
- boolean hasPrivate = false;
- if (e.methods instanceof FastArray) {
- FastArray methods = (FastArray) e.methods;
- final int len = methods.size();
- final Object[] data = methods.getArray();
- for (int i = 0; i != len; ++i) {
+ public void methodNameAction(final Class<?> clazz, final
MetaMethodIndex.Cache entry) {
+ if (hasPrivateInMethods(clazz, entry)) { // GROOVY-5193, etc.
+ // We have private methods for that name, so remove the
+ // multimethods. That is the same as in our index for
+ // super, so just copy the list from there. It is not
+ // possible to use a pointer here because the methods
+ // in the index for super are replaced later by MOP
+ // methods like super$5$foo
+ Object o = entry.methodsForSuper;
+ if (o instanceof FastArray) {
+ entry.methods = ((FastArray) o).copy();
+ } else {
+ entry.methods = o;
+ }
+ }
+ }
+
+ private boolean hasPrivateInMethods(final Class<?> clazz, final
MetaMethodIndex.Cache entry) {
+ if (entry.methods instanceof FastArray) {
+ FastArray methods = (FastArray) entry.methods;
+ int size = methods.size();
+ var data = methods.getArray();
+ for (int i = 0; i != size; ++i) {
MetaMethod method = (MetaMethod) data[i];
- if (method.isPrivate() && clazz ==
method.getDeclaringClass().getTheClass()) {
- hasPrivate = true;
- break;
+ if (method.isPrivate() &&
method.getDeclaringClass().getTheClass() == clazz) {
+ return true;
}
}
- } else {
- MetaMethod method = (MetaMethod) e.methods;
- if (method.isPrivate() && clazz ==
method.getDeclaringClass().getTheClass()) {
- hasPrivate = true;
+ } else if (entry.methods instanceof MetaMethod) {
+ MetaMethod method = (MetaMethod) entry.methods;
+ if (method.isPrivate() &&
method.getDeclaringClass().getTheClass() == clazz) {
+ return true;
}
}
-
- if (!hasPrivate) return;
-
- // We have private methods for that name, so remove the
- // multimethods. That is the same as in our index for
- // super, so just copy the list from there. It is not
- // possible to use a pointer here, because the methods
- // in the index for super are replaced later by MOP
- // methods like super$5$foo
- final Object o = e.methodsForSuper;
- if (o instanceof FastArray) {
- e.methods = ((FastArray) o).copy();
- } else {
- e.methods = o;
- }
+ return false;
}
};
mia.iterate();
diff --git
a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
index 2414ba05f3..5cf32d125e 100644
--- a/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
+++ b/src/main/java/org/codehaus/groovy/classgen/ClassCompletionVerifier.java
@@ -74,6 +74,7 @@ import static
org.codehaus.groovy.ast.tools.GenericsUtils.addMethodGenerics;
import static org.codehaus.groovy.ast.tools.GenericsUtils.buildWildcardType;
import static
org.codehaus.groovy.ast.tools.GenericsUtils.correctToGenericsSpecRecurse;
import static org.codehaus.groovy.ast.tools.GenericsUtils.createGenericsSpec;
+import static
org.codehaus.groovy.transform.sc.StaticCompilationVisitor.isStaticallyCompiled;
import static org.objectweb.asm.Opcodes.ACC_FINAL;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_STATIC;
@@ -538,8 +539,8 @@ out: for (ClassNode sc : superTypes) {
checkOverloadingPrivateAndPublic(node);
}
checkMethodForIncorrectModifiers(node);
- checkGenericsUsage(node, node.getParameters());
checkGenericsUsage(node, node.getReturnType());
+ checkGenericsUsage(node, node.getParameters());
for (Parameter param : node.getParameters()) {
if (ClassHelper.isPrimitiveVoid(param.getType())) {
addError("The " + getDescription(param) + " in " +
getDescription(node) + " has invalid type void", param);
@@ -588,6 +589,7 @@ out: for (ClassNode sc : superTypes) {
}
private void checkOverloadingPrivateAndPublic(final MethodNode node) {
+ if (isStaticallyCompiled(currentClass)) return; // GROOVY-11627
boolean mixed = false;
if (node.isPublic()) {
for (MethodNode mn :
currentClass.getDeclaredMethods(node.getName())) {
diff --git a/src/test/groovy/bugs/Groovy5193.groovy
b/src/test/groovy/bugs/Groovy5193.groovy
index d0284b5ae3..9ec5883a4d 100644
--- a/src/test/groovy/bugs/Groovy5193.groovy
+++ b/src/test/groovy/bugs/Groovy5193.groovy
@@ -21,6 +21,7 @@ package bugs
import org.codehaus.groovy.control.MultipleCompilationErrorsException
import org.junit.jupiter.api.Test
+import static groovy.test.GroovyAssert.assertScript
import static groovy.test.GroovyAssert.shouldFail
final class Groovy5193 {
@@ -54,4 +55,29 @@ final class Groovy5193 {
'''
assert err.message.contains('Mixing private and public/protected
methods of the same name causes multimethods to be disabled')
}
+
+ @Test
+ void testMixingMethodsWithPrivatePublicAccessInSameClass3() {
+ assertScript '''
+ class A {
+ def f(x) { g(x) }
+ def g(x) {"object case"}
+ }
+ class B extends A {
+ private g(String x) {"string case"}
+ }
+
+ def a = new A()
+ assert a.f(null) == "object case"
+ assert a.g(null) == "object case"
+ assert a.f("xx") == "object case"
+ assert a.g("xx") == "object case"
+
+ def b = new B()
+ assert b.f(null) == "object case"
+ assert b.g(null) == "object case"
+ assert b.f("xx") == "object case"
+ assert b.g("xx") == "string case"
+ '''
+ }
}
diff --git
a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
index 32a21c5cdb..53b3c731e6 100644
---
a/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
+++
b/src/test/groovy/org/codehaus/groovy/transform/traitx/TraitASTTransformationTest.groovy
@@ -1810,21 +1810,24 @@ final class TraitASTTransformationTest {
"""
}
- // GROOVY-5193
+ // GROOVY-5193, GROOVY-11627
@Test
void testMixPrivatePublicMethodsOfSameName() {
- def err = shouldFail shell, '''
+ assertScript shell, '''
+ @CompileStatic
trait T {
- private String secret(String s) { s.toUpperCase() }
- String secret() { 'public' }
- String foo() { secret('secret') }
+ private String f(String s) { s.toUpperCase() }
+ public String f(Object o) { 'default value' }
+ public String f( ) { 'unknown value' }
+ def m() {
+ f('secret')
+ }
}
class C implements T {
}
- assert new C().foo() == 'SECRET'
+ assert new C().m() == 'SECRET'
'''
- assert err =~ 'Mixing private and public/protected methods of the same
name causes multimethods to be disabled'
}
@Test