Changeset 26899 in webkit for trunk/JavaScriptCore

Timestamp:

Oct 22, 2007, 4:36:36 PM (18 years ago)

Author:

darin

Message:

JavaScriptCore:

Reviewed by Geoff.

• fix ​http://bugs.webkit.org/show_bug.cgi?id=15632 js1_5/Array/array-001.js test failing

One of the JavaScriptCore tests was failing; it failed because of
my change to NumberImp::getUInt32. The incorrect code I copied was
from JSImmediate::getUInt32, and was a pre-existing bug.

This patch fixes correctness, but will surely slow down SunSpider.
We may be able to code this tighter and get the speed back.

• kjs/JSImmediate.h: (KJS::JSImmediate::getInt32): Renamed from toInt32 to more accurately reflect the fact that this function only returns true if the value is accurate (no fractional part, etc.). Changed code so that it returns false when the value has a fraction. (KJS::JSImmediate::getUInt32): Ditto.

• kjs/internal.cpp: (KJS::NumberImp::getInt32): Changed code so that it returns false when the value has a fraction. Restores the old behavior. (KJS::NumberImp::getUInt32): Ditto.

• kjs/value.h: (KJS::JSValue::getInt32): Updated for name change. (KJS::JSValue::getUInt32): Ditto. (KJS::JSValue::toInt32): Ditto. (KJS::JSValue::toUInt32): Ditto.

LayoutTests:

Reviewed by Geoff.

• tests for ​http://bugs.webkit.org/show_bug.cgi?id=15632

Added tests for cases where you use something that looks like an array
index, but it has a fractional part.

• fast/js/kde/resources/Array.js: Added tests.
• fast/js/kde/Array-expected.txt: Updated.

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• kjs/JSImmediate.h (modified) (6 diffs)
• kjs/internal.cpp (modified) (1 diff)
• kjs/value.h (modified) (5 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2007-10-22  Darin Adler  <[email protected]>
+
+        Reviewed by Geoff.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=15632
+          js1_5/Array/array-001.js test failing
+
+        One of the JavaScriptCore tests was failing; it failed because of
+        my change to NumberImp::getUInt32. The incorrect code I copied was
+        from JSImmediate::getUInt32, and was a pre-existing bug.
+
+        This patch fixes correctness, but will surely slow down SunSpider.
+        We may be able to code this tighter and get the speed back.
+
+        * kjs/JSImmediate.h:
+        (KJS::JSImmediate::getInt32): Renamed from toInt32 to more accurately
+        reflect the fact that this function only returns true if the value is
+        accurate (no fractional part, etc.). Changed code so that it returns
+        false when the value has a fraction.
+        (KJS::JSImmediate::getUInt32): Ditto.
+
+        * kjs/internal.cpp:
+        (KJS::NumberImp::getInt32): Changed code so that it returns false when
+        the value has a fraction. Restores the old behavior.
+        (KJS::NumberImp::getUInt32): Ditto.
+
+        * kjs/value.h:
+        (KJS::JSValue::getInt32): Updated for name change.
+        (KJS::JSValue::getUInt32): Ditto.
+        (KJS::JSValue::toInt32): Ditto.
+        (KJS::JSValue::toUInt32): Ditto.
+
 2007-10-22  Darin Adler  <[email protected]>
 

trunk/JavaScriptCore/kjs/JSImmediate.h

@@ -88 +88 @@
     static JSType type(const JSValue*);
 
-    static bool toInt32(const JSValue*, int32_t&);
-    static bool toUInt32(const JSValue*, uint32_t&);
+    static bool getInt32(const JSValue*, int32_t&);
+    static bool getUInt32(const JSValue*, uint32_t&);
 
     // It would nice just to use fromDouble() to create these values, but that would prevent them from
@@ -172 +172 @@
     }
 
-    static bool toInt32(const JSValue* v, int32_t& i)
+    static bool getInt32(const JSValue* v, int32_t& i)
     {
         ASSERT(isImmediate(v));
@@ -179 +179 @@
         floatUnion.asBits = static_cast<uint32_t>(unTag(v));
         float f = floatUnion.asFloat;
-        if (!(f >= -2147483648.0F && f < 2147483648.0F))
-            return false;
         i = static_cast<int32_t>(f);
-        return isNumber(v);
-    }
-
-    static bool toUInt32(const JSValue* v, uint32_t& i)
+        return isNumber(v) && i == f;
+    }
+
+    static bool getUInt32(const JSValue* v, uint32_t& i)
     {
         ASSERT(isImmediate(v));
@@ -192 +190 @@
         floatUnion.asBits = static_cast<uint32_t>(unTag(v));
         float f = floatUnion.asFloat;
-        if (!(f >= 0.0F && f < 4294967296.0F))
-            return false;
         i = static_cast<uint32_t>(f);
-        return isNumber(v);
+        return isNumber(v) && i == f;
     }
 };
@@ -225 +221 @@
     }
 
-    static bool toInt32(const JSValue* v, int32_t& i)
+    static bool getInt32(const JSValue* v, int32_t& i)
     {
         double d = toDouble(v);
-        if (!(d >= -2147483648.0 && d < 2147483648.0))
-            return false;
         i = static_cast<int32_t>(d);
-        return isNumber(v);
-    }
-
-    static bool toUInt32(const JSValue* v, uint32_t& i)
+        return isNumber(v) && i == d;
+    }
+
+    static bool getUInt32(const JSValue* v, uint32_t& i)
     {
         double d = toDouble(v);
-        if (!(d >= 0.0 && d < 4294967296.0))
-            return false;
         i = static_cast<uint32_t>(d);
-        return isNumber(v);
+        return isNumber(v) && i == d;
     }
 };
@@ -271 +263 @@
 }
 
-inline bool JSImmediate::toInt32(const JSValue* v, int32_t& i)
-{
-    return FPBitValues<is32bit, is64bit>::toInt32(v, i);
-}
-
-inline bool JSImmediate::toUInt32(const JSValue* v, uint32_t& i)
-{
-    return FPBitValues<is32bit, is64bit>::toUInt32(v, i);
+inline bool JSImmediate::getInt32(const JSValue* v, int32_t& i)
+{
+    return FPBitValues<is32bit, is64bit>::getInt32(v, i);
+}
+
+inline bool JSImmediate::getUInt32(const JSValue* v, uint32_t& i)
+{
+    return FPBitValues<is32bit, is64bit>::getUInt32(v, i);
 }
 

trunk/JavaScriptCore/kjs/internal.cpp

@@ -114 +114 @@
 bool NumberImp::getInt32(int32_t& int32) const
 {
-    if (!(val >= -2147483648.0 && val < 2147483648.0))
-        return false;
     int32 = static_cast<int32_t>(val);
-    return true;
+    return int32 == val;
 }
 
 bool NumberImp::getUInt32(uint32_t& uint32) const
 {
-    if (!(val >= 0.0 && val < 4294967296.0))
-        return false;
     uint32 = static_cast<uint32_t>(val);
-    return true;
+    return uint32 == val;
 }
 

trunk/JavaScriptCore/kjs/value.h

@@ -335 +335 @@
 inline bool JSValue::getInt32(int32_t& v) const
 {
-    return JSImmediate::isImmediate(this) ? JSImmediate::toInt32(this, v) : asCell()->getInt32(v);
+    return JSImmediate::isImmediate(this) ? JSImmediate::getInt32(this, v) : asCell()->getInt32(v);
 }
 
 inline bool JSValue::getUInt32(uint32_t& v) const
 {
-    return JSImmediate::isImmediate(this) ? JSImmediate::toUInt32(this, v) : asCell()->getUInt32(v);
+    return JSImmediate::isImmediate(this) ? JSImmediate::getUInt32(this, v) : asCell()->getUInt32(v);
 }
 
@@ -387 +387 @@
 {
     int32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toInt32(this, i))
+    if (JSImmediate::isImmediate(this) && JSImmediate::getInt32(this, i))
         return i;
     bool ok;
@@ -396 +396 @@
 {
     uint32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toUInt32(this, i))
+    if (JSImmediate::isImmediate(this) && JSImmediate::getUInt32(this, i))
         return i;
     bool ok;
@@ -405 +405 @@
 {
     int32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toInt32(this, i)) {
+    if (JSImmediate::isImmediate(this) && JSImmediate::getInt32(this, i)) {
         ok = true;
         return i;
@@ -415 +415 @@
 {
     uint32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toUInt32(this, i)) {
+    if (JSImmediate::isImmediate(this) && JSImmediate::getUInt32(this, i)) {
         ok = true;
         return i;