This is an automated email from the ASF dual-hosted git repository.
emilles pushed a commit to branch GROOVY_4_0_X
in repository https://gitbox.apache.org/repos/asf/groovy.git
The following commit(s) were added to refs/heads/GROOVY_4_0_X by this push:
new e267e346a2 GROOVY-11782: indy guard for null alternation of primitive
parameters
e267e346a2 is described below
commit e267e346a2f0aa26661f8003d876d7e2af686b28
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 | 25 +++++-----
.../codehaus/groovy/vmplugin/v8/TypeHelper.java | 53 ++++++++++++----------
src/test/groovy/bugs/Groovy11126.groovy | 48 +++++++++-----------
.../{Groovy11126.groovy => Groovy11782.groovy} | 46 +++++++------------
4 files changed, 82 insertions(+), 90 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 30c6434346..14704cfb65 100644
--- a/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
+++ b/src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java
@@ -898,8 +898,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;
if (callSite instanceof CacheableCallSite) {
@@ -924,7 +923,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
@@ -945,35 +944,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;
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..0ea1507ed4 100644
--- a/src/test/groovy/bugs/Groovy11126.groovy
+++ b/src/test/groovy/bugs/Groovy11126.groovy
@@ -20,39 +20,35 @@ package bugs
import org.junit.Test
-import static groovy.test.GroovyAssert.assertScript
-
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 55%
copy from src/test/groovy/bugs/Groovy11126.groovy
copy to src/test/groovy/bugs/Groovy11782.groovy
index cb3cebdff1..a508196559 100644
--- a/src/test/groovy/bugs/Groovy11126.groovy
+++ b/src/test/groovy/bugs/Groovy11782.groovy
@@ -20,39 +20,27 @@ package bugs
import org.junit.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)
+ }
}
}
