Skip to content

Commit c3c5131

Browse files
committed
[GR-29744] Fix ReferenceEqualNode to never compare Boolean by identity
* Previously it could be an issue if an instance of java.lang.Boolean is created which is different than Boolean.TRUE and Boolean.FALSE. * Simplify by removing the special RubyDynamicObject case. * The fallback guard is really complicated, let the DSL generate it, it does not seem possible to be much better anyway.
1 parent bcb6846 commit c3c5131

File tree

2 files changed

+10
-45
lines changed

2 files changed

+10
-45
lines changed

src/main/java/org/truffleruby/core/basicobject/BasicObjectNodes.java

Lines changed: 8 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import com.oracle.truffle.api.RootCallTarget;
1313
import com.oracle.truffle.api.dsl.CachedLanguage;
14+
import com.oracle.truffle.api.dsl.Fallback;
1415
import com.oracle.truffle.api.object.Shape;
1516
import org.truffleruby.Layouts;
1617
import org.truffleruby.RubyContext;
@@ -113,6 +114,9 @@ protected boolean equal(VirtualFrame frame, Object a, Object b) {
113114

114115
}
115116

117+
/** This node is not trivial because primitives must be compared by value and never by identity. Also, this node
118+
* must consider (byte) n and (short) n and (int) n and (long) n equal, as well as (float) n and (double) n. So even
119+
* if a and b have different classes they might still be equal if they are primitives. */
116120
@CoreMethod(names = { "equal?", "==" }, required = 1)
117121
public abstract static class ReferenceEqualNode extends CoreMethodArrayArgumentsNode {
118122

@@ -142,57 +146,16 @@ protected boolean equal(double a, double b) {
142146
return Double.doubleToRawLongBits(a) == Double.doubleToRawLongBits(b);
143147
}
144148

145-
@Specialization
146-
protected boolean equal(RubyDynamicObject a, RubyDynamicObject b) {
149+
@Specialization(guards = { "a.getClass() == b.getClass()", "!isPrimitive(a)" }) // since a and b have the same class, implies !isPrimitive(b)
150+
protected boolean equalSameClassNonPrimitive(Object a, Object b) {
147151
return a == b;
148152
}
149153

150-
@Specialization(
151-
guards = {
152-
"isNotRubyDynamicObject(a)",
153-
"isNotRubyDynamicObject(b)",
154-
"!sameClass(a, b)",
155-
"isNotIntLong(a) || isNotIntLong(b)" })
156-
protected boolean equalIncompatiblePrimitiveTypes(Object a, Object b) {
157-
return false;
158-
}
159-
160-
@Specialization(
161-
guards = {
162-
"isNotRubyDynamicObject(a)",
163-
"isNotRubyDynamicObject(b)",
164-
"sameClass(a, b)",
165-
"isNotIntLongDouble(a) || isNotIntLongDouble(b)" })
166-
protected boolean equalOtherSameClass(Object a, Object b) {
167-
return a == b;
168-
}
169-
170-
@Specialization(guards = "isNotRubyDynamicObject(a)")
171-
protected boolean equal(Object a, RubyDynamicObject b) {
172-
return false;
173-
}
174-
175-
@Specialization(guards = "isNotRubyDynamicObject(b)")
176-
protected boolean equal(RubyDynamicObject a, Object b) {
154+
@Fallback
155+
protected boolean fallback(Object a, Object b) {
177156
return false;
178157
}
179158

180-
protected boolean isNotRubyDynamicObject(Object value) {
181-
return !(value instanceof RubyDynamicObject);
182-
}
183-
184-
protected boolean sameClass(Object a, Object b) {
185-
return a.getClass() == b.getClass();
186-
}
187-
188-
protected boolean isNotIntLong(Object v) {
189-
return !(v instanceof Integer) && !(v instanceof Long);
190-
}
191-
192-
protected boolean isNotIntLongDouble(Object v) {
193-
return !(v instanceof Integer) && !(v instanceof Long) && !(v instanceof Double);
194-
}
195-
196159
}
197160

198161
@GenerateUncached

src/main/java/org/truffleruby/language/RubyGuards.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,8 @@ public static boolean isImplicitLongOrDouble(Object object) {
6565
object instanceof Long || object instanceof Float || object instanceof Double;
6666
}
6767

68+
/** Does not include {@link Character} as those are converted as the interop boundary and are not implicit casts in
69+
* {@link RubyTypes}. */
6870
public static boolean isPrimitive(Object object) {
6971
return object instanceof Boolean || object instanceof Byte || object instanceof Short ||
7072
object instanceof Integer || object instanceof Long || object instanceof Float ||

0 commit comments

Comments
 (0)