Unbreak name/hashcode/equal for float/fixnum/etc. tmp vars

* Closure, float, boolean, fixnum were all using "%v_" prefixes. * Some code refactoring somewhere broke this. * Since closure and non-clusre tmp vars aren't used in the same scope, this doesn't cause hashing conflicts. But, with unboxing, this bug got exposed since these types co-exist within the same scope. * IR output now also use the "%cl_1_5" form for local variables in closures which makes for easier debugging. * Also used a base equals implementation in TemporaryVariable which mirrors the hashCode() implementation there.
jruby · Jan 10, 2015 · 0c11cad · 0c11cad
1 parent 2dd881b
commit 0c11cad
Showing 6 changed files with 18 additions and 22 deletions.
diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryBooleanVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryBooleanVariable.java
@@ -35,8 +35,9 @@
  * Represents a temporary variable for an unboxed Boolean operand.
  */
 public class TemporaryBooleanVariable extends TemporaryLocalVariable {
+    public static final String PREFIX = "%b_";
     public TemporaryBooleanVariable(int offset) {
-        super(offset);
+        super(PREFIX+offset, offset);
     }
 
     @Override
@@ -46,7 +47,7 @@ public TemporaryVariableType getType() {
 
     @Override
     public String getPrefix() {
-        return "%b_";
+        return PREFIX;
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryClosureVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryClosureVariable.java
@@ -6,7 +6,7 @@ public class TemporaryClosureVariable extends TemporaryLocalVariable {
     private final int closureId;
 
     public TemporaryClosureVariable(int closureId, int offset) {
-        super(offset);
+        super("%cl_" + closureId + "_" + offset, offset);
 
         this.closureId = closureId;
     }

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryFixnumVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryFixnumVariable.java
@@ -35,8 +35,9 @@
  * Represents a temporary variable for an unboxed Float operand.
  */
 public class TemporaryFixnumVariable extends TemporaryLocalVariable {
+    public static final String PREFIX = "%i_";
     public TemporaryFixnumVariable(int offset) {
-        super(offset);
+        super(PREFIX+offset, offset);
     }
 
     @Override
@@ -46,7 +47,7 @@ public TemporaryVariableType getType() {
 
     @Override
     public String getPrefix() {
-        return "%i_";
+        return PREFIX;
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryFloatVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryFloatVariable.java
@@ -35,8 +35,9 @@
  * Represents a temporary variable for an unboxed Float operand.
  */
 public class TemporaryFloatVariable extends TemporaryLocalVariable {
+    public static final String PREFIX = "%f_";
     public TemporaryFloatVariable(int offset) {
-        super(offset);
+        super(PREFIX+offset, offset);
     }
 
     @Override
@@ -46,7 +47,7 @@ public TemporaryVariableType getType() {
 
     @Override
     public String getPrefix() {
-        return "%f_";
+        return PREFIX;
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryLocalVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryLocalVariable.java
@@ -34,15 +34,8 @@ public TemporaryVariableType getType() {
         return TemporaryVariableType.LOCAL;
     }
 
-    @Override
-    public boolean equals(Object other) {
-        if (other == null || !(other instanceof TemporaryLocalVariable)) return false;
-
-        return super.equals(other) && getOffset() == ((TemporaryLocalVariable) other).getOffset();
-    }
-
     public String getPrefix() {
-        return "%v_";
+        return PREFIX;
     }
 
     @Override

diff --git a/core/src/main/java/org/jruby/ir/operands/TemporaryVariable.java b/core/src/main/java/org/jruby/ir/operands/TemporaryVariable.java
@@ -16,13 +16,6 @@ public TemporaryVariable(String name) {
      */
     public abstract TemporaryVariableType getType();
 
-    @Override
-    public boolean equals(Object other) {
-        if (other == null || !(other instanceof TemporaryVariable)) return false;
-
-        return getType() == ((TemporaryVariable) other).getType();
-    }
-
     public String getName() {
         return name;
     }
@@ -32,6 +25,13 @@ public int hashCode() {
         return getName().hashCode();
     }
 
+    @Override
+    public boolean equals(Object other) {
+        if (other == null || !(other instanceof TemporaryVariable)) return false;
+
+        return ((TemporaryVariable)other).getName().equals(getName());
+    }
+
     @Override
     public int compareTo(Object other) {
         if (!(other instanceof TemporaryVariable)) return 0;