You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2011/04/15 03:07:02 UTC

svn commit: r1092560 - in /tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5: internal/plastic/ plastic/

Author: hlship
Date: Fri Apr 15 01:07:02 2011
New Revision: 1092560

URL: http://svn.apache.org/viewvc?rev=1092560&view=rev
Log:
TAP5-853: Revise and simplify how local variables are referenced, introducing a new type (LocalVariable) and a new interface (LocalVariableCallback)

Added:
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariable.java
      - copied, changed from r1092559, tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariableCallback.java
      - copied, changed from r1092559, tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.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/LocalVariable.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
    tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/InstructionBuilder.java

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=1092560&r1=1092559&r2=1092560&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 Fri Apr 15 01:07:02 2011
@@ -18,6 +18,7 @@ import java.lang.reflect.Method;
 import java.util.HashMap;
 import java.util.Map;
 
+import org.apache.tapestry5.internal.plastic.InstructionBuilderState.LVInfo;
 import org.apache.tapestry5.internal.plastic.asm.Label;
 import org.apache.tapestry5.internal.plastic.asm.MethodVisitor;
 import org.apache.tapestry5.internal.plastic.asm.Opcodes;
@@ -25,6 +26,8 @@ import org.apache.tapestry5.internal.pla
 import org.apache.tapestry5.plastic.Condition;
 import org.apache.tapestry5.plastic.InstructionBuilder;
 import org.apache.tapestry5.plastic.InstructionBuilderCallback;
+import org.apache.tapestry5.plastic.LocalVariable;
+import org.apache.tapestry5.plastic.LocalVariableCallback;
 import org.apache.tapestry5.plastic.MethodDescription;
 import org.apache.tapestry5.plastic.PlasticField;
 import org.apache.tapestry5.plastic.PlasticMethod;
@@ -578,58 +581,39 @@ public class InstructionBuilderImpl exte
         return this;
     }
 
-    public InstructionBuilder startVariable(String name, String type, InstructionBuilderCallback callback)
+    public InstructionBuilder startVariable(String type, final LocalVariableCallback callback)
     {
         check();
 
-        Label start = state.newLabel();
-        Label end = new Label();
+        final LocalVariable var = state.startVariable(type);
 
-        LocalVariable var = new LocalVariable(name, type, state.localIndex++);
-
-        v.visitLocalVariable(name, cache.toDesc(type), null, start, end, var.index);
-
-        LocalVariable prior = state.locals.put(name, var);
-
-        runSubBuilder(callback);
-
-        v.visitLabel(end);
-
-        state.localIndex--;
-
-        // Restore the original variable with this name, probably just null though.
+        runSubBuilder(new InstructionBuilderCallback()
+        {
+            public void doBuild(InstructionBuilder builder)
+            {
+                callback.doBuild(var, builder);
+            }
+        });
 
-        state.locals.put(name, prior);
+        state.stopVariable(var);
 
         return this;
     }
 
-    public InstructionBuilder storeVariable(String name)
+    public InstructionBuilder storeVariable(LocalVariable var)
     {
         check();
 
-        LocalVariable var = state.locals.get(name);
-
-        PrimitiveType type = PrimitiveType.getByName(var.type);
-
-        int opcode = type == null ? ASTORE : type.storeOpcode;
-
-        v.visitVarInsn(opcode, var.index);
+        state.store(var);
 
         return this;
     }
 
-    public InstructionBuilder loadVariable(String name)
+    public InstructionBuilder loadVariable(LocalVariable var)
     {
         check();
 
-        LocalVariable var = state.locals.get(name);
-
-        PrimitiveType type = PrimitiveType.getByName(var.type);
-
-        int opcode = type == null ? ALOAD : type.loadOpcode;
-
-        v.visitVarInsn(opcode, var.index);
+        state.load(var);
 
         return this;
     }
@@ -716,7 +700,6 @@ public class InstructionBuilderImpl exte
 
         runSubBuilder(new InstructionBuilderCallback()
         {
-
             public void doBuild(InstructionBuilder builder)
             {
                 callback.buildBody(builder);
@@ -730,13 +713,13 @@ public class InstructionBuilderImpl exte
         return this;
     }
 
-    public InstructionBuilder increment(String name)
+    public InstructionBuilder increment(LocalVariable variable)
     {
         check();
 
-        LocalVariable var = state.locals.get(name);
+        LVInfo info = state.locals.get(variable);
 
-        v.visitIincInsn(var.index, 1);
+        v.visitIincInsn(info.offset, 1);
 
         return this;
     }
@@ -752,29 +735,27 @@ public class InstructionBuilderImpl exte
 
     public InstructionBuilder iterateArray(final InstructionBuilderCallback callback)
     {
-        final String name = newUniqueLocalVarName("i");
-
-        startVariable(name, "int", new InstructionBuilderCallback()
+        startVariable("int", new LocalVariableCallback()
         {
-            public void doBuild(InstructionBuilder builder)
+            public void doBuild(final LocalVariable indexVariable, InstructionBuilder builder)
             {
-                builder.loadConstant(0).storeVariable(name);
+                builder.loadConstant(0).storeVariable(indexVariable);
 
                 builder.doWhile(Condition.LESS_THAN, new WhileCallback()
                 {
                     public void buildTest(InstructionBuilder builder)
                     {
                         builder.dupe().arrayLength();
-                        builder.loadVariable(name).swap();
+                        builder.loadVariable(indexVariable).swap();
                     }
 
                     public void buildBody(InstructionBuilder builder)
                     {
-                        builder.dupe().loadVariable(name).loadArrayElement();
+                        builder.dupe().loadVariable(indexVariable).loadArrayElement();
 
                         callback.doBuild(builder);
-                        
-                        builder.increment(name);
+
+                        builder.increment(indexVariable);
                     }
                 });
             }
@@ -783,24 +764,6 @@ public class InstructionBuilderImpl exte
         return this;
     }
 
-    private String newUniqueLocalVarName(String base)
-    {
-        if (!state.locals.containsKey(base))
-            return base;
-
-        int index = 0;
-
-        while (true)
-        {
-            String name = base + "_" + index;
-
-            if (!state.locals.containsKey(name))
-                return name;
-
-            index++;
-        }
-    }
-
     private void runSubBuilder(InstructionBuilderCallback callback)
     {
         new InstructionBuilderImpl(state).doCallback(callback);

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=1092560&r1=1092559&r2=1092560&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 Fri Apr 15 01:07:02 2011
@@ -19,6 +19,7 @@ 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.LocalVariable;
 import org.apache.tapestry5.plastic.MethodDescription;
 
 /**
@@ -35,7 +36,26 @@ public class InstructionBuilderState imp
 
     int localIndex;
 
-    final Map<String, LocalVariable> locals = PlasticInternalUtils.newMap();
+    int varSuffix;
+
+    static class LVInfo
+    {
+        final int width, offset, loadOpcode, storeOpcode;
+
+        final Label end;
+
+        public LVInfo(int width, int offset, int loadOpcode, int storeOpcode, Label end)
+        {
+            this.width = width;
+            this.offset = offset;
+            this.loadOpcode = loadOpcode;
+            this.storeOpcode = storeOpcode;
+            this.end = end;
+        }
+    }
+
+    /** Map from LocalVariable to Integer offset. */
+    final Map<LocalVariable, LVInfo> locals = PlasticInternalUtils.newMap();
 
     /** Index for argument (0 is first true argument); allows for double-width primitive types. */
     final int[] argumentIndex;
@@ -74,6 +94,7 @@ public class InstructionBuilderState imp
         localIndex = offset;
     }
 
+    /** Creates a new Label and adds it to the method. */
     Label newLabel()
     {
         Label result = new Label();
@@ -82,4 +103,59 @@ public class InstructionBuilderState imp
 
         return result;
     }
+
+    LocalVariable startVariable(String type)
+    {
+        Label start = newLabel();
+        Label end = new Label();
+
+        PrimitiveType ptype = PrimitiveType.getByName(type);
+
+        int width = (ptype != null && ptype.isWide()) ? 2 : 1;
+
+        int loadOpcode = ptype == null ? ALOAD : ptype.loadOpcode;
+        int storeOpcode = ptype == null ? ASTORE : ptype.storeOpcode;
+
+        LVInfo info = new LVInfo(width, localIndex, loadOpcode, storeOpcode, end);
+
+        localIndex += width;
+
+        LocalVariable var = new LocalVariableImpl(type);
+
+        locals.put(var, info);
+
+        visitor.visitLocalVariable(nextVarName(), nameCache.toDesc(type), null, start, end, info.offset);
+
+        return var;
+    }
+
+    void load(LocalVariable var)
+    {
+        LVInfo info = locals.get(var);
+
+        visitor.visitVarInsn(info.loadOpcode, info.offset);
+    }
+
+    void store(LocalVariable var)
+    {
+        LVInfo info = locals.get(var);
+
+        visitor.visitVarInsn(info.storeOpcode, info.offset);
+    }
+
+    void stopVariable(LocalVariable variable)
+    {
+        LVInfo info = locals.get(variable);
+
+        visitor.visitLabel(info.end);
+
+        locals.remove(variable);
+
+        localIndex -= info.width;
+    }
+
+    private String nextVarName()
+    {
+        return "var" + varSuffix++;
+    }
 }

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java?rev=1092560&r1=1092559&r2=1092560&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java Fri Apr 15 01:07:02 2011
@@ -14,19 +14,19 @@
 
 package org.apache.tapestry5.internal.plastic;
 
-public class LocalVariable
-{
-    final String name;
-
-    final String type;
+import org.apache.tapestry5.plastic.LocalVariable;
 
-    // Adjusted index i.e., 1 + # of args + var index
-    final int index;
+class LocalVariableImpl implements LocalVariable
+{
+    private String type;
 
-    LocalVariable(String name, String type, int index)
+    LocalVariableImpl(String type)
     {
-        this.name = name;
         this.type = type;
-        this.index = index;
+    }
+
+    public String getType()
+    {
+        return type;
     }
 }

Modified: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java?rev=1092560&r1=1092559&r2=1092560&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/PlasticClassImpl.java Fri Apr 15 01:07:02 2011
@@ -43,12 +43,13 @@ import org.apache.tapestry5.plastic.Anno
 import org.apache.tapestry5.plastic.ClassInstantiator;
 import org.apache.tapestry5.plastic.ComputedValue;
 import org.apache.tapestry5.plastic.Condition;
-import org.apache.tapestry5.plastic.WhenCallback;
 import org.apache.tapestry5.plastic.FieldConduit;
 import org.apache.tapestry5.plastic.FieldHandle;
 import org.apache.tapestry5.plastic.InstanceContext;
 import org.apache.tapestry5.plastic.InstructionBuilder;
 import org.apache.tapestry5.plastic.InstructionBuilderCallback;
+import org.apache.tapestry5.plastic.LocalVariable;
+import org.apache.tapestry5.plastic.LocalVariableCallback;
 import org.apache.tapestry5.plastic.MethodAdvice;
 import org.apache.tapestry5.plastic.MethodDescription;
 import org.apache.tapestry5.plastic.MethodHandle;
@@ -1299,11 +1300,11 @@ public class PlasticClassImpl extends Lo
             // and throws no checked exceptions, then the variable actually isn't used. This code
             // should be refactored a bit once there are tests for those cases.
 
-            builder.startVariable("invocation", invocationClassName, new InstructionBuilderCallback()
+            builder.startVariable(invocationClassName, new LocalVariableCallback()
             {
-                public void doBuild(InstructionBuilder builder)
+                public void doBuild(final LocalVariable invocation, InstructionBuilder builder)
                 {
-                    builder.dupe().storeVariable("invocation");
+                    builder.dupe().storeVariable(invocation);
 
                     builder.invoke(AbstractMethodInvocation.class, MethodInvocation.class, "proceed");
 
@@ -1315,7 +1316,7 @@ public class PlasticClassImpl extends Lo
                         {
                             public void doBuild(InstructionBuilder builder)
                             {
-                                builder.loadVariable("invocation").loadTypeConstant(Exception.class);
+                                builder.loadVariable(invocation).loadTypeConstant(Exception.class);
                                 builder.invokeVirtual(invocationClassName, Throwable.class.getName(),
                                         "getCheckedException", Class.class.getName());
                                 builder.throwException();
@@ -1324,7 +1325,7 @@ public class PlasticClassImpl extends Lo
                     }
 
                     if (!isVoid)
-                        builder.loadVariable("invocation").getField(invocationClassName, RETURN_VALUE,
+                        builder.loadVariable(invocation).getField(invocationClassName, RETURN_VALUE,
                                 description.returnType);
 
                     builder.returnResult();

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=1092560&r1=1092559&r2=1092560&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 Fri Apr 15 01:07:02 2011
@@ -25,9 +25,10 @@ import java.lang.reflect.Method;
  * Most methods return the same instance of InstructionBuilder, allowing for a "fluid" API.
  * <p>
  * More complex functionality, such as {@linkplain #startTryCatch(InstructionBuilderCallback, TryCatchCallback)
- * try/catch blocks}, is more DSL (domain specific language) like, and is based on callbacks. This looks better in
+ * try/catch blocks}, is more like a DSL (domain specific language), and is based on callbacks. This looks better in
  * Groovy and will be more reasonable once JDK 1.8 closures are available; in the meantime, it means some deeply nested
- * inner classes, but helps ensure that correct bytecode is generated.
+ * inner classes, but helps ensure that correct bytecode is generated and helps to limit the amount of bookkeeping is
+ * necessary on the part of coce using InstructionBuilder.
  */
 @SuppressWarnings("rawtypes")
 public interface InstructionBuilder
@@ -377,28 +378,26 @@ public interface InstructionBuilder
     /**
      * Starts a block where the given name is active.
      * 
-     * @param name
-     *            name of local variable
      * @param type
      *            type of local variable
      * @param callback
      *            generates code used when variable is in effect
      */
-    InstructionBuilder startVariable(String name, String type, InstructionBuilderCallback callback);
+    InstructionBuilder startVariable(String type, LocalVariableCallback callback);
 
     /**
      * Stores the value on top of the stack to a local variable (previously defined by
      * {@link #startVariable(String, String, InstructionBuilderCallback)}.
      */
     @Opcodes("ASTORE, ISTORE, LSTORE, FSTORE, DSTORE")
-    InstructionBuilder storeVariable(String name);
+    InstructionBuilder storeVariable(LocalVariable variable);
 
     /**
-     * Loads a value from a local variable and pushes it onto the stack. The variable must have been previously defined
-     * by {@link #startVariable(String, String, InstructionBuilderCallback)}.
+     * Loads a value from a local variable and pushes it onto the stack. The is defined by
+     * {@link #startVariable(String, InstructionBuilderCallback)} and made available via {@link LocalVariableCallback}.
      */
     @Opcodes("ALOAD, ILOAD, LLOAD, FLOAD, DLOAD")
-    InstructionBuilder loadVariable(String name);
+    InstructionBuilder loadVariable(LocalVariable variable);
 
     /**
      * Executes conditional code based on a {@link Condition}. The testing opcodes all pop
@@ -461,7 +460,7 @@ public interface InstructionBuilder
      * @return this builder
      */
     @Opcodes("IINC")
-    InstructionBuilder increment(String name);
+    InstructionBuilder increment(LocalVariable variable);
 
     /** Expects the top object on the stack to be an array. Replaces it with the length of that array. */
     @Opcodes("ARRAYLENGTH")

Copied: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariable.java (from r1092559, tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java)
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariable.java?p2=tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariable.java&p1=tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java&r1=1092559&r2=1092560&rev=1092560&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariable.java Fri Apr 15 01:07:02 2011
@@ -12,21 +12,13 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry5.internal.plastic;
+package org.apache.tapestry5.plastic;
 
-public class LocalVariable
+/**
+ * Variable available inside a {@link LocalVariableCallback}.
+ */
+public interface LocalVariable
 {
-    final String name;
-
-    final String type;
-
-    // Adjusted index i.e., 1 + # of args + var index
-    final int index;
-
-    LocalVariable(String name, String type, int index)
-    {
-        this.name = name;
-        this.type = type;
-        this.index = index;
-    }
+    /** Returns the Java type of the variable. */
+    String getType();
 }

Copied: tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariableCallback.java (from r1092559, tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java)
URL: http://svn.apache.org/viewvc/tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariableCallback.java?p2=tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariableCallback.java&p1=tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java&r1=1092559&r2=1092560&rev=1092560&view=diff
==============================================================================
--- tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/internal/plastic/LocalVariable.java (original)
+++ tapestry/tapestry5/trunk/plastic/src/main/java/org/apache/tapestry5/plastic/LocalVariableCallback.java Fri Apr 15 01:07:02 2011
@@ -12,21 +12,20 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package org.apache.tapestry5.internal.plastic;
+package org.apache.tapestry5.plastic;
 
-public class LocalVariable
+/**
+ * A callback for generating code where a local variable is in effect.
+ */
+public interface LocalVariableCallback
 {
-    final String name;
-
-    final String type;
-
-    // Adjusted index i.e., 1 + # of args + var index
-    final int index;
-
-    LocalVariable(String name, String type, int index)
-    {
-        this.name = name;
-        this.type = type;
-        this.index = index;
-    }
+    /**
+     * Callback to generate code with a local variable
+     * 
+     * @param variable
+     *            the variable that is available within the block
+     * @param builder
+     *            builder the block where the variable is active
+     */
+    void doBuild(LocalVariable variable, InstructionBuilder builder);
 }