GROOVY-12140: normalize reflectively boxed primitive returns through valueOf

Method#invoke boxes primitive return values with fresh instances, unlike
MethodHandle-based and generated-bytecode invocation, which box through
the valueOf caches. Dispatch paths that invoke reflectively therefore
broke reference identity (===) of primitive returns: classic-mode
reflective call sites (e.g. varargs methods), and — in both compilation
modes — anything routed through MetaMethod#doMethodInvoke, notably
dynamic-name calls such as obj."$name"(), where a constant-name call
to the same method returns the cached box.

Reflective results are now re-normalized through the valueOf caches at
the two chokepoints: CachedMethod#invoke (all doMethodInvoke routes) and
PlainObjectMetaMethodSite#doInvoke (classic call sites holding a raw
Method). The shared helper lives in MetaClassHelper#normalizeBoxedReturn.
float/double are left as-is since valueOf does not cache them on any
path. Also reproducible on GROOVY_5_0_X, so a candidate for backport.

Author: paulk-asert

src/main/java/org/codehaus/groovy/reflection/CachedMethod.java

@@ -23,6 +23,7 @@
 import groovy.lang.MissingMethodException;
 import org.codehaus.groovy.classgen.asm.BytecodeHelper;
 import org.codehaus.groovy.runtime.InvokerInvocationException;
+import org.codehaus.groovy.runtime.MetaClassHelper;
 import org.codehaus.groovy.runtime.callsite.CallSite;
 import org.codehaus.groovy.runtime.callsite.CallSiteGenerator;
 import org.codehaus.groovy.runtime.callsite.PogoMetaMethodSite;
@@ -499,7 +500,7 @@ public final Object invoke(final Object object, final Object[] arguments) {
         makeAccessibleIfNecessary();
 
         try {
-            return cachedMethod.invoke(object, arguments);
+            return normalizeBoxedReturn(cachedMethod.invoke(object, arguments));
         } catch (IllegalArgumentException | IllegalAccessException e) {
             throw new InvokerInvocationException(e);
         } catch (InvocationTargetException e) {
@@ -508,6 +509,16 @@ public final Object invoke(final Object object, final Object[] arguments) {
         }
     }
 
+    /**
+     * Keeps reference identity ({@code ===}) of primitive returns consistent
+     * across dispatch paths (GROOVY-12140).
+     *
+     * @see MetaClassHelper#normalizeBoxedReturn(Object, Class)
+     */
+    private Object normalizeBoxedReturn(final Object value) {
+        return MetaClassHelper.normalizeBoxedReturn(value, cachedMethod.getReturnType());
+    }
+
     private void makeAccessibleIfNecessary() {
         if (!makeAccessibleDone) {
             ReflectionUtils.makeAccessibleInPrivilegedAction(cachedMethod);

src/main/java/org/codehaus/groovy/runtime/MetaClassHelper.java

@@ -1009,6 +1009,30 @@ public static void unwrap(Object[] arguments) {
         }
     }
 
+    /**
+     * Re-normalizes a reflectively boxed primitive return value through the
+     * {@code valueOf} caches. {@link java.lang.reflect.Method#invoke} boxes
+     * primitive returns with fresh instances, unlike MethodHandle-based and
+     * generated-bytecode invocation, which box through the caches; without
+     * normalization, reference identity ({@code ===}) of primitive returns
+     * would differ between dispatch paths (GROOVY-12140).
+     *
+     * @param value the reflectively boxed return value (may be {@code null})
+     * @param returnType the declared return type of the invoked method
+     * @return the cache-normalized value
+     * @since 6.0.0
+     */
+    public static Object normalizeBoxedReturn(final Object value, final Class<?> returnType) {
+        if (value == null || !returnType.isPrimitive()) return value; // null includes void
+        if (returnType == int.class) return Integer.valueOf((Integer) value);
+        if (returnType == boolean.class) return Boolean.valueOf((Boolean) value);
+        if (returnType == long.class) return Long.valueOf((Long) value);
+        if (returnType == char.class) return Character.valueOf((Character) value);
+        if (returnType == byte.class) return Byte.valueOf((Byte) value);
+        if (returnType == short.class) return Short.valueOf((Short) value);
+        return value; // float/double: valueOf does not cache on any path
+    }
+
     /**
      * Sets the metaclass for an object, by delegating to the appropriate
      * {@link DefaultGroovyMethods} helper method. This method was introduced as

src/main/java/org/codehaus/groovy/runtime/callsite/PlainObjectMetaMethodSite.java

@@ -22,6 +22,7 @@
 import groovy.lang.GroovyRuntimeException;
 import groovy.lang.MetaClass;
 import groovy.lang.MetaMethod;
+import org.codehaus.groovy.runtime.MetaClassHelper;
 import org.codehaus.groovy.runtime.ScriptBytecodeAdapter;
 
 import java.lang.reflect.InvocationTargetException;
@@ -40,7 +41,8 @@ public PlainObjectMetaMethodSite(CallSite site, MetaClass metaClass, MetaMethod
 
     protected static Object doInvoke(Object receiver, Object[] args, Method reflect) throws Throwable {
         try {
-            return reflect.invoke(receiver, args);
+            // GROOVY-12140: keep === identity of primitive returns consistent
+            return MetaClassHelper.normalizeBoxedReturn(reflect.invoke(receiver, args), reflect.getReturnType());
         } catch (InvocationTargetException e) {
             Throwable cause = e.getCause();
             if (cause instanceof GroovyRuntimeException) {

src/test/groovy/bugs/Groovy12140.groovy

@@ -0,0 +1,66 @@
+/*
+ *  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 bugs
+
+import org.codehaus.groovy.control.CompilerConfiguration
+import org.junit.jupiter.api.Test
+
+import static groovy.test.GroovyAssert.assertScript
+
+final class Groovy12140 {
+
+    private static final String SCRIPT = '''
+        class W {
+            int num(int... xs) { 42 }
+            int val() { 42 }
+            char ch() { (char) 'a' }
+            boolean flag() { true }
+            long lng() { 42L }
+            byte byt() { (byte) 42 }
+            short shrt() { (short) 42 }
+        }
+        def w = new W()
+        def name = 'val'
+        // dynamic-name calls dispatch reflectively via doMethodInvoke, in both
+        // compilation modes: primitive returns must box through the valueOf
+        // caches for identity (===) parity with directly dispatched calls
+        // (float/double are deliberately not asserted: valueOf caches nothing
+        // for them on any dispatch path)
+        assert w."$name"() === 42
+        assert w."${'ch'}"() === Character.valueOf((char) 'a')
+        assert w."${'flag'}"() === Boolean.TRUE
+        assert w."${'lng'}"() === Long.valueOf(42L)
+        assert w."${'byt'}"() === Byte.valueOf((byte) 42)
+        assert w."${'shrt'}"() === Short.valueOf((short) 42)
+        // varargs methods use the reflective call-site variant under classic
+        assert w.num(1, 2) === 42
+    '''
+
+    @Test
+    void testPrimitiveReturnBoxIdentity() {
+        assertScript SCRIPT
+    }
+
+    @Test
+    void testPrimitiveReturnBoxIdentityClassic() {
+        def config = new CompilerConfiguration()
+        config.optimizationOptions.indy = false
+        new GroovyShell(config).evaluate SCRIPT
+    }
+}