You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tapestry.apache.org by hl...@apache.org on 2011/04/06 21:12:25 UTC
svn commit: r1089588 - in /tapestry/tapestry5/trunk/plastic/src:
main/java/org/apache/tapestry5/internal/plastic/
main/java/org/apache/tapestry5/plastic/
test/groovy/org/apache/tapestry5/plastic/ test/java/testsubjects/
Author: hlship
Date: Wed Apr 6 19:12:25 2011
New Revision: 1089588
URL: http://svn.apache.org/viewvc?rev=1089588&view=rev
Log:
TAP5-843: Handle wide primitive argument types correctly
(all plastic and ioc tests now pass)
Added:
tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/WidePrimitives.java
Modified:
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderImpl.java
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderState.java
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PrimitiveType.java
tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java
tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderImpl.java?rev=1089588&r1=1089587&r2=1089588&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderImpl.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderImpl.java Wed Apr 6 19:12:25 2011
@@ -118,9 +118,7 @@ public class InstructionBuilderImpl exte
int opcode = type == null ? ALOAD : type.loadOpcode;
- // TODO: May need to adjust offset for static (0) vs. instance (1) method.
-
- v.visitVarInsn(opcode, index + 1);
+ v.visitVarInsn(state.argumentLoadOpcode[index], state.argumentIndex[index]);
return this;
}
Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderState.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderState.java?rev=1089588&r1=1089587&r2=1089588&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderState.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/InstructionBuilderState.java Wed Apr 6 19:12:25 2011
@@ -18,13 +18,14 @@ import java.util.Map;
import org.apache.tapestry5.internal.plastic.asm.Label;
import org.apache.tapestry5.internal.plastic.asm.MethodVisitor;
+import org.apache.tapestry5.internal.plastic.asm.Opcodes;
import org.apache.tapestry5.plastic.MethodDescription;
/**
* Stores information about the method whose instructions are being constructed, to make it easier
* to share data across multiple instances.
*/
-public class InstructionBuilderState
+public class InstructionBuilderState implements Opcodes
{
final MethodDescription description;
@@ -36,6 +37,12 @@ public class InstructionBuilderState
final Map<String, LocalVariable> locals = PlasticInternalUtils.newMap();
+ /** Index for argument (0 is first true argument); allows for double-width primitive types. */
+ final int[] argumentIndex;
+
+ /** Opcode used to load argument (0 is first true argument). */
+ final int[] argumentLoadOpcode;
+
protected InstructionBuilderState(MethodDescription description, MethodVisitor visitor, NameCache nameCache)
{
this.description = description;
@@ -44,7 +51,27 @@ public class InstructionBuilderState
// TODO: Account for static methods?
- localIndex = 1 + description.argumentTypes.length;
+ int argCount = description.argumentTypes.length;
+
+ argumentIndex = new int[argCount];
+ argumentLoadOpcode = new int[argCount];
+
+ // first argument index is for "this"
+
+ int offset = 1;
+
+ for (int i = 0; i < argCount; i++)
+ {
+ PrimitiveType type = PrimitiveType.getByName(description.argumentTypes[i]);
+
+ argumentIndex[i] = offset++;
+ argumentLoadOpcode[i] = type == null ? ALOAD : type.loadOpcode;
+
+ if (type != null && type.isWide())
+ offset++;
+ }
+
+ localIndex = offset;
}
Label newLabel()
Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PrimitiveType.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PrimitiveType.java?rev=1089588&r1=1089587&r2=1089588&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PrimitiveType.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PrimitiveType.java Wed Apr 6 19:12:25 2011
@@ -103,6 +103,11 @@ public enum PrimitiveType implements Opc
}
}
+ public boolean isWide()
+ {
+ return this == LONG || this == DOUBLE;
+ }
+
/**
* Returns the primitive type matching the given type name or null for a non-primitive type (an array type,
* or an class name).
Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java?rev=1089588&r1=1089587&r2=1089588&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java Wed Apr 6 19:12:25 2011
@@ -47,7 +47,9 @@ public interface InstructionBuilder
InstructionBuilder loadNull();
/**
- * Loads an argument onto the stack, using the opcode appropriate to the argument's type.
+ * Loads an argument onto the stack, using the opcode appropriate to the argument's type. In addition
+ * this automatically adjusts for arguments of primitive type long or double (which take up two
+ * local variable indexes, rather than one as for all other types)
*
* @param index
* to argument (0 is the first argument, not this)
Modified: tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy?rev=1089588&r1=1089587&r2=1089588&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy (original)
+++ tapestry/tapestry5/trunk/plastic/src/test/groovy/org/apache/tapestry5/plastic/MethodAdviceTests.groovy Wed Apr 6 19:12:25 2011
@@ -99,4 +99,24 @@ class MethodAdviceTests extends Abstract
thrown(SQLException)
}
+
+ /**
+ * This is important because each double/long takes up two local variable slots.
+ *
+ * @return
+ */
+ def "method with long and double parameters"() {
+ setup:
+
+ def mgr = createMgr({ PlasticClass pc ->
+ findMethod(pc, "doMath").addAdvice(new NoopAdvice())
+ } as PlasticClassTransformer)
+
+ def o = mgr.getClassInstantiator("testsubjects.WidePrimitives").newInstance()
+
+ expect:
+ "The interceptor builds proper bytecode to pass the values through"
+
+ o.doMath(2l, 4.0d, 5, 6l) == 38d
+ }
}
Added: tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/WidePrimitives.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/WidePrimitives.java?rev=1089588&view=auto
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/WidePrimitives.java (added)
+++ tapestry/tapestry5/trunk/plastic/src/test/java/testsubjects/WidePrimitives.java Wed Apr 6 19:12:25 2011
@@ -0,0 +1,9 @@
+package testsubjects;
+
+public class WidePrimitives
+{
+ public double doMath(long first, double second, int third, long fourth)
+ {
+ return first * second + third * fourth;
+ }
+}