Ensure that ToPrimitive is called on all objects involved in comparisons <, <=, ...

Review URL: http://codereview.chromium.org/2834022

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@4971 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/ia32/codegen-ia32.cc b/src/ia32/codegen-ia32.cc
index c0c5442..1a847c1 100644 (file)
--- a/src/ia32/codegen-ia32.cc
+++ b/src/ia32/codegen-ia32.cc
@@ -11686,16 +11686,18 @@ void CompareStub::Generate(MacroAssembler* masm) {
       __ 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, &not_identical);
+      }
       __ bind(&heap_number);
       // It is a heap number, so return non-equal if it's NaN and equal if
       // it's not NaN.

diff --git a/src/runtime.js b/src/runtime.js
index 3e4d473..ab6e3e9 100644 (file)
--- a/src/runtime.js
+++ b/src/runtime.js
@@ -112,7 +112,7 @@ function STRICT_EQUALS(x) {
 // 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 @@ function COMPARE(x, ncr) {
     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 {

diff --git a/src/x64/codegen-x64.cc b/src/x64/codegen-x64.cc
index 0c4cd16..89636ab 100644 (file)
--- a/src/x64/codegen-x64.cc
+++ b/src/x64/codegen-x64.cc
@@ -8959,23 +8959,32 @@ void CompareStub::Generate(MacroAssembler* masm) {
     // 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, &not_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);

diff --git a/test/mjsunit/to_number_order.js b/test/mjsunit/to_number_order.js
index 1329bad..d17e600 100644 (file)
--- a/test/mjsunit/to_number_order.js
+++ b/test/mjsunit/to_number_order.js
@@ -40,6 +40,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.
 assertEquals("hestfisk", x, "atan2");
@@ -122,8 +130,86 @@ x = "";
 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");