Skip to content

Commit 089590d

Browse files
eric-milleseric-milles
committed
GROOVY-11782: indy guard for null alternation of primitive parameters
3_0_X backport
1 parent f3f686d commit 089590d

5 files changed

Lines changed: 73 additions & 25 deletions

File tree

‎src/main/java/org/codehaus/groovy/vmplugin/v7/IndyGuardsFiltersAndSignatures.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Collection;
4545
import java.util.HashSet;
4646
import java.util.Map;
47+
import java.util.Objects;
4748

4849
import static org.codehaus.groovy.vmplugin.v7.IndyInterface.LOOKUP;
4950

@@ -71,7 +72,7 @@ public class IndyGuardsFiltersAndSignatures {
7172

7273
protected static final MethodHandle
7374
SAME_CLASS, UNWRAP_METHOD,
74-
SAME_MC, IS_NULL,
75+
SAME_MC, IS_NULL, NON_NULL,
7576
UNWRAP_EXCEPTION, META_METHOD_INVOKER,
7677
GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY,
7778
HAS_CATEGORY_IN_CURRENT_THREAD_GUARD,
@@ -91,6 +92,7 @@ public class IndyGuardsFiltersAndSignatures {
9192
UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER);
9293
SAME_MC = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", METACLASS1_GUARD);
9394
IS_NULL = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isNull", OBJECT_GUARD);
95+
NON_NULL = LOOKUP.findStatic(Objects.class, "nonNull", MethodType.methodType(boolean.class, Object.class));
9496
UNWRAP_EXCEPTION = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", GRE_GUARD);
9597
GROOVY_OBJECT_INVOKER = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "invokeGroovyObjectInvoker", INVOKER.insertParameterTypes(0, MissingMethodException.class));
9698

‎src/main/java/org/codehaus/groovy/vmplugin/v7/Selector.java‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,7 @@
8585
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.MOP_GET;
8686
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.MOP_INVOKE_CONSTRUCTOR;
8787
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.MOP_INVOKE_METHOD;
88+
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.NON_NULL;
8889
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.NULL_REF;
8990
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.SAME_CLASS;
9091
import static org.codehaus.groovy.vmplugin.v7.IndyGuardsFiltersAndSignatures.SAME_MC;
@@ -880,8 +881,7 @@ public void addExceptionHandler() {
880881
* Sets all argument and receiver guards.
881882
*/
882883
public void setGuards(Object receiver) {
883-
if (handle == null) return;
884-
if (!cache) return;
884+
if (!cache || handle == null) return;
885885

886886
MethodHandle fallback;
887887
if (callSite instanceof CacheableCallSite) {
@@ -906,7 +906,7 @@ public void setGuards(Object receiver) {
906906
if (LOG_ENABLED) LOG.info("added class equality check");
907907
}
908908

909-
if (!useMetaClass && isCategoryMethod) {
909+
if (isCategoryMethod && !useMetaClass) {
910910
// category method needs Thread check
911911
// cases:
912912
// (1) method is a category method
@@ -935,14 +935,12 @@ public void setGuards(Object receiver) {
935935
if (arg == null) {
936936
test = IS_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
937937
if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
938+
} else if (pt[i].isPrimitive()) { // GROOVY-11782: if null then uncache
939+
test = NON_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
940+
if (LOG_ENABLED) LOG.info("added non-null argument check at pos " + i);
938941
} else {
939-
Class<?> argClass = arg.getClass();
940-
if (pt[i].isPrimitive()) continue;
941-
//if (Modifier.isFinal(argClass.getModifiers()) && TypeHelper.argumentClassIsParameterClass(argClass,pt[i])) continue;
942-
test = SAME_CLASS.
943-
bindTo(argClass).
944-
asType(MethodType.methodType(boolean.class, pt[i]));
945-
if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
942+
test = SAME_CLASS.bindTo(arg.getClass()).asType(MethodType.methodType(boolean.class, pt[i]));
943+
if (LOG_ENABLED) LOG.info("added same-class argument check at pos " + i);
946944
}
947945
Class<?>[] drops = new Class[i];
948946
System.arraycopy(pt, 0, drops, 0, drops.length);

‎src/main/java/org/codehaus/groovy/vmplugin/v8/IndyGuardsFiltersAndSignatures.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.Collection;
4545
import java.util.HashSet;
4646
import java.util.Map;
47+
import java.util.Objects;
4748

4849
import static org.codehaus.groovy.vmplugin.v8.IndyInterface.LOOKUP;
4950

@@ -59,7 +60,7 @@ public class IndyGuardsFiltersAndSignatures {
5960
INVOKER = MethodType.methodType(Object.class, Object.class, String.class, Object[].class);
6061

6162
protected static final MethodHandle
62-
SAME_CLASS, SAME_MC, IS_NULL,
63+
SAME_CLASS, SAME_MC, IS_NULL, NON_NULL,
6364
UNWRAP_METHOD, UNWRAP_EXCEPTION,
6465
HAS_CATEGORY_IN_CURRENT_THREAD_GUARD,
6566
META_METHOD_INVOKER, GROOVY_OBJECT_INVOKER, GROOVY_OBJECT_GET_PROPERTY,
@@ -80,6 +81,7 @@ public class IndyGuardsFiltersAndSignatures {
8081
SAME_CLASS = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "sameClass", MethodType.methodType(boolean.class, Class.class, Object.class));
8182
SAME_MC = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isSameMetaClass", MethodType.methodType(boolean.class, MetaClass.class, Object.class));
8283
IS_NULL = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "isNull", OBJECT_GUARD);
84+
NON_NULL = LOOKUP.findStatic(Objects.class, "nonNull", MethodType.methodType(boolean.class, Object.class));
8385
UNWRAP_METHOD = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", OBJECT_FILTER);
8486
UNWRAP_EXCEPTION = LOOKUP.findStatic(IndyGuardsFiltersAndSignatures.class, "unwrap", MethodType.methodType(Object.class, GroovyRuntimeException.class));
8587
HAS_CATEGORY_IN_CURRENT_THREAD_GUARD = LOOKUP.findStatic(GroovyCategorySupport.class, "hasCategoryInCurrentThread", MethodType.methodType(boolean.class));

‎src/main/java/org/codehaus/groovy/vmplugin/v8/Selector.java‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@
8686
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.MOP_GET;
8787
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.MOP_INVOKE_CONSTRUCTOR;
8888
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.MOP_INVOKE_METHOD;
89+
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.NON_NULL;
8990
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.NULL_REF;
9091
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.SAME_CLASS;
9192
import static org.codehaus.groovy.vmplugin.v8.IndyGuardsFiltersAndSignatures.SAME_MC;
@@ -899,8 +900,7 @@ public void addExceptionHandler() {
899900
* Sets all argument and receiver guards.
900901
*/
901902
public void setGuards(Object receiver) {
902-
if (handle == null) return;
903-
if (!cache) return;
903+
if (!cache || handle == null) return;
904904

905905
MethodHandle fallback;
906906
if (callSite instanceof CacheableCallSite) {
@@ -925,7 +925,7 @@ public void setGuards(Object receiver) {
925925
if (LOG_ENABLED) LOG.info("added class equality check");
926926
}
927927

928-
if (!useMetaClass && isCategoryMethod) {
928+
if (isCategoryMethod && !useMetaClass) {
929929
// category method needs Thread check
930930
// cases:
931931
// (1) method is a category method
@@ -950,20 +950,16 @@ public void setGuards(Object receiver) {
950950
Class<?>[] pt = handle.type().parameterArray();
951951
for (int i = 0; i < args.length; i++) {
952952
Object arg = args[i];
953-
Class<?> paramType = pt[i];
954953
MethodHandle test;
955-
956954
if (arg == null) {
957-
test = IS_NULL.asType(MethodType.methodType(boolean.class, paramType));
955+
test = IS_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
958956
if (LOG_ENABLED) LOG.info("added null argument check at pos " + i);
957+
} else if (pt[i].isPrimitive()) { // GROOVY-11782: if null then uncache
958+
test = NON_NULL.asType(MethodType.methodType(boolean.class, pt[i]));
959+
if (LOG_ENABLED) LOG.info("added non-null argument check at pos " + i);
959960
} else {
960-
Class<?> argClass = arg.getClass();
961-
if (paramType.isPrimitive()) continue;
962-
//if (Modifier.isFinal(argClass.getModifiers()) && TypeHelper.argumentClassIsParameterClass(argClass,pt[i])) continue;
963-
test = SAME_CLASS.
964-
bindTo(argClass).
965-
asType(MethodType.methodType(boolean.class, paramType));
966-
if (LOG_ENABLED) LOG.info("added same class check at pos " + i);
961+
test = SAME_CLASS.bindTo(arg.getClass()).asType(MethodType.methodType(boolean.class, pt[i]));
962+
if (LOG_ENABLED) LOG.info("added same-class argument check at pos " + i);
967963
}
968964
Class<?>[] drops = new Class[i];
969965
System.arraycopy(pt, 0, drops, 0, drops.length);

‎src/test/groovy/bugs/Groovy11782.groovy‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package groovy.bugs
20+
21+
import org.codehaus.groovy.control.CompilerConfiguration
22+
import org.junit.Test
23+
24+
final class Groovy11782 {
25+
26+
@Test
27+
void testNullForPrimitiveParameter() {
28+
def config = new CompilerConfiguration()
29+
config.optimizationOptions.indy = true
30+
new GroovyShell(config).evaluate '''
31+
class C11782 {
32+
private static int inc(int i) {
33+
++i
34+
}
35+
}
36+
37+
def functor = { Integer i ->
38+
C11782.inc(i)
39+
}
40+
41+
10000.times {
42+
assert functor(0) == 1
43+
}
44+
groovy.test.GroovyAssert.shouldFail(MissingMethodException) {
45+
functor(null)
46+
}
47+
assert functor(1) == 2
48+
'''
49+
}
50+
}

0 commit comments

Comments
 (0)