Commit e4273007 authored by adamk's avatar adamk Committed by Commit bot

Properly handle holes following spreads in array literals

Before this change, the spread desugaring would naively call
`%AppendElement($R, the_hole)` and in some cases $R would have
a non-holey elements kind, putting the array into the bad state
of exposing holes to author code.

This patch avoids calling %AppendElement with a hole, instead
simply incrementing $R.length when it sees a hole in the literal
(this is safe because $R is known to be an Array). The existing
logic for elements transitions takes care of giving the array a
holey ElementsKind.

BUG=chromium:644215

Review-Url: https://codereview.chromium.org/2321533003
Cr-Commit-Position: refs/heads/master@{#39294}
parent cd86053f
......@@ -296,6 +296,7 @@ class AstValue : public ZoneObject {
F(eval, "eval") \
F(function, "function") \
F(get_space, "get ") \
F(length, "length") \
F(let, "let") \
F(native, "native") \
F(new_target, ".new.target") \
......
......@@ -4979,15 +4979,32 @@ Expression* Parser::RewriteSpreads(ArrayLiteral* lit) {
if (spread == nullptr) {
// If the element is not a spread, we're adding a single:
// %AppendElement($R, value)
ZoneList<Expression*>* append_element_args = NewExpressionList(2);
append_element_args->Add(factory()->NewVariableProxy(result), zone());
append_element_args->Add(value, zone());
do_block->statements()->Add(
factory()->NewExpressionStatement(
factory()->NewCallRuntime(Runtime::kAppendElement,
append_element_args, kNoSourcePosition),
kNoSourcePosition),
zone());
// or, in case of a hole,
// ++($R.length)
if (!value->IsLiteral() ||
!value->AsLiteral()->raw_value()->IsTheHole()) {
ZoneList<Expression*>* append_element_args = NewExpressionList(2);
append_element_args->Add(factory()->NewVariableProxy(result), zone());
append_element_args->Add(value, zone());
do_block->statements()->Add(
factory()->NewExpressionStatement(
factory()->NewCallRuntime(Runtime::kAppendElement,
append_element_args,
kNoSourcePosition),
kNoSourcePosition),
zone());
} else {
Property* length_property = factory()->NewProperty(
factory()->NewVariableProxy(result),
factory()->NewStringLiteral(ast_value_factory()->length_string(),
kNoSourcePosition),
kNoSourcePosition);
CountOperation* count_op = factory()->NewCountOperation(
Token::INC, true /* prefix */, length_property, kNoSourcePosition);
do_block->statements()->Add(
factory()->NewExpressionStatement(count_op, kNoSourcePosition),
zone());
}
} else {
// If it's a spread, we're adding a for/of loop iterating through it.
Variable* each = NewTemporary(ast_value_factory()->dot_for_string());
......
......@@ -418,6 +418,7 @@ RUNTIME_FUNCTION(Runtime_AppendElement) {
CONVERT_ARG_HANDLE_CHECKED(JSArray, array, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
CHECK(!value->IsTheHole(isolate));
uint32_t index;
CHECK(array->length()->ToArrayIndex(&index));
......
// Copyright 2016 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
// Flags: --allow-natives-syntax
var arr = [...[],,];
assertTrue(%HasFastHoleyElements(arr));
assertEquals(1, arr.length);
assertFalse(arr.hasOwnProperty(0));
assertEquals(undefined, arr[0]);
// Should not crash.
assertThrows(() => arr[0][0], TypeError);
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment