Refactoring: extracted common logic of working with local variable table into getValuesStoredOrLoadedToVariable

compiler/backend/src/org/jetbrains/jet/codegen/optimization/boxing/RedundantBoxingMethodTransformer.java

@@ -16,6 +16,8 @@
 
 package org.jetbrains.jet.codegen.optimization.boxing;
 
+import com.google.common.base.Predicate;
+import com.google.common.collect.Collections2;
 import com.intellij.openapi.util.Pair;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.jet.codegen.optimization.OptimizationUtils;
@@ -28,8 +30,7 @@ import org.jetbrains.org.objectweb.asm.tree.analysis.Analyzer;
 import org.jetbrains.org.objectweb.asm.tree.analysis.BasicValue;
 import org.jetbrains.org.objectweb.asm.tree.analysis.Frame;
 
-import java.util.HashSet;
+import java.util.*;
-import java.util.Set;
 
 public class RedundantBoxingMethodTransformer extends MethodTransformer {
     public RedundantBoxingMethodTransformer(MethodTransformer methodTransformer) {
@@ -64,32 +65,9 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer {
                 continue;
             }
 
-            adaptLocalVariableTableEntryForBoxedValues(node, frames, localVariableNode);
+            for (BasicValue value : getValuesStoredOrLoadedToVariable(localVariableNode, node, frames)) {
-        }
+                if (value == null || !(value instanceof BoxedBasicValue) || !((BoxedBasicValue) value).isSafeToRemove()) continue;
-    }
+                localVariableNode.desc = ((BoxedBasicValue) value).getPrimitiveType().getDescriptor();
-
-    private static void adaptLocalVariableTableEntryForBoxedValues(
-            @NotNull MethodNode node, @NotNull Frame<BasicValue>[] frames, @NotNull LocalVariableNode localVariableNode
-    ) {
-        InsnList insnList = node.instructions;
-        int from = insnList.indexOf(localVariableNode.start) + 1;
-        int to = insnList.indexOf(localVariableNode.end) - 1;
-
-        for (int i = from; i <= to; i++) {
-            AbstractInsnNode insn = insnList.get(i);
-            if (insn.getOpcode() == Opcodes.ASTORE && ((VarInsnNode) insn).var == localVariableNode.index) {
-                if (frames[i] == null) {
-                    return;
-                }
-
-                BasicValue top = frames[i].getStack(frames[i].getStackSize() - 1);
-                if (!(top instanceof BoxedBasicValue) || !((BoxedBasicValue) top).isSafeToRemove()) {
-                    return;
-                }
-
-                localVariableNode.desc = ((BoxedBasicValue) top).getPrimitiveType().getDescriptor();
-
-                return;
             }
         }
     }
@@ -109,7 +87,6 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer {
             @NotNull MethodNode node,
             @NotNull Frame<BasicValue>[] frames
     ) {
-        InsnList insnList = node.instructions;
         boolean needToRepeat = false;
 
         for (LocalVariableNode localVariableNode : node.localVariables) {
@@ -117,86 +94,74 @@ public class RedundantBoxingMethodTransformer extends MethodTransformer {
                 continue;
             }
 
-            int index = localVariableNode.index;
+            List<BasicValue> usedValues = getValuesStoredOrLoadedToVariable(localVariableNode, node, frames);
-            int from = insnList.indexOf(localVariableNode.start) + 1;
-            int to = insnList.indexOf(localVariableNode.end) - 1;
 
-            if (isThereUnsafeStoreInstruction(insnList, frames, from, to, index)) {
+            Collection<BasicValue> boxed = Collections2.filter(usedValues, new Predicate<BasicValue>() {
-                needToRepeat |= markAllBoxedValuesStoredAsUnsafeToRemove(insnList, frames, values, from, to, index);
+                @Override
+                public boolean apply(BasicValue input) {
+                    return input instanceof BoxedBasicValue;
+                }
+            });
+
+            if (boxed.isEmpty()) continue;
+
+            final BoxedBasicValue firstBoxed = (BoxedBasicValue) boxed.iterator().next();
+
+            if (!Collections2.filter(usedValues, new Predicate<BasicValue>() {
+                @Override
+                public boolean apply(BasicValue input) {
+                    return input == null ||
+                           !(input instanceof BoxedBasicValue) ||
+                           !((BoxedBasicValue) input).isSafeToRemove() ||
+                           !((BoxedBasicValue) input).getPrimitiveType().equals(firstBoxed.getPrimitiveType());
+                }
+            }).isEmpty()) {
+                for (BasicValue value : usedValues) {
+                    if (value instanceof BoxedBasicValue && ((BoxedBasicValue) value).isSafeToRemove()) {
+                        values.remove((BoxedBasicValue) value);
+                        needToRepeat = true;
+                    }
+                }
             }
         }
 
         return needToRepeat;
     }
 
-    /**
+    @NotNull
-     * Check if there are unsafe ASTORE instructions, that put into var something but boxed values of the same type
+    private static List<BasicValue> getValuesStoredOrLoadedToVariable(
-     *
+            @NotNull LocalVariableNode localVariableNode,
-     * @param insnList
+            @NotNull MethodNode node,
-     * @param frames
+            @NotNull Frame<BasicValue>[] frames
-     * @param from
-     * @param to
-     * @param varIndex
-     * @return
-     */
-    private static boolean isThereUnsafeStoreInstruction(
-            @NotNull InsnList insnList,
-            @NotNull Frame<BasicValue>[] frames,
-            int from, int to, int varIndex
     ) {
-        Type usedAsType = null;
+        List<BasicValue> values = new ArrayList<BasicValue>();
+        InsnList insnList = node.instructions;
+        int from = insnList.indexOf(localVariableNode.start) + 1;
+        int to = insnList.indexOf(localVariableNode.end) - 1;
 
         for (int i = from; i <= to; i++) {
-            AbstractInsnNode insn = insnList.get(i); //TODO: handle exception?
+            if (i < 0 || i >= insnList.size()) continue;
-            if (insn.getOpcode() == Opcodes.ASTORE && ((VarInsnNode) insn).var == varIndex) {
-                if (frames[i] == null) {
-                    return true;
-                }
 
-                BasicValue top = frames[i].getStack(frames[i].getStackSize() - 1);
-                if (!(top instanceof BoxedBasicValue) || !((BoxedBasicValue) top).isSafeToRemove()) {
-                    return true;
-                }
-
-                if (usedAsType == null) {
-                    usedAsType = ((BoxedBasicValue) top).getPrimitiveType();
-                }
-
-                if (!usedAsType.equals(((BoxedBasicValue) top).getPrimitiveType())) {
-                    return true;
-                }
-            }
-        }
-
-        return false;
-    }
-
-    private static boolean markAllBoxedValuesStoredAsUnsafeToRemove(
-            @NotNull InsnList insnList,
-            @NotNull Frame<BasicValue>[] frames,
-            @NotNull RedundantBoxedValuesCollection values,
-            int from, int to, int varIndex
-    ) {
-        boolean wasChanges = false;
-
-        for (int i = from; i <= to; i++) {
             AbstractInsnNode insn = insnList.get(i);
-            if (insn.getOpcode() == Opcodes.ASTORE && ((VarInsnNode) insn).var == varIndex) {
+            if ((insn.getOpcode() == Opcodes.ASTORE || insn.getOpcode() == Opcodes.ALOAD) &&
+                ((VarInsnNode) insn).var == localVariableNode.index) {
+
+                // frames[i] can be null in case of exception handlers
                 if (frames[i] == null) {
+                    values.add(null);
                     continue;
                 }
 
-                BasicValue top = frames[i].getStack(frames[i].getStackSize() - 1);
+                if (insn.getOpcode() == Opcodes.ASTORE) {
-                if (!(top instanceof BoxedBasicValue) || !((BoxedBasicValue) top).isSafeToRemove()) {
+                    values.add(frames[i].getStack(frames[i].getStackSize() - 1));
-                    continue;
+                }
+                else {
+                    values.add(frames[i].getLocal(((VarInsnNode) insn).var));
                 }
-
-                wasChanges |= true;
-                values.remove((BoxedBasicValue) top);
             }
         }
 
-        return wasChanges;
+        return values;
     }
 
     @NotNull