This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_5_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_5_0_X by this push:
new bbbf036b9c GROOVY-11782: indy guard for null alternation of primitive
parameters
bbbf036b9c is described below
commit bbbf036b9c4878b50bf49c2d6488ec2e1ab72d5e
Author: Eric Milles <[email protected]>
AuthorDate: Sun Oct 12 11:35:00 2025 -0500
GROOVY-11782: indy guard for null alternation of primitive parameters
---
.../org/codehaus/groovy/vmplugin/v8/Selector.java | 27 +++++------
.../codehaus/groovy/vmplugin/v8/TypeHelper.java | 53 ++++++++++++----------
src/test/groovy/bugs/Groovy11126.groovy | 50 ++++++++++----------
.../{Groovy11126.groovy => Groovy11782.groovy} | 48 ++++++++------------
4 files changed, 85 insertions(+), 93 deletions(-)
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
index f173e9df8d..c6d9b565d5 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -929,8 +929,7 @@ public abstract class Selector {
* Sets all argument and receiver guards.
*/
public void setGuards(Object receiver) {
- if (handle == null) return;
- if (!cache) return;
+ if (!cache || handle == null) return;
MethodHandle fallback = callSite.getFallbackTarget();
@@ -950,7 +949,7 @@ public abstract class Selector {
if (LOG_ENABLED) LOG.info("added class equality check");
}
- if (!useMetaClass && isCategoryMethod) {
+ if (isCategoryMethod && !useMetaClass) {
// category method needs Thread check
// cases:
// (1) method is a category method
@@ -971,35 +970,37 @@ public abstract class Selector {
handle = switchPoint.guardWithTest(handle, fallback);
if (LOG_ENABLED) LOG.info("added switch point guard");
+ java.util.function.Predicate<Class<?>> nonFinalOrNullUnsafe = (t)
-> {
+ return !Modifier.isFinal(t.getModifiers())
+ || TypeHelper.getUnboxedType(t).isPrimitive(); //
GROOVY-11782
+ };
+
// guards for receiver and parameter
Class<?>[] pt = handle.type().parameterArray();
if (Arrays.stream(args).anyMatch(Objects::isNull)) {
for (int i = 0; i < args.length; i++) {
- Object arg = args[i];
- Class<?> paramType = pt[i];
MethodHandle test;
+ var arg = args[i];
if (arg == null) {
- test =
IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
+ test =
IS_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
if (LOG_ENABLED) LOG.info("added null argument check
at pos " + i);
} else {
- if (Modifier.isFinal(paramType.getModifiers())) {
- // primitive types are also `final`
- continue;
- }
- test =
SAME_CLASS.bindTo(arg.getClass()).asType(MethodType.methodType(boolean.class,
paramType));
- if (LOG_ENABLED) LOG.info("added same class check at
pos " + i);
+ if (nonFinalOrNullUnsafe.negate().test(pt[i]))
continue; // null-safe type that cannot change
+ test =
SAME_CLASS.bindTo(arg.getClass()).asType(MethodType.methodType(boolean.class,
pt[i]));
+ if (LOG_ENABLED) LOG.info("added same-class argument
check at pos " + i);
}
Class<?>[] drops = new Class[i];
System.arraycopy(pt, 0, drops, 0, drops.length);
test = MethodHandles.dropArguments(test, 0, drops);
handle = MethodHandles.guardWithTest(test, handle,
fallback);
}
- } else if (Arrays.stream(pt).anyMatch(paramType ->
!Modifier.isFinal(paramType.getModifiers()))) {
+ } else if (Arrays.stream(pt).anyMatch(nonFinalOrNullUnsafe)) {
MethodHandle test = SAME_CLASSES
.bindTo(Arrays.stream(args).map(Object::getClass).toArray(Class[]::new))
.asCollector(Object[].class, pt.length)
.asType(MethodType.methodType(boolean.class, pt));
handle = MethodHandles.guardWithTest(test, handle, fallback);
+ if (LOG_ENABLED) LOG.info("added same-class argument check");
} else if (safeNavigationOrig) { // GROOVY-11126
MethodHandle test =
NON_NULL.asType(MethodType.methodType(boolean.class, pt[0]));
handle = MethodHandles.guardWithTest(test, handle, fallback);
diff --git a/src/main/java/org/codehaus/groovy/vmplugin/v8/TypeHelper.java
b/src/main/java/org/codehaus/groovy/vmplugin/v8/TypeHelper.java
index 1ebd9f664a..cda886e07d 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/TypeHelper.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/TypeHelper.java
@@ -25,34 +25,41 @@ import java.math.BigInteger;
/**
* This class contains helper methods for converting and comparing types.
- * WARNING: This class is for internal use only. do not use it outside of its
- * package and not outside groovy-core.
+ * WARNING: This class is for internal use only. Do not use it outside of its
+ * package and not outside of groovy-core.
*/
public class TypeHelper {
+
+ protected static Class<?> getUnboxedType(Class<?> c) {
+ if (c == null || c.isPrimitive()) ;
+ else if (c == Boolean.class) c = Boolean.TYPE;
+ else if (c == Byte.class) c = Byte.TYPE;
+ else if (c == Character.class) c = Character.TYPE;
+ else if (c == Float.class) c = Float.TYPE;
+ else if (c == Double.class) c = Double.TYPE;
+ else if (c == Integer.class) c = Integer.TYPE;
+ else if (c == Long.class) c = Long.TYPE;
+ else if (c == Short.class) c = Short.TYPE;
+ //else if (c == Void.class) c = Void.TYPE;
+ return c;
+ }
+
/**
- * Get wrapper class for a given class.
- * If the class is for a primitive number type, then the wrapper class
- * will be returned. If it is no primitive number type, we return the
- * class itself.
+ * Gets wrapper class for a given class. If the class is for a primitive
+ * number type, then the wrapper class will be returned. If it is not a
+ * primitive number type, we return the class itself.
*/
protected static Class<?> getWrapperClass(Class<?> c) {
- if (c == Integer.TYPE) {
- c = Integer.class;
- } else if (c == Byte.TYPE) {
- c = Byte.class;
- } else if (c == Long.TYPE) {
- c = Long.class;
- } else if (c == Double.TYPE) {
- c = Double.class;
- } else if (c == Float.TYPE) {
- c = Float.class;
- } else if (c == Boolean.TYPE) {
- c = Boolean.class;
- } else if (c == Character.TYPE) {
- c = Character.class;
- } else if (c == Short.TYPE) {
- c = Short.class;
- }
+ if (c == null || !c.isPrimitive()) ;
+ else if (c == Integer.TYPE) c = Integer.class;
+ else if (c == Byte.TYPE) c = Byte.class;
+ else if (c == Long.TYPE) c = Long.class;
+ else if (c == Double.TYPE) c = Double.class;
+ else if (c == Float.TYPE) c = Float.class;
+ else if (c == Boolean.TYPE) c = Boolean.class;
+ else if (c == Character.TYPE) c = Character.class;
+ else if (c == Short.TYPE) c = Short.class;
+ //else if (c == Void.TYPE) c = Void.class;
return c;
}
diff --git a/src/test/groovy/bugs/Groovy11126.groovy
b/src/test/groovy/bugs/Groovy11126.groovy
index cb3cebdff1..f2b01cd2db 100644
--- a/src/test/groovy/bugs/Groovy11126.groovy
+++ b/src/test/groovy/bugs/Groovy11126.groovy
@@ -18,41 +18,37 @@
*/
package bugs
-import org.junit.Test
-
-import static groovy.test.GroovyAssert.assertScript
+import org.junit.jupiter.api.Test
final class Groovy11126 {
+ private safeCall(String string) {
+ string?.size()
+ }
+
+ private safeName(String string) {
+ string?.chars
+ }
+
@Test
void testSafeCall() {
- assertScript '''
- def test(String string) {
- string?.size()
- }
- 10000.times {
- test('1')
- test('')
- }
- test(null)
- test('22')
- test(null)
- '''
+ 10000.times {
+ safeCall('1')
+ safeCall('')
+ }
+ safeCall(null)
+ safeCall('22')
+ safeCall(null)
}
@Test
void testSafeName() {
- assertScript '''
- def test(String string) {
- string?.chars
- }
- 10000.times {
- test('1')
- test('')
- }
- test(null)
- test('22')
- test(null)
- '''
+ 10000.times {
+ safeName('1')
+ safeName('')
+ }
+ safeName(null)
+ safeName('22')
+ safeName(null)
}
}
diff --git a/src/test/groovy/bugs/Groovy11126.groovy
b/src/test/groovy/bugs/Groovy11782.groovy
similarity index 54%
copy from src/test/groovy/bugs/Groovy11126.groovy
copy to src/test/groovy/bugs/Groovy11782.groovy
index cb3cebdff1..1790adc1ff 100644
--- a/src/test/groovy/bugs/Groovy11126.groovy
+++ b/src/test/groovy/bugs/Groovy11782.groovy
@@ -18,41 +18,29 @@
*/
package bugs
-import org.junit.Test
+import org.junit.jupiter.api.Test
-import static groovy.test.GroovyAssert.assertScript
+import static groovy.test.GroovyAssert.shouldFail
-final class Groovy11126 {
+final class Groovy11782 {
- @Test
- void testSafeCall() {
- assertScript '''
- def test(String string) {
- string?.size()
- }
- 10000.times {
- test('1')
- test('')
- }
- test(null)
- test('22')
- test(null)
- '''
+ private static int inc(int i) {
+ ++i
+ }
+
+ final Closure functor = { Integer i ->
+ Groovy11782.inc(i)
}
@Test
- void testSafeName() {
- assertScript '''
- def test(String string) {
- string?.chars
- }
- 10000.times {
- test('1')
- test('')
- }
- test(null)
- test('22')
- test(null)
- '''
+ void testNullForPrimitiveParameter() {
+ assert functor(0) == 1
+ shouldFail(MissingMethodException) {
+ functor(null)
+ }
+ assert functor(1) == 2
+ shouldFail(MissingMethodException) {
+ functor(null)
+ }
}
}
