Author: [EMAIL PROTECTED]
Date: Wed Oct 29 03:02:09 2008
New Revision: 635
Modified:
branches/bleeding_edge/src/factory.cc
branches/bleeding_edge/src/factory.h
branches/bleeding_edge/src/objects.h
branches/bleeding_edge/src/runtime.cc
branches/bleeding_edge/test/mjsunit/array-concat.js
Log:
Fix issue with Array.concat not preserving holes in the
top-level arrays.
Review URL: http://codereview.chromium.org/8694
Modified: branches/bleeding_edge/src/factory.cc
==============================================================================
--- branches/bleeding_edge/src/factory.cc (original)
+++ branches/bleeding_edge/src/factory.cc Wed Oct 29 03:02:09 2008
@@ -42,6 +42,12 @@
}
+Handle<FixedArray> Factory::NewFixedArrayWithHoles(int size) {
+ ASSERT(0 <= size);
+ CALL_HEAP_FUNCTION(Heap::AllocateFixedArrayWithHoles(size), FixedArray);
+}
+
+
Handle<Dictionary> Factory::NewDictionary(int at_least_space_for) {
ASSERT(0 <= at_least_space_for);
CALL_HEAP_FUNCTION(Dictionary::Allocate(at_least_space_for), Dictionary);
Modified: branches/bleeding_edge/src/factory.h
==============================================================================
--- branches/bleeding_edge/src/factory.h (original)
+++ branches/bleeding_edge/src/factory.h Wed Oct 29 03:02:09 2008
@@ -37,10 +37,14 @@
class Factory : public AllStatic {
public:
- // Allocate a new fixed array.
+ // Allocate a new fixed array with undefined entries.
static Handle<FixedArray> NewFixedArray(
int size,
PretenureFlag pretenure = NOT_TENURED);
+
+ // Allocate a new fixed array with non-existing entries (the hole).
+ static Handle<FixedArray> NewFixedArrayWithHoles(int size);
+
static Handle<Dictionary> NewDictionary(int at_least_space_for);
static Handle<DescriptorArray> NewDescriptorArray(int
number_of_descriptors);
Modified: branches/bleeding_edge/src/objects.h
==============================================================================
--- branches/bleeding_edge/src/objects.h (original)
+++ branches/bleeding_edge/src/objects.h Wed Oct 29 03:02:09 2008
@@ -1975,9 +1975,6 @@
// Fill in details for properties into storage.
void CopyKeysTo(FixedArray* storage);
- // Returns the value at entry.
- static int ValueIndexFor(int entry) { return EntryToIndex(entry)+1; }
-
// For transforming properties of a JSObject.
Object* TransformPropertiesToFastFor(JSObject* obj,
int unused_property_fields);
Modified: branches/bleeding_edge/src/runtime.cc
==============================================================================
--- branches/bleeding_edge/src/runtime.cc (original)
+++ branches/bleeding_edge/src/runtime.cc Wed Oct 29 03:02:09 2008
@@ -4128,16 +4128,17 @@
/**
* A helper function of Runtime_ArrayConcat.
*
- * The first argument is an Array of Arrays and objects. It is the same as
- * the arguments array of Array::concat JS function.
+ * The first argument is an Array of arrays and objects. It is the
+ * same as the arguments array of Array::concat JS function.
*
- * If an argument is an Array object, the function visits array elements.
- * If an argument is not an Array object, the function visits the object
- * as if it is an one-element array.
+ * If an argument is an Array object, the function visits array
+ * elements. If an argument is not an Array object, the function
+ * visits the object as if it is an one-element array.
*
- * If the result array index overflows 32-bit integer, the rounded
non-negative
- * number is used as new length. For example, if one array length is 2^32
- 1,
- * second array length is 1, the concatenated array length is 0.
+ * If the result array index overflows 32-bit integer, the rounded
+ * non-negative number is used as new length. For example, if one
+ * array length is 2^32 - 1, second array length is 1, the
+ * concatenated array length is 0.
*/
static uint32_t IterateArguments(Handle<JSArray> arguments,
ArrayConcatVisitor* visitor) {
@@ -4201,13 +4202,16 @@
Handle<JSArray> result = Factory::NewJSArray(0);
uint32_t estimate_nof_elements = IterateArguments(arguments, NULL);
- // If estimated number of elements is more than half of length,
- // A fixed array (fast case) is more time & space-efficient than a
dictionary.
+ // If estimated number of elements is more than half of length, a
+ // fixed array (fast case) is more time and space-efficient than a
+ // dictionary.
bool fast_case = (estimate_nof_elements * 2) >= result_length;
Handle<FixedArray> storage;
if (fast_case) {
- storage = Factory::NewFixedArray(result_length);
+ // The backing storage array must have non-existing elements to
+ // preserve holes across concat operations.
+ storage = Factory::NewFixedArrayWithHoles(result_length);
} else {
// TODO(126): move 25% pre-allocation logic into Dictionary::Allocate
Modified: branches/bleeding_edge/test/mjsunit/array-concat.js
==============================================================================
--- branches/bleeding_edge/test/mjsunit/array-concat.js (original)
+++ branches/bleeding_edge/test/mjsunit/array-concat.js Wed Oct 29 03:02:09
2008
@@ -107,3 +107,14 @@
assertEquals(1, c.length);
assertEquals("Hello", c[0]);
assertEquals("Hello", c.toString());
+
+// Check that concat preserves holes.
+var holey = [void 0,'a',,'c'].concat(['d',,'f',[0,,2],void 0])
+assertEquals(9, holey.length); // hole in embedded array is ignored
+for (var i = 0; i < holey.length; i++) {
+ if (i == 2 || i == 5) {
+ assertFalse(i in holey);
+ } else {
+ assertTrue(i in holey);
+ }
+}
--~--~---------~--~----~------------~-------~--~----~
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
-~----------~----~----~----~------~----~------~--~---
