Add coercion support for covariant parameter types in injected callbacks

src/main/java/org/spongepowered/asm/mixin/injection/Coerce.java

@@ -0,0 +1,49 @@
+/*
+ * This file is part of Sponge, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.asm.mixin.injection;
+
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Target;
+
+
+/**
+ * Indicates that a local capture injector should coerce top-level primitive
+ * types to covariant types.
+ * 
+ * <p>During LVT generation it is not always possible to inflect the exact local
+ * type for types represented internally as integers, for example booleans and
+ * shorts. However adding a surrogate for these cases is overkill when the type
+ * is known for certain by the injector. Since the bytecode for all types stored
+ * as integer interally will be valid, we can force the local type to any
+ * covariant type as long as we know this in advance.</p>
+ * 
+ * <p>This annotation allows a covariant type parameter to be marked, and thus
+ * coerced to the correct type when the LVT generation would otherwise mark the
+ * type as invalid.</p>
+ */
+@Target({ ElementType.PARAMETER })
+public @interface Coerce {
+
+}

src/main/java/org/spongepowered/asm/mixin/injection/callback/CallbackInjector.java

@@ -31,17 +31,8 @@
 import org.apache.logging.log4j.Logger;
 import org.spongepowered.asm.lib.Opcodes;
 import org.spongepowered.asm.lib.Type;
-import org.spongepowered.asm.lib.tree.AbstractInsnNode;
-import org.spongepowered.asm.lib.tree.InsnList;
-import org.spongepowered.asm.lib.tree.InsnNode;
-import org.spongepowered.asm.lib.tree.JumpInsnNode;
-import org.spongepowered.asm.lib.tree.LabelNode;
-import org.spongepowered.asm.lib.tree.LdcInsnNode;
-import org.spongepowered.asm.lib.tree.LocalVariableNode;
-import org.spongepowered.asm.lib.tree.MethodInsnNode;
-import org.spongepowered.asm.lib.tree.MethodNode;
-import org.spongepowered.asm.lib.tree.TypeInsnNode;
-import org.spongepowered.asm.lib.tree.VarInsnNode;
+import org.spongepowered.asm.lib.tree.*;
+import org.spongepowered.asm.mixin.injection.Coerce;
 import org.spongepowered.asm.mixin.injection.InjectionError;
 import org.spongepowered.asm.mixin.injection.InjectionPoint;
 import org.spongepowered.asm.mixin.injection.InvalidInjectionException;
@@ -66,6 +57,11 @@ public class CallbackInjector extends Injector {
      */
     private class Callback extends InsnList {
 
+        /**
+         * Handler method
+         */
+        private final MethodNode handler;
+
         /**
          * Target method handle
          */
@@ -145,7 +141,8 @@ private class Callback extends InsnList {
          */
         final int marshallVar;
 
-        Callback(Target target, final AbstractInsnNode node, final LocalVariableNode[] locals, boolean captureLocals) {
+        Callback(MethodNode handler, Target target, final AbstractInsnNode node, final LocalVariableNode[] locals, boolean captureLocals) {
+            this.handler = handler;
             this.target = target;
             this.node = node;
             this.locals = locals;
@@ -170,7 +167,7 @@ private class Callback extends InsnList {
             }
 
             // Calc number of args for the handler method, additional 1 is to ignore the CallbackInfo arg
-            this.extraArgs = Math.max(0, ASMHelper.getFirstNonArgLocalIndex(CallbackInjector.this.getMethod()) - (this.frameSize + 1));
+            this.extraArgs = Math.max(0, ASMHelper.getFirstNonArgLocalIndex(this.handler) - (this.frameSize + 1));
             this.argNames = argNames != null ? argNames.toArray(new String[argNames.size()]) : null;
             this.canCaptureLocals = captureLocals && locals != null && locals.length > this.frameSize;
             this.isAtReturn = this.node instanceof InsnNode && this.isValueReturnOpcode(this.node.getOpcode());
@@ -195,6 +192,10 @@ private boolean isValueReturnOpcode(int opcode) {
         String getDescriptor() {
             return this.canCaptureLocals ? this.descl : this.desc;
         }
+
+        String getDescriptorWithAllLocals() {
+            return this.target.getCallbackDescriptor(true, this.localTypes, this.target.arguments, this.frameSize, Short.MAX_VALUE);
+        }
 
         /**
          * Add an instruction to this callback and increment the appropriate
@@ -218,6 +219,46 @@ void inject() {
             this.target.insns.insertBefore(this.node, this);
             this.target.addToStack(Math.max(this.invoke, this.ctor));
         }
+
+        boolean checkDescriptor(String desc) {
+            if (this.getDescriptor().equals(desc)) {
+                return true; // Descriptor matches exactly, this is good
+            }
+
+            if (this.extraArgs > 0) {
+                Type[] inTypes = Type.getArgumentTypes(desc);
+                Type[] myTypes = Type.getArgumentTypes(this.descl);
+
+                if (inTypes.length != myTypes.length) {
+                    return false;
+                }
+
+                for (int arg = this.frameSize + 1; arg < myTypes.length; arg++) {
+                    Type type = inTypes[arg];
+                    if (type.equals(myTypes[arg])) {
+                        continue; // Type matches
+                    }
+
+                    if (type.getSort() >= Type.ARRAY) {
+                        return false; // Reference types must match exactly
+                    }
+
+                    AnnotationNode coerce = ASMHelper.getInvisibleParameterAnnotation(this.handler, Coerce.class, arg);
+                    if (coerce == null) {
+                        return false; // No @Coerce specified, types must match
+                    }
+
+                    boolean canCoerce = CallbackInjector.canCoerce(inTypes[arg], myTypes[arg]);
+                    if (!canCoerce) {
+                        return false; // Can't coerce source type to local type, give up
+                    }
+                }
+
+                return true;
+            }
+
+            return false;
+        }
     }
 
     private final Logger logger = LogManager.getLogger("mixin");
@@ -280,7 +321,7 @@ protected void inject(Target target, AbstractInsnNode node) {
             locals = Locals.getLocalsAt(this.classNode, target.method, node);
         }
 
-        this.inject(new Callback(target, node, locals, this.localCapture.isCaptureLocals()));
+        this.inject(new Callback(this.methodNode, target, node, locals, this.localCapture.isCaptureLocals()));
     }
 
     /**
@@ -299,7 +340,7 @@ private void inject(final Callback callback) {
         // is invalid and we have to generate an error handler stub method
         MethodNode callbackMethod = this.methodNode;
 
-        if (!callback.getDescriptor().equals(this.methodNode.desc)) {
+        if (!callback.checkDescriptor(this.methodNode.desc)) {
             if (this.info.getTargets().size() != 0) {
                 return; // Look for a match in other targets before failing
             }
@@ -337,7 +378,7 @@ private void inject(final Callback callback) {
                         "Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfo;",
                         "Lorg/spongepowered/asm/mixin/injection/callback/CallbackInfoReturnable;");
 
-                if (callback.getDescriptor().equals(returnableSig)) {
+                if (callback.checkDescriptor(returnableSig)) {
                     // Switching out CallbackInfo for CallbackInfoReturnable
                     // worked, so notify the user that they done derped
                     throw new InvalidInjectionException(this.info, "Invalid descriptor on callback: CallbackInfoReturnable is required!");  
@@ -398,7 +439,7 @@ private MethodNode generateErrorMethod(Callback callback, String errorClass, Str
      * @param callback callback handle
      */
     private void printLocals(final Callback callback) {
-        Type[] args = Type.getArgumentTypes(callback.descl);
+        Type[] args = Type.getArgumentTypes(callback.getDescriptorWithAllLocals());
         SignaturePrinter methodSig = new SignaturePrinter(callback.target.method, callback.argNames);
         SignaturePrinter handlerSig = new SignaturePrinter(this.methodNode.name, callback.target.returnType, args, callback.argNames);
         handlerSig.setModifiers(this.methodNode);
@@ -412,7 +453,7 @@ private void printLocals(final Callback callback) {
         printer.add("%20s : %s %s", "Instruction", callback.node.getClass().getSimpleName(), ASMHelper.getOpcodeName(callback.node.getOpcode()));
         printer.hr();
         if (callback.locals.length > callback.frameSize) {
-            printer.add("  LOCAL                  TYPE  NAME");
+            printer.add("  %s  %20s  %s", "LOCAL", "TYPE", "NAME");
             for (int l = 0; l < callback.locals.length; l++) {
                 String marker = l == callback.frameSize ? ">" : " ";
                 if (callback.locals[l] != null) {
@@ -556,15 +597,6 @@ protected void injectReturnCode(final Callback callback) {
     protected boolean isStatic() {
         return this.isStatic;
     }
-
-    /**
-     * Explicit to avoid creation of synthetic accessor
-     * 
-     * @return the callback method
-     */
-    protected MethodNode getMethod() {
-        return this.methodNode;
-    }
 
     private static List<String> summariseLocals(String desc, int pos) {
         return CallbackInjector.summariseLocals(Type.getArgumentTypes(desc), pos);
@@ -581,4 +613,21 @@ private static List<String> summariseLocals(Type[] locals, int pos) {
         }
         return list;
     }
+
+    public static boolean canCoerce(Type from, Type to) {
+        return CallbackInjector.canCoerce(from.getDescriptor(), to.getDescriptor());
+    }
+
+    public static boolean canCoerce(String from, String to) {
+        if (from.length() > 1 || to.length() > 1) {
+            return false;
+        }
+
+        return CallbackInjector.canCoerce(from.charAt(0), to.charAt(0));
+    }
+
+    public static boolean canCoerce(char from, char to) {
+        return to == 'I' && "IBSCZ".indexOf(from) > -1;
+    }
+
 }

src/main/java/org/spongepowered/asm/mixin/injection/struct/Target.java

@@ -227,7 +227,7 @@ private int[] calcArgIndices(int local) {
      * @return generated descriptor
      */
     public String getCallbackDescriptor(final Type[] locals, Type[] argumentTypes) {
-        return this.getCallbackDescriptor(false, locals, argumentTypes, 0, Integer.MAX_VALUE);
+        return this.getCallbackDescriptor(false, locals, argumentTypes, 0, Short.MAX_VALUE);
     }
 
     /**

src/main/java/org/spongepowered/asm/util/ASMHelper.java

@@ -606,6 +606,49 @@ public static AnnotationNode getInvisibleAnnotation(ClassNode classNode, Class<?
         return ASMHelper.getAnnotation(classNode.invisibleAnnotations, Type.getDescriptor(annotationClass));
     }
 
+    /**
+     * Get a runtime-visible parameter annotation of the specified class from
+     * the supplied method node
+     *
+     * @param method Source method
+     * @param annotationClass Type of annotation to search for
+     * @param paramIndex Index of the parameter to fetch annotation for
+     * @return the annotation, or null if not present
+     */
+    public static AnnotationNode getVisibleParameterAnnotation(MethodNode method, Class<? extends Annotation> annotationClass, int paramIndex) {
+        return ASMHelper.getParameterAnnotation(method.visibleParameterAnnotations, Type.getDescriptor(annotationClass), paramIndex);
+    }
+
+    /**
+     * Get an invisible parameter annotation of the specified class from the
+     * supplied method node
+     *
+     * @param method Source method
+     * @param annotationClass Type of annotation to search for
+     * @param paramIndex Index of the parameter to fetch annotation for
+     * @return the annotation, or null if not present
+     */
+    public static AnnotationNode getInvisibleParameterAnnotation(MethodNode method, Class<? extends Annotation> annotationClass, int paramIndex) {
+        return ASMHelper.getParameterAnnotation(method.invisibleParameterAnnotations, Type.getDescriptor(annotationClass), paramIndex);
+    }
+
+    /**
+     * Get a parameter annotation of the specified class from the supplied
+     * method node
+     *
+     * @param parameterAnnotations Annotations for the parameter
+     * @param annotationType Type of annotation to search for
+     * @param paramIndex Index of the parameter to fetch annotation for
+     * @return the annotation, or null if not present
+     */
+    public static AnnotationNode getParameterAnnotation(List<AnnotationNode>[] parameterAnnotations, String annotationType, int paramIndex) {
+        if (parameterAnnotations == null || paramIndex < 0 || paramIndex >= parameterAnnotations.length) {
+            return null;
+        }
+
+        return ASMHelper.getAnnotation(parameterAnnotations[paramIndex], annotationType);
+    }
+
     /**
      * Search for and return an annotation node matching the specified type
      * within the supplied