Skip to content

Array.prototype.sort() should sort in place. #335

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Array.prototype.sort() should sort in place. #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9f6bded
Array.prototype.sort() should sort in place.
dbatyai Jul 8, 2015
eca1ec4
Array.prototype.sort() now handles defined properties only.
dbatyai Jul 14, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions jerry-core/ecma/base/ecma-helpers-string.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -992,6 +992,39 @@ ecma_string_to_number (const ecma_string_t *str_p) /**< ecma-string */
JERRY_UNREACHABLE ();
} /* ecma_string_to_number */

/**
* Check if string is array index.
*
* @return true - if string is valid array index
* false - otherwise
*/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@return section is missing.

bool
ecma_string_get_array_index (const ecma_string_t *str_p, /**< ecma-string */
uint32_t *out_index_p) /**< out: index */
{
bool is_array_index = true;
if (str_p->container == ECMA_STRING_CONTAINER_UINT32_IN_DESC)
{
*out_index_p = str_p->u.uint32_number;
}
else
{
ecma_number_t num = ecma_string_to_number (str_p);
*out_index_p = ecma_number_to_uint32 (num);

ecma_string_t *to_uint32_to_string_p = ecma_new_ecma_string_from_uint32 (*out_index_p);

is_array_index = ecma_compare_ecma_strings (str_p,
to_uint32_to_string_p);

ecma_deref_ecma_string (to_uint32_to_string_p);
}

is_array_index = is_array_index && (*out_index_p != ECMA_MAX_VALUE_OF_VALID_ARRAY_INDEX);

return is_array_index;
} /* ecma_string_is_array_index */

/**
* Convert ecma-string's contents to a utf-8 string and put it to the buffer.
*
Expand Down
1 change: 1 addition & 0 deletions jerry-core/ecma/base/ecma-helpers.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ extern ecma_string_t* ecma_copy_or_ref_ecma_string (ecma_string_t *string_desc_p
extern void ecma_deref_ecma_string (ecma_string_t *string_p);
extern void ecma_check_that_ecma_string_need_not_be_freed (const ecma_string_t *string_p);
extern ecma_number_t ecma_string_to_number (const ecma_string_t *str_p);
extern bool ecma_string_get_array_index (const ecma_string_t *str_p, uint32_t *index);
extern ssize_t ecma_string_to_utf8_string (const ecma_string_t *string_desc_p,
lit_utf8_byte_t *buffer_p,
ssize_t buffer_size);
Expand Down
125 changes: 106 additions & 19 deletions jerry-core/ecma/builtin-objects/ecma-builtin-array-prototype.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1448,44 +1448,126 @@ ecma_builtin_array_prototype_object_sort (ecma_value_t this_arg, /**< this argum

uint32_t len = ecma_number_to_uint32 (len_number);

MEM_DEFINE_LOCAL_ARRAY (values_buffer, len, ecma_value_t);
uint32_t defined_prop_count = 0;
/* Count number of array index properties. */
for (ecma_property_t *property_p = ecma_get_property_list (obj_p);
property_p != NULL;
property_p = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p))
{
ecma_string_t *property_name_p;

if (property_p->type == ECMA_PROPERTY_NAMEDDATA)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_data_property.name_p);
}
else if (property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_accessor_property.name_p);
}
else
{
continue;
}

uint32_t index;
if (ecma_string_get_array_index (property_name_p, &index) && index < len)
{
defined_prop_count++;
}
}

MEM_DEFINE_LOCAL_ARRAY (values_buffer, defined_prop_count, ecma_value_t);
uint32_t copied_num = 0;

/* Copy unsorted array into a native c array. */
for (uint32_t index = 0; index < len && ecma_is_completion_value_empty (ret_value); index++)
for (ecma_property_t *property_p = ecma_get_property_list (obj_p);
property_p != NULL && ecma_is_completion_value_empty (ret_value);
property_p = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p))
{
ecma_string_t *index_string_p = ecma_new_ecma_string_from_uint32 (index);
ECMA_TRY_CATCH (index_value, ecma_op_object_get (obj_p, index_string_p), ret_value);
ecma_string_t *property_name_p;

values_buffer[index] = ecma_copy_value (index_value, true);
copied_num++;
if (property_p->type == ECMA_PROPERTY_NAMEDDATA)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_data_property.name_p);
}
else if (property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_accessor_property.name_p);
}
else
{
continue;
}

ECMA_FINALIZE (index_value);
ecma_deref_ecma_string (index_string_p);
uint32_t index;
if (ecma_string_get_array_index (property_name_p, &index) && index < len)
{
ECMA_TRY_CATCH (index_value, ecma_op_object_get (obj_p, property_name_p), ret_value);

values_buffer[copied_num++] = ecma_copy_value (index_value, true);

ECMA_FINALIZE (index_value);
}
}

JERRY_ASSERT (copied_num == len || !ecma_is_completion_value_empty (ret_value));
JERRY_ASSERT (copied_num == defined_prop_count || !ecma_is_completion_value_empty (ret_value));

/* Sorting. */
if (len > 1 && ecma_is_completion_value_empty (ret_value))
if (copied_num > 1 && ecma_is_completion_value_empty (ret_value))
{
ECMA_TRY_CATCH (sort_value,
ecma_builtin_array_prototype_object_array_heap_sort_helper (values_buffer,
(int)(len - 1),
(int)(copied_num - 1),
arg1),
ret_value);
ECMA_FINALIZE (sort_value);
}

if (ecma_is_completion_value_empty (ret_value))
/* Put sorted values to the front of the array. */
for (uint32_t index = 0; index < copied_num && ecma_is_completion_value_empty (ret_value); index++)
{
/*
* FIXME: Casting len to ecma_length_t may overflow, but since ecma_length_t is still at least
* 16 bits long, with an array of that size, we would run out of memory way before this happens.
*/
JERRY_ASSERT ((ecma_length_t) len == len);
/* Copy the sorted array into a new array. */
ret_value = ecma_op_create_array_object (values_buffer, (ecma_length_t) len, false);
ecma_string_t *index_string_p = ecma_new_ecma_string_from_uint32 (index);
ECMA_TRY_CATCH (put_value,
ecma_op_object_put (obj_p, index_string_p, values_buffer[index], true),
ret_value);
ECMA_FINALIZE (put_value);
ecma_deref_ecma_string (index_string_p);
}

/* Undefined properties should be in the back of the array. */
ecma_property_t *next_prop;
for (ecma_property_t *property_p = ecma_get_property_list (obj_p);
property_p != NULL && ecma_is_completion_value_empty (ret_value);
property_p = next_prop)
{
ecma_string_t *property_name_p;
next_prop = ECMA_GET_POINTER (ecma_property_t, property_p->next_property_p);

if (property_p->type == ECMA_PROPERTY_NAMEDDATA)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_data_property.name_p);
}
else if (property_p->type == ECMA_PROPERTY_NAMEDACCESSOR)
{
property_name_p = ECMA_GET_NON_NULL_POINTER (ecma_string_t,
property_p->u.named_accessor_property.name_p);
}
else
{
continue;
}

uint32_t index;
if (ecma_string_get_array_index (property_name_p, &index) && index < len && index >= copied_num)
{
ECMA_TRY_CATCH (del_value, ecma_op_object_delete (obj_p, property_name_p, true), ret_value);
ECMA_FINALIZE (del_value);
}
}

/* Free values that were copied to the local array. */
Expand All @@ -1494,6 +1576,11 @@ ecma_builtin_array_prototype_object_sort (ecma_value_t this_arg, /**< this argum
ecma_free_value (values_buffer[index], true);
}

if (ecma_is_completion_value_empty (ret_value))
{
ret_value = ecma_make_normal_completion_value (ecma_copy_value (this_arg, true));
}

MEM_FINALIZE_LOCAL_ARRAY (values_buffer);

ECMA_OP_TO_NUMBER_FINALIZE (len_number);
Expand Down
23 changes: 1 addition & 22 deletions jerry-core/ecma/operations/ecma-array-object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,29 +386,8 @@ ecma_op_array_object_define_own_property (ecma_object_t *obj_p, /**< the array o
{
// 4.a.
uint32_t index;
bool is_index;

if (property_name_p->container == ECMA_STRING_CONTAINER_UINT32_IN_DESC)
{
index = property_name_p->u.uint32_number;
is_index = true;
}
else
{
ecma_number_t number = ecma_string_to_number (property_name_p);
index = ecma_number_to_uint32 (number);

ecma_string_t *to_uint32_to_string_p = ecma_new_ecma_string_from_uint32 (index);

is_index = ecma_compare_ecma_strings (property_name_p,
to_uint32_to_string_p);

ecma_deref_ecma_string (to_uint32_to_string_p);
}

is_index = is_index && (index != ECMA_MAX_VALUE_OF_VALID_ARRAY_INDEX);

if (!is_index)
if (!ecma_string_get_array_index (property_name_p, &index))
{
// 5.
return ecma_op_general_object_define_own_property (obj_p,
Expand Down
36 changes: 24 additions & 12 deletions tests/jerry/array-prototype-sort.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,22 +14,22 @@
// limitations under the License.

var array = ["Peach", "Apple", "Orange", "Grape", "Cherry", "Apricot", "Grapefruit"];
var sorted = array.sort();
array.sort();

assert(sorted[0] === "Apple");
assert(sorted[1] === "Apricot");
assert(sorted[2] === "Cherry");
assert(sorted[3] === "Grape");
assert(sorted[4] === "Grapefruit");
assert(sorted[5] === "Orange");
assert(sorted[6] === "Peach");
assert(array[0] === "Apple");
assert(array[1] === "Apricot");
assert(array[2] === "Cherry");
assert(array[3] === "Grape");
assert(array[4] === "Grapefruit");
assert(array[5] === "Orange");
assert(array[6] === "Peach");

var array = [6, 4, 5, 1, 2, 9, 7, 3, 0, 8];

// Default comparison
var sorted = array.sort();
array.sort();
for (i = 0; i < array.length; i++) {
assert(sorted[i] === i);
assert(array[i] === i);
}

// Using custom comparison function
Expand All @@ -43,9 +43,21 @@ function f(arg1, arg2) {
}
}

var sorted = array.sort(f);
array.sort(f);
for (i = 0; i < array.length; i++) {
assert(sorted[sorted.length - i - 1] === i);
assert(array[array.length - i - 1] === i);
}

// Sorting sparse array
var array = [1,,2,,3,,4,undefined,5];
var expected = [1,2,3,4,5,undefined,,,,];

array.sort();

assert(array.length === expected.length);
for (i = 0; i < array.length; i++) {
assert(expected.hasOwnProperty (i) === array.hasOwnProperty (i));
assert(array[i] === expected[i]);
}

// Checking behavior when provided comparefn is not callable
Expand Down