Revision: 4971
Author: [email protected]
Date: Mon Jun 28 23:47:19 2010
Log: Ensure that ToPrimitive is called on all objects involved in
comparisons <, <=, >, >=. Ensures that ToPrimitive is called when
comparing an object to undefined. Fixes bugs on all platforms.
Review URL: http://codereview.chromium.org/2834022
http://code.google.com/p/v8/source/detail?r=4971
Modified:
/branches/bleeding_edge/src/ia32/codegen-ia32.cc
/branches/bleeding_edge/src/runtime.js
/branches/bleeding_edge/src/x64/codegen-x64.cc
/branches/bleeding_edge/test/mjsunit/to_number_order.js
=======================================
--- /branches/bleeding_edge/src/ia32/codegen-ia32.cc Fri Jun 25 05:31:49
2010
+++ /branches/bleeding_edge/src/ia32/codegen-ia32.cc Mon Jun 28 23:47:19
2010
@@ -11686,16 +11686,18 @@
__ Set(eax, Immediate(Smi::FromInt(EQUAL)));
__ ret(0);
} else {
- Label return_equal;
Label heap_number;
- // If it's not a heap number, then return equal.
__ cmp(FieldOperand(edx, HeapObject::kMapOffset),
Immediate(Factory::heap_number_map()));
- __ j(equal, &heap_number);
- __ bind(&return_equal);
- __ Set(eax, Immediate(Smi::FromInt(EQUAL)));
- __ ret(0);
-
+ if (cc_ == equal) {
+ __ j(equal, &heap_number);
+ // Identical objects are equal for operators ==, !=, and ===.
+ __ Set(eax, Immediate(Smi::FromInt(EQUAL)));
+ __ ret(0);
+ } else {
+ // Identical objects must call ToPrimitive for <, <=, >, and >=.
+ __ j(not_equal, ¬_identical);
+ }
__ bind(&heap_number);
// It is a heap number, so return non-equal if it's NaN and equal if
// it's not NaN.
=======================================
--- /branches/bleeding_edge/src/runtime.js Tue May 25 03:35:55 2010
+++ /branches/bleeding_edge/src/runtime.js Mon Jun 28 23:47:19 2010
@@ -112,7 +112,7 @@
// the result when either (or both) the operands are NaN.
function COMPARE(x, ncr) {
var left;
-
+ var right;
// Fast cases for string, numbers and undefined compares.
if (IS_STRING(this)) {
if (IS_STRING(x)) return %_StringCompare(this, x);
@@ -123,14 +123,18 @@
if (IS_UNDEFINED(x)) return ncr;
left = this;
} else if (IS_UNDEFINED(this)) {
+ if (!IS_UNDEFINED(x)) {
+ %ToPrimitive(x, NUMBER_HINT);
+ }
+ return ncr;
+ } else if (IS_UNDEFINED(x)) {
+ %ToPrimitive(this, NUMBER_HINT);
return ncr;
} else {
- if (IS_UNDEFINED(x)) return ncr;
left = %ToPrimitive(this, NUMBER_HINT);
}
- // Default implementation.
- var right = %ToPrimitive(x, NUMBER_HINT);
+ right = %ToPrimitive(x, NUMBER_HINT);
if (IS_STRING(left) && IS_STRING(right)) {
return %_StringCompare(left, right);
} else {
=======================================
--- /branches/bleeding_edge/src/x64/codegen-x64.cc Mon Jun 28 03:54:07 2010
+++ /branches/bleeding_edge/src/x64/codegen-x64.cc Mon Jun 28 23:47:19 2010
@@ -8959,23 +8959,32 @@
// Test for NaN. Sadly, we can't just compare to Factory::nan_value(),
// so we do the second best thing - test it ourselves.
// Note: if cc_ != equal, never_nan_nan_ is not used.
- __ Set(rax, EQUAL);
+ // We cannot set rax to EQUAL until just before return because
+ // rax must be unchanged on jump to not_identical.
+
if (never_nan_nan_ && (cc_ == equal)) {
+ __ Set(rax, EQUAL);
__ ret(0);
} else {
Label heap_number;
- // If it's not a heap number, then return equal.
+ // If it's not a heap number, then return equal for (in)equality
operator.
__ Cmp(FieldOperand(rdx, HeapObject::kMapOffset),
Factory::heap_number_map());
- __ j(equal, &heap_number);
- __ ret(0);
+ if (cc_ == equal) {
+ __ j(equal, &heap_number);
+ __ Set(rax, EQUAL);
+ __ ret(0);
+ } else {
+ // Identical objects must still be converted to primitive for <
and >.
+ __ j(not_equal, ¬_identical);
+ }
__ bind(&heap_number);
// It is a heap number, so return equal if it's not NaN.
// For NaN, return 1 for every condition except greater and
// greater-equal. Return -1 for them, so the comparison yields
// false for all conditions except not-equal.
-
+ __ Set(rax, EQUAL);
__ movsd(xmm0, FieldOperand(rdx, HeapNumber::kValueOffset));
__ ucomisd(xmm0, xmm0);
__ setcc(parity_even, rax);
=======================================
--- /branches/bleeding_edge/test/mjsunit/to_number_order.js Tue Jul 7
02:50:12 2009
+++ /branches/bleeding_edge/test/mjsunit/to_number_order.js Mon Jun 28
23:47:19 2010
@@ -39,6 +39,14 @@
assertEquals(2, Math.max(v,w));
assertEquals("hestfisk", x, "max");
+x = "";
+assertEquals(1, Math.max(v,v));
+assertEquals("hesthest", x, "max_identical");
+
+x = "";
+assertEquals(2, Math.min(w,w));
+assertEquals("fiskfisk", x, "max");
+
x = "";
assertEquals(Math.atan2(1, 2), Math.atan2(v, w));
// JSC says fiskhest.
@@ -122,8 +130,86 @@
new Date().setUTCFullYear(year, month, date, hours, minutes, seconds, ms);
assertEquals("123", x, "Date.setUTCFullYear");
+x = "";
var a = { valueOf: function() { x += "hest"; return 97; } };
var b = { valueOf: function() { x += "fisk"; return 98; } };
assertEquals("ab", String.fromCharCode(a, b), "String.fromCharCode");
+assertEquals("hestfisk", x, "String.fromCharCode valueOf order");
+
+
+
+// Test whether valueOf is called when comparing identical objects
+x = "";
+assertTrue(a < b, "Compare objects a < b");
+assertEquals("hestfisk", x, "Compare objects a < b valueOf order");
+
+x = "";
+assertFalse(a < a, "Compare objects a < a");
+// assertEquals("hesthest", x, "Compare objects a < a valueOf order");
+
+x = "";
+assertTrue(a == a, "Compare objects a == a");
+assertEquals("", x, "Compare objects a == a valueOf not called");
+
+x = "";
+assertFalse(b > b, "Compare objects b > b");
+assertEquals("fiskfisk", x, "Compare objects b > b valueOf order");
+
+x = "";
+assertTrue(b >= b, "Compare objects b >= b");
+assertEquals("fiskfisk", x, "Compare objects b >= b valueOf order");
+
+x = "";
+assertFalse(a > b, "Compare objects a > b");
+assertEquals("fiskhest", x, "Compare objects a > b valueOf order");
+
+x = "";
+assertFalse(a > void(0), "Compare objects a > undefined");
+assertEquals("hest", x, "Compare objects a > undefined valueOf order");
+
+x = "";
+assertFalse(void(0) > b, "Compare objects undefined > b");
+assertEquals("fisk", x, "Compare objects undefined > b valueOf order");
+
+
+function identical_object_comparison() {
+ x = "";
+ assertTrue(a < b, "Compare objects a < b");
+ assertEquals("hestfisk", x, "Compare objects a < b valueOf order");
+
+ x = "";
+ assertFalse(a < a, "Compare objects a < a");
+ // assertEquals("hesthest", x, "Compare objects a < a valueOf order");
+
+ x = "";
+ assertTrue(a == a, "Compare objects a == a");
+ assertEquals("", x, "Compare objects a == a valueOf not called");
+
+ x = "";
+ assertFalse(b > b, "Compare objects b > b");
+ assertEquals("fiskfisk", x, "Compare objects b > b valueOf order");
+
+ x = "";
+ assertTrue(b >= b, "Compare objects b >= b");
+ assertEquals("fiskfisk", x, "Compare objects b >= b valueOf order");
+
+ x = "";
+ assertFalse(a > b, "Compare objects a > b");
+ assertEquals("fiskhest", x, "Compare objects a > b valueOf order");
+
+ x = "";
+ assertFalse(a > void(0), "Compare objects a > undefined");
+ assertEquals("hest", x, "Compare objects a > undefined valueOf order");
+
+ x = "";
+ assertFalse(void(0) > b, "Compare objects undefined > b");
+ assertEquals("fisk", x, "Compare objects undefined > b valueOf order");
+}
+
+// Call inside loop to test optimization and possible caching.
+for (i = 0; i < 3; ++i) {
+ identical_object_comparison();
+}
+
print("ok");
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
