git://git.webkit.org / WebKit-https.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 27e5590)
JavaScriptCore:
authordarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Oct 2007 23:36:36 +0000 (23:36 +0000)
committerdarin <darin@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 22 Oct 2007 23:36:36 +0000 (23:36 +0000)
        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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@26899 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JavaScriptCore/ChangeLog patch | blob | history
JavaScriptCore/kjs/JSImmediate.h patch | blob | history
JavaScriptCore/kjs/internal.cpp patch | blob | history
JavaScriptCore/kjs/value.h patch | blob | history
LayoutTests/ChangeLog patch | blob | history
LayoutTests/fast/js/kde/Array-expected.txt patch | blob | history
LayoutTests/fast/js/kde/resources/Array.js patch | blob | history

diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 5c237c5b24e9c5e0256bda5e5235aa23a47499ba..1989c9d8db95f4c1acb70262cd6a955af288475c 100644 (file)
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,35 @@
+2007-10-22  Darin Adler  <darin@apple.com>
+
+        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  <darin@apple.com>
 
         Reviewed by Brady.
diff --git a/JavaScriptCore/kjs/JSImmediate.h b/JavaScriptCore/kjs/JSImmediate.h
index 7a625b026f6999fcb267c6a333ae2012819d6576..2a116d260f3c0e900ca55581d59def2a7d500ba9 100644 (file)
--- a/JavaScriptCore/kjs/JSImmediate.h
+++ b/JavaScriptCore/kjs/JSImmediate.h
@@ -87,8 +87,8 @@ public:
     static UString toString(const JSValue*);
     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
     // turning into compile-time constants.
@@ -171,30 +171,26 @@ template<> struct JSImmediate::FPBitValues<true, false> {
         return floatUnion.asFloat;
     }
 
-    static bool toInt32(const JSValue* v, int32_t& i)
+    static bool getInt32(const JSValue* v, int32_t& i)
     {
         ASSERT(isImmediate(v));
 
         FloatUnion floatUnion;
         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);
+        return isNumber(v) && i == f;
     }
 
-    static bool toUInt32(const JSValue* v, uint32_t& i)
+    static bool getUInt32(const JSValue* v, uint32_t& i)
     {
         ASSERT(isImmediate(v));
 
         FloatUnion floatUnion;
         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;
     }
 };
 
@@ -224,22 +220,18 @@ template<> struct JSImmediate::FPBitValues<false, true> {
         return doubleUnion.asDouble;
     }
 
-    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);
+        return isNumber(v) && i == d;
     }
 
-    static bool toUInt32(const JSValue* v, uint32_t& i)
+    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;
     }
 };
 
@@ -270,14 +262,14 @@ inline double JSImmediate::toDouble(const JSValue* v)
     return FPBitValues<is32bit, is64bit>::toDouble(v);
 }
 
-inline bool JSImmediate::toInt32(const JSValue* v, int32_t& i)
+inline bool JSImmediate::getInt32(const JSValue* v, int32_t& i)
 {
-    return FPBitValues<is32bit, is64bit>::toInt32(v, i);
+    return FPBitValues<is32bit, is64bit>::getInt32(v, i);
 }
 
-inline bool JSImmediate::toUInt32(const JSValue* v, uint32_t& i)
+inline bool JSImmediate::getUInt32(const JSValue* v, uint32_t& i)
 {
-    return FPBitValues<is32bit, is64bit>::toUInt32(v, i);
+    return FPBitValues<is32bit, is64bit>::getUInt32(v, i);
 }
 
 } // namespace KJS
diff --git a/JavaScriptCore/kjs/internal.cpp b/JavaScriptCore/kjs/internal.cpp
index d877543da648ecf1e421b28e09ac5d1d29e931ad..48954c1604cb7c33b16a63f5ff40b381756a271b 100644 (file)
--- a/JavaScriptCore/kjs/internal.cpp
+++ b/JavaScriptCore/kjs/internal.cpp
@@ -113,18 +113,14 @@ JSObject *NumberImp::toObject(ExecState *exec) const
 
 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;
 }
 
 // --------------------------- GetterSetterImp ---------------------------------
diff --git a/JavaScriptCore/kjs/value.h b/JavaScriptCore/kjs/value.h
index ac1291af525c7231b5703fbedfcb35216f888157..65cb500579309c12011361b1466225e80d5ddef8 100644 (file)
--- a/JavaScriptCore/kjs/value.h
+++ b/JavaScriptCore/kjs/value.h
@@ -334,12 +334,12 @@ inline const JSObject *JSValue::getObject() const
 
 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);
 }
 
 inline void JSValue::mark()
@@ -386,7 +386,7 @@ inline JSObject* JSValue::toObject(ExecState* exec) const
 ALWAYS_INLINE int32_t JSValue::toInt32(ExecState* exec) const
 {
     int32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toInt32(this, i))
+    if (JSImmediate::isImmediate(this) && JSImmediate::getInt32(this, i))
         return i;
     bool ok;
     return toInt32SlowCase(exec, ok);
@@ -395,7 +395,7 @@ ALWAYS_INLINE int32_t JSValue::toInt32(ExecState* exec) const
 inline uint32_t JSValue::toUInt32(ExecState* exec) const
 {
     uint32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toUInt32(this, i))
+    if (JSImmediate::isImmediate(this) && JSImmediate::getUInt32(this, i))
         return i;
     bool ok;
     return toUInt32SlowCase(exec, ok);
@@ -404,7 +404,7 @@ inline uint32_t JSValue::toUInt32(ExecState* exec) const
 inline int32_t JSValue::toInt32(ExecState* exec, bool& ok) const
 {
     int32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toInt32(this, i)) {
+    if (JSImmediate::isImmediate(this) && JSImmediate::getInt32(this, i)) {
         ok = true;
         return i;
     }
@@ -414,7 +414,7 @@ inline int32_t JSValue::toInt32(ExecState* exec, bool& ok) const
 inline uint32_t JSValue::toUInt32(ExecState* exec, bool& ok) const
 {
     uint32_t i;
-    if (JSImmediate::isImmediate(this) && JSImmediate::toUInt32(this, i)) {
+    if (JSImmediate::isImmediate(this) && JSImmediate::getUInt32(this, i)) {
         ok = true;
         return i;
     }
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 5fec7e8660fa6d7d33c27bd0752b4744fb4161a2..d50664933110aaaac986b856edff9437b15f5fd6 100644 (file)
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2007-10-22  Darin Adler  <darin@apple.com>
+
+        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.
+
 2007-10-22  Darin Adler  <darin@apple.com>
 
         * fast/js/kde/resources/Array.js: Added tests to cover missing value behavior
diff --git a/LayoutTests/fast/js/kde/Array-expected.txt b/LayoutTests/fast/js/kde/Array-expected.txt
index 00216aa12c0b34bc0d5eb867ec32040653fc5e24..30e6c7aacbf7a19007f770ad7e3ab237ea42834c 100644 (file)
--- a/LayoutTests/fast/js/kde/Array-expected.txt
+++ b/LayoutTests/fast/js/kde/Array-expected.txt
@@ -75,6 +75,12 @@ PASS arr.length is 40
 PASS arr[maxint] is undefined
 PASS arr.length is maxint
 PASS arr[maxint-1] is "test2"
+PASS arr.length is 40
+PASS arr[55.5] is "test"
+PASS arr[65.11111111111111111111111111111] is "test"
+PASS arr.length is 40
+PASS arr[55.5] is undefined
+PASS arr[65.11111111111111111111111111111] is undefined
 PASS propnames.length is 3
 PASS propnames[0] is '0'
 PASS propnames[1] is '1'
diff --git a/LayoutTests/fast/js/kde/resources/Array.js b/LayoutTests/fast/js/kde/resources/Array.js
index 0b2b40a3f4c34dbced17209976446caa0f5abbcf..6f2937860c9a5018212d97b4faf03a88972b12b9 100644 (file)
--- a/LayoutTests/fast/js/kde/resources/Array.js
+++ b/LayoutTests/fast/js/kde/resources/Array.js
@@ -149,6 +149,19 @@ arr[maxint-1] = "test2";
 shouldBe("arr.length","maxint");
 shouldBe("arr[maxint-1]","\"test2\"");
 
+// Floating point numbers also should not be treated as valid array indices.
+arr.length = 40;
+arr[55.5] = "test"; // does fit in a JSImmediate number
+arr[65.11111111111111111111111111111] = "test"; // does not fit in a JSImmediate number
+shouldBe("arr.length","40");
+shouldBe("arr[55.5]","\"test\"");
+shouldBe("arr[65.11111111111111111111111111111]","\"test\"");
+delete arr[55.5];
+delete arr[65.11111111111111111111111111111];
+shouldBe("arr.length","40");
+shouldBe("arr[55.5]","undefined");
+shouldBe("arr[65.11111111111111111111111111111]","undefined");
+
 arr = new Array('a','b','c');
 arr.__proto__ = { 1: 'x' };
 var propnames = new Array();
Mirror of the WebKit open source project from svn.webkit.org.