Reviewers: caitp,
https://codereview.chromium.org/1225223004/diff/1/src/ast.cc
File src/ast.cc (right):
https://codereview.chromium.org/1225223004/diff/1/src/ast.cc#newcode514
src/ast.cc:514: constants_length = i;
On 2015/07/14 05:49:53, caitp wrote:
On 2015/07/14 05:49:03, caitp wrote:
> Looks like you just need a `break` here to make this work
Otherwise if there are multiple spread elements, it will make the
constant
length too high
Oops, indeed, I refactored this from a different approach below.
Description:
Work in progress fix for spreading an array literal inside an array literal
DO NOT LAND
BUG=v8:4298
Please review this at https://codereview.chromium.org/1225223004/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Affected files (+15, -7 lines):
M src/ast.cc
A + test/mjsunit/harmony/regress/regress-4298.js
Index: src/ast.cc
diff --git a/src/ast.cc b/src/ast.cc
index
f7eab6ec710dd438fbe3d9095e90f5e00e086d85..a25ceb92844e82758110c13f6d0719f68e070ef6
100644
--- a/src/ast.cc
+++ b/src/ast.cc
@@ -505,9 +505,19 @@ void ObjectLiteral::BuildConstantProperties(Isolate*
isolate) {
void ArrayLiteral::BuildConstantElements(Isolate* isolate) {
if (!constant_elements_.is_null()) return;
+ // Find the first spread; everything before that is part of
+ // the constant elements.
+ // TODO(adamk): Store constant length in the AST.
+ int constants_length = values()->length();
+ for (int i = 0; i < values()->length(); i++) {
+ if (values()->at(i)->IsSpread()) {
+ constants_length = i;
+ }
+ }
+
// Allocate a fixed array to hold all the object literals.
Handle<JSArray> array = isolate->factory()->NewJSArray(
- FAST_HOLEY_SMI_ELEMENTS, values()->length(), values()->length(),
+ FAST_HOLEY_SMI_ELEMENTS, constants_length, constants_length,
Strength::WEAK, INITIALIZE_ARRAY_ELEMENTS_WITH_HOLE);
// Fill in the literals.
@@ -515,9 +525,9 @@ void ArrayLiteral::BuildConstantElements(Isolate*
isolate) {
int depth_acc = 1;
bool is_holey = false;
int array_index = 0;
- for (int n = values()->length(); array_index < n; array_index++) {
+ for (; array_index < constants_length; array_index++) {
Expression* element = values()->at(array_index);
- if (element->IsSpread()) break;
+ DCHECK(!element->IsSpread());
MaterializedLiteral* m_literal = element->AsMaterializedLiteral();
if (m_literal != NULL) {
m_literal->BuildConstants(isolate);
@@ -543,9 +553,6 @@ void ArrayLiteral::BuildConstantElements(Isolate*
isolate) {
.Assert();
}
- if (array_index != values()->length()) {
- JSArray::SetLength(array, array_index);
- }
JSObject::ValidateElements(array);
Handle<FixedArrayBase> element_values(array->elements());
Index: test/mjsunit/harmony/regress/regress-4298.js
diff --git a/test/message/formal-parameters-trailing-comma.js
b/test/mjsunit/harmony/regress/regress-4298.js
similarity index 75%
copy from test/message/formal-parameters-trailing-comma.js
copy to test/mjsunit/harmony/regress/regress-4298.js
index
09cdb8904bee73012d9c53efaceff8e892666c15..c4fe5191382114f7c4caf5aa93879fa1f83b407a
100644
--- a/test/message/formal-parameters-trailing-comma.js
+++ b/test/mjsunit/harmony/regress/regress-4298.js
@@ -2,4 +2,5 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-function foo(b, a, a,) { return a }
+var arr = [1, 2, ...[3]];
+assertEquals([1, 2, 3], arr);
--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
