Skip to content

Commit

Permalink
deps: backport e427300 from upstream V8
Browse files Browse the repository at this point in the history
Original commit message:

    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}

Fixes: #12018

PR-URL: #12037
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
  • Loading branch information
targos authored and MylesBorins committed Mar 28, 2017
1 parent ba105fd commit 21944e8
Show file tree
Hide file tree
Showing 5 changed files with 44 additions and 10 deletions.
2 changes: 1 addition & 1 deletion deps/v8/include/v8-version.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
#define V8_MAJOR_VERSION 5
#define V8_MINOR_VERSION 1
#define V8_BUILD_NUMBER 281
#define V8_PATCH_LEVEL 97
#define V8_PATCH_LEVEL 98

// Use 1 for candidates and 0 otherwise.
// (Boolean macro values are not supported by all preprocessors.)
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/ast/ast-value-factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,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") \
Expand Down
37 changes: 28 additions & 9 deletions deps/v8/src/parsing/parser.cc
Original file line number Diff line number Diff line change
Expand Up @@ -5582,16 +5582,35 @@ 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, zone());
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,
// or, in case of a hole,
// ++($R.length)
if (!value->IsLiteral() ||
!value->AsLiteral()->raw_value()->IsTheHole()) {
ZoneList<Expression*>* append_element_args =
NewExpressionList(2, zone());
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,
RelocInfo::kNoPosition),
RelocInfo::kNoPosition),
zone());
} else {
Property* length_property = factory()->NewProperty(
factory()->NewVariableProxy(result),
factory()->NewStringLiteral(ast_value_factory()->length_string(),
RelocInfo::kNoPosition),
RelocInfo::kNoPosition),
zone());
RelocInfo::kNoPosition);
CountOperation* count_op = factory()->NewCountOperation(
Token::INC, true /* prefix */, length_property,
RelocInfo::kNoPosition);
do_block->statements()->Add(
factory()->NewExpressionStatement(count_op,
RelocInfo::kNoPosition),
zone());
}
} else {
// If it's a spread, we're adding a for/of loop iterating through it.
Variable* each =
Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/runtime/runtime-object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,7 @@ RUNTIME_FUNCTION(Runtime_AppendElement) {

CONVERT_ARG_HANDLE_CHECKED(JSArray, array, 0);
CONVERT_ARG_HANDLE_CHECKED(Object, value, 1);
CHECK(!value->IsTheHole());

uint32_t index;
CHECK(array->length()->ToArrayIndex(&index));
Expand Down
13 changes: 13 additions & 0 deletions deps/v8/test/mjsunit/regress/regress-crbug-644215.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// 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);

0 comments on commit 21944e8

Please sign in to comment.