Erik/Adam, mind taking a look?
https://codereview.chromium.org/1212473002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Comments addressed, landing
https://codereview.chromium.org/1212473002/diff/40001/src/ast-literal-reindexer.h
File src/ast-literal-reindexer.h (right):
https://codereview.chromium.org/1212473002/diff/40001/src/ast-literal-reindexer.h#newcode13
src/ast-literal-reindexer.h:13: #include
still lgtm.
Great cleanup!
https://codereview.chromium.org/1203103003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Reviewers: wingo,
Message:
PTAL.
Description:
[destructuring] Re-index materialized literals in arrow function parameters.
R=wi...@igalia.com
BUG=v8:811
LOG=N
Please review this at https://codereview.chromium.org/1212473002/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
Nevermind, will look into failures
https://codereview.chromium.org/1212473002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
lgtm
https://codereview.chromium.org/1208443002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/06/23 21:40:17, adamk wrote:
I don't see any obvious things missing from your test cases, this looks
ready
to
port to me.
+1
https://codereview.chromium.org/1203813002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this
https://codereview.chromium.org/1203103003/diff/1/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):
https://codereview.chromium.org/1203103003/diff/1/src/arm/builtins-arm.cc#newcode627
src/arm/builtins-arm.cc:627: __ pop(r1); // Constructor function.
Use __ Drop(1)
https://codereview.chromium.org/1203103003/diff/1/src/arm/builtins-arm.cc
File src/arm/builtins-arm.cc (right):
https://codereview.chromium.org/1203103003/diff/1/src/arm/builtins-arm.cc#newcode627
src/arm/builtins-arm.cc:627: __ pop(r1); // Constructor function.
On 2015/06/24 15:54:10, arv
lgtm
https://codereview.chromium.org/1203103003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Reviewers: adamk_chromiunm.org, arv,
Message:
PTAL. Sad panda.
Description:
Use C runtime functions for ThrowNewXXError desugarings.
JS runtime function calls cause Hydrogen to bail out.
R=ad...@chromiunm.org,a...@chromium.org
Please review this at https://codereview.chromium.org/1210533003/
On 2015/06/24 16:54:36, arv wrote:
This looks bad:
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_arm_rel/builds/5473/steps/Check/logs/inline-construct
Indeed. Seems to have to do with inlining?
https://codereview.chromium.org/1203103003/
--
--
v8-dev mailing list
Reviewers: adamk, arv,
Message:
PTAL, ia32 only for now.
Description:
Do not add extra argument for new.target
JSConstructStub for subclass constructors instead locates new.target in
a known location on the stack.
R=a...@chromium.org,ad...@chromium.org
BUG=v8:3886
LOG=N
Please review this at
Yup, passes all tests
https://codereview.chromium.org/1196193014/diff/1/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/1196193014/diff/1/src/ia32/full-codegen-ia32.cc#newcode269
src/ia32/full-codegen-ia32.cc:269: SetVar(new_target_var,
Comments addressed, landing.
https://codereview.chromium.org/1196193014/diff/1/src/ia32/builtins-ia32.cc
File src/ia32/builtins-ia32.cc (right):
https://codereview.chromium.org/1196193014/diff/1/src/ia32/builtins-ia32.cc#newcode556
src/ia32/builtins-ia32.cc:556: __ mov(ebx, Operand(esp,
Comments addressed.
Initializers (+default parameters) have completely wrong scope so far. I'll
address everything relating to initializers in the follow-up
https://codereview.chromium.org/1189743003/diff/80001/src/parser.cc
File src/parser.cc (right):
Comments addressed (sorry had to rebase).
PTAL
https://codereview.chromium.org/1189743003/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1189743003/diff/60001/src/parser.cc#newcode4301
src/parser.cc:4301:
On 2015/06/19 19:25:26, arv wrote:
-1 newline
lgtm - let's land this?
https://codereview.chromium.org/1191653008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
PS#8 attempts to fix a read-bogus-memory serialization issue.
Yang, ptal at a change in serialize.cc
https://codereview.chromium.org/1189743003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to
Very sorry for a looong delay :(
Looks good to me modulo other people's comments and a test comment below.
https://codereview.chromium.org/1168643005/diff/31/test/cctest/test-parsing.cc
File test/cctest/test-parsing.cc (right):
Please take another look
https://codereview.chromium.org/1189743003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1189743003/diff/1/src/parser.cc#newcode4227
src/parser.cc:4227: descriptor.mode = VAR;
Changing to LET - it is always LET for patterns since it
lgtm
https://codereview.chromium.org/1168643005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Refactored formal parameter parsing to better encapsulate state and separate
concerns.
Will now address other comments.
https://codereview.chromium.org/1189743003/diff/1/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1189743003/diff/1/src/scopes.h#newcode377
Reviewers: arv, rossberg,
Message:
PTAL
Description:
[destructuring] Implement parameter pattern matching.
Scoping for initializers is yet incorrect. Defaults are not supported.
R=a...@chromium.org,rossb...@chromium.org
BUG=v8:811
LOG=N
Please review this at
lgtm
https://codereview.chromium.org/1180943007/diff/20001/src/preparser.cc
File src/preparser.cc (right):
https://codereview.chromium.org/1180943007/diff/20001/src/preparser.cc#newcode602
src/preparser.cc:602: if (!FLAG_strong_this) break;
Please add:
// Fall through.
This is ready for review.
https://codereview.chromium.org/1167393005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
comments addressed, landing
https://codereview.chromium.org/1167393005/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1167393005/diff/20001/src/parser.cc#newcode1187
src/parser.cc:1187: BlockState block_state(scope_, scope);
On 2015/06/15 15:21:59, arv
https://codereview.chromium.org/1178523002/diff/20001/src/expression-classifier.h
File src/expression-classifier.h (right):
https://codereview.chromium.org/1178523002/diff/20001/src/expression-classifier.h#newcode40
src/expression-classifier.h:40: ArrowFormalParametersWithRestProduction
= 1 7,
lgtm
lulz :) lgtm, thanks!
https://codereview.chromium.org/1174543003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/06/08 18:29:43, arv wrote:
Dmitry, welcome back. Can you take a look?
lgtm but Toon is back too, would be nice if he can have a look as well
https://codereview.chromium.org/1161073002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You
On 2015/06/09 16:26:36, Dmitry S. Lomov wrote:
On 2015/06/09 15:35:49, caitp wrote:
So, do we have any comments about this patch? I'd like to do something
with
this
one at some point
Sorry for being slow on this, I am looking at it - will need a bit of time
though.
Sorry for sabotaging
On 2015/06/10 08:08:33, Dmitry S. Lomov wrote:
On 2015/06/09 16:26:36, Dmitry S. Lomov wrote:
On 2015/06/09 15:35:49, caitp wrote:
So, do we have any comments about this patch? I'd like to do something
with
this
one at some point
Sorry for being slow on this, I am looking at it - will
Reviewers: caitp,
Message:
Hey Caitlin, here is your patch rebased for your convenience
Description:
Rebase for parse destructuring assignment patch
COMMIT=false
R=caitpotte...@gmail.com
Please review this at https://codereview.chromium.org/1173003002/
Base URL:
Reviewers: arv, caitp, wingo,
Message:
WIP - Please take initial look at your leasure.
I am almost done, but some tests still fail, in particular
var f = ({x = 42, y = 15}) = {};
Not sure yet why.
Description:
[destructuring] Parse binding patterns in formal parameters.
lgtm
https://codereview.chromium.org/1169853002/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1169853002/diff/1/src/parser.cc#newcode795
src/parser.cc:795: Variable::NORMAL, pos, pos + 10);
On 2015/06/08 19:56:44, arv wrote:
Is there a way to get the
https://codereview.chromium.org/1170153003/diff/20001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1170153003/diff/20001/src/preparser.h#newcode1961
src/preparser.h:1961: }
On 2015/06/09 14:53:08, wingo wrote:
Do we actually want to detect formal parameter
Please take another look
https://codereview.chromium.org/1170153003/diff/1/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1170153003/diff/1/src/preparser.h#newcode1593
src/preparser.h:1593: void DeclareFormalParameter(void* scope,
PreParserIdentifier param,
On
lgtm
https://codereview.chromium.org/1163323006/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/06/09 15:35:49, caitp wrote:
So, do we have any comments about this patch? I'd like to do something
with
this
one at some point
Sorry for being slow on this, I am looking at it - will need a bit of time
though.
https://codereview.chromium.org/1168643005/
--
--
v8-dev mailing list
Reviewers: arv, rossberg, wingo,
Message:
PTAL
Description:
[destructuring] Refactor duplicate parameter name detection.
Pushed the detection logic down to ParseAndClassifyIdentifier in
preparation to having patterns in parameter positions.
Reviewers: arv, rossberg,
Message:
PTAL.
Turned out to be easy enough after my ParseVariableDeclarations refactoring.
Description:
[destructuring] Implement pattern matching in lexcal for-of/for-in.
R=a...@chromium.org,rossb...@chromium.org
BUG=v8:811
LOG=N
Please review this at
Reviewers: arv, rossberg,
Message:
PTAL
Description:
[destructuring] Grand for statement parsing unification.
Also support patterns in ``for (var p in/of ...)``
This CL extends the rewriting we used to do for ``for (let p in/of...)`` to
``for (var p in/of ...)``. For all for..in/of loop
Landing
https://codereview.chromium.org/1149043005/diff/1/test/mjsunit/harmony/destructuring.js
File test/mjsunit/harmony/destructuring.js (right):
https://codereview.chromium.org/1149043005/diff/1/test/mjsunit/harmony/destructuring.js#newcode680
test/mjsunit/harmony/destructuring.js:680: var
lgtm
https://codereview.chromium.org/1145213005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
lgtm
https://codereview.chromium.org/1151853003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
https://codereview.chromium.org/1149043005/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1149043005/diff/1/src/parser.cc#newcode3461
src/parser.cc:3461: // special case for legacy for (var/const x =
in)
On 2015/05/21 17:18:04, rossberg wrote:
Does this
Here is a design I came up with yesterday evening (nothing done yet): for
[a, b, c, ...d]
parser builds an ast:
%SpreadArrayLiteral([a,b,c], d)
where builtin:
function SpreadArrayLiteral(lit, rest) {
let index = lit.length;
let done = false;
while (!done) {
On 2015/05/20 07:59:17, Michael Starzinger wrote:
On 2015/05/20 07:06:46, Dmitry Lomov (chromium) wrote:
Here is a design I came up with yesterday evening (nothing done yet):
for
[a, b, c, ...d]
parser builds an ast:
%SpreadArrayLiteral([a,b,c], d)
This will have similar issues
Reviewers: arv, rossberg,
Message:
PTAL.
I have shamelessly stolen bits and pieces of arv@'s patch here :)
I only parse array literal spreads in patterns - spreads are still
disallowed in
array literal expressions.
Description:
[destructuring] Implement spread binding patterns.
CL looks good to me (not sure about why TF still tries to optimize though)
(I stole some bits for https://codereview.chromium.org/1151503002/ :))
https://codereview.chromium.org/1125183008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You
(so re CL I am not blocked on this for destructuring in any way - take your
time)
https://codereview.chromium.org/1125183008/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups
Landing
https://codereview.chromium.org/1139603005/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1139603005/diff/1/src/parser.cc#newcode3095
src/parser.cc:3095:
Runtime::FunctionForId(Runtime::kThrowIteratorResultNotAnObject),
On 2015/05/19 17:03:48, arv
Quick responses to comments (I'll address others)
https://codereview.chromium.org/1151503002/diff/1/src/pattern-rewriter.cc
File src/pattern-rewriter.cc (right):
https://codereview.chromium.org/1151503002/diff/1/src/pattern-rewriter.cc#newcode308
src/pattern-rewriter.cc:308: // array = [];
On
Addressed comments + do not allocate a literal index for Spread.
PTAL
https://codereview.chromium.org/1151503002/diff/1/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/1151503002/diff/1/src/ast.h#newcode2224
src/ast.h:2224: // Literal index is only used when Spread is
Neat! lgtm % TF (looks good to me too but I am no expert)
https://codereview.chromium.org/1125183008/diff/11/src/ia32/full-codegen-ia32.cc
File src/ia32/full-codegen-ia32.cc (right):
https://codereview.chromium.org/1125183008/diff/11/src/ia32/full-codegen-ia32.cc#newcode1863
-AddStatement(descriptor_-parser-BuildAssertIsCoercible(temp),
+ zone());
for (ObjectLiteralProperty* property : *pattern-properties()) {
-// TODO(dslomov): computed property names.
RecurseIntoSubpattern(
property-value(),
factory()-NewProperty(factory
lgtm
https://codereview.chromium.org/1133843007/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/05/19 14:26:59, rossberg wrote:
lgtm
Thanks - here goes Big Destructuring Landing Party
https://codereview.chromium.org/1146683002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the
Landing
https://codereview.chromium.org/1129083009/diff/20001/test/mjsunit/harmony/destructuring.js
File test/mjsunit/harmony/destructuring.js (right):
https://codereview.chromium.org/1129083009/diff/20001/test/mjsunit/harmony/destructuring.js#newcode344
Reviewers: arv, rossberg_chromiumn.org, wingo,
Message:
PTAL
Description:
[destructuring] Implement BindingArrayPattern
(everything except Spread is implemeneted)
R=a...@chromium.org,rossb...@chromiumn.org,wi...@igalia.com
BUG=v8:811
LOG=N
Please review this at
lgtm
https://codereview.chromium.org/1143813002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
https://codereview.chromium.org/1129083009/diff/20001/test/mjsunit/harmony/destructuring.js
File test/mjsunit/harmony/destructuring.js (right):
https://codereview.chromium.org/1129083009/diff/20001/test/mjsunit/harmony/destructuring.js#newcode286
test/mjsunit/harmony/destructuring.js:286:
lgtm
https://codereview.chromium.org/1134333005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Thanks for the review, all great suggestions! PTAL
https://codereview.chromium.org/1139773005/diff/80001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1139773005/diff/80001/src/parser.cc#newcode4138
src/parser.cc:4138: // return %ThrowNonCoercible(var);
On
(sorry had to rebase because NewThrowTypeError changed under me)
https://codereview.chromium.org/1139773005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups v8-dev group.
To
Removed undetectable check. Whatever we do, we must be consistent betweet
let {x} = foo
and
let x = foo.x
The latter has no undetectable check, so the former shouldn't as well.
https://codereview.chromium.org/1139773005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
On 2015/05/18 13:06:08, wingo wrote:
On 2015/05/18 12:30:14, Dmitry Lomov (chromium) wrote:
PTAL. This is based on https://codereview.chromium.org/1139773005/
Very nicely done! LGTM. Not sure what to do about duplicate identifiers
in
the
pre-parser though; any idea? E.g. let { x, x } =
Comments addressed. Sorry, I have no idea what is the meaning
of 'undetectable'
:(
https://codereview.chromium.org/1139773005/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1139773005/diff/20001/src/parser.cc#newcode4137
src/parser.cc:4137: // if (var
Reviewers: arv, rossberg, wingo,
Message:
PTAL. This is based on https://codereview.chromium.org/1139773005/
Description:
[destructuring] Implement initializers in patterns.
R=a...@chromium.org,rossb...@chromium.org,wi...@igalia.com
BUG=v8:811
LOG=N
Please review this at
On 2015/05/18 13:19:52, wingo wrote:
So e.g. function foo() { var {x,x} = {} } should signal an early error,
even
if
foo() is never run and only parsed by the preparser.
I meant function foo() { let {x,x} = {} } of course.
Hmm, I see no difference between:
function foo() { let {x,x}
Comments addressed + rebased on top of latest patch for
https://codereview.chromium.org/1139773005/
https://codereview.chromium.org/1146683002/diff/1/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1146683002/diff/1/src/parser.h#newcode1015
src/parser.h:1015: Scope*
Landing
https://codereview.chromium.org/1139773005/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1139773005/diff/11/src/parser.cc#newcode4148
src/parser.cc:4148: // return throw /* type error kNonCoercible) */;
On 2015/05/18 14:43:42, arv wrote:
PS3 is the correct patch on top of
https://codereview.chromium.org/1139773005/
https://codereview.chromium.org/1146683002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups
On 2015/05/18 18:53:26, Dmitry Lomov (chromium) wrote:
PS3 is the correct patch on top of
https://codereview.chromium.org/1139773005/
(ok now it is PS2 :))
https://codereview.chromium.org/1146683002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
Overall looks good but I have one question
https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode1015
src/bootstrapper.cc:1015:
Reviewers: arv, rossberg,
Message:
ptal
Description:
[destructuring] More tests for object literal pattern
R=a...@chromium.org,rossb...@chromium.org
BUG=v8:811
LOG=N
Please review this at https://codereview.chromium.org/1139773005/
Base URL: https://chromium.googlesource.com/v8/v8.git@master
On 2015/05/18 09:52:27, Dmitry Lomov (chromium) wrote:
ptal
(sorry seems like I forgot to push a button on Friday)
https://codereview.chromium.org/1139773005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you
lgtm
https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/1134333005/diff/1/src/bootstrapper.cc#newcode1015
src/bootstrapper.cc:1015: CacheInitialJSArrayMaps(native_context(),
initial_map, false);
On 2015/05/18
This is ready for review. What is done in this CL is mostly refactoring. I
only
support C-style for loops in this CL.
Various forms of for-each made the CLe mashroom out of control, so I'll do
that
in subsequent CLs.
https://codereview.chromium.org/1128043006/diff/1/src/parser.cc
File
lgtm
https://codereview.chromium.org/1138153003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
lgtm with bumped stack limit
https://codereview.chromium.org/1138153003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Nice! lgtm
https://codereview.chromium.org/1123383005/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Reviewers: arv, rossberg,
Message:
Work in progress for lulz and horrorz.
I'll probably rewrite a lot here.
Description:
[destructuring] Adapting PatternRewriter to work in for-statements.
WIP
BUG=v8:811
Please review this at https://codereview.chromium.org/1128043006/
Base URL:
The approach looks good to me overall.
It will be nice to keep the LoadIC for home object instead of using a
runtime
function.
I do not think a new Variable::Kind is warranted (for DCHECK, you can just
compare the name).
AST node split between super() and super.prop seems reasonable to me as
Comments addressed, landing
https://codereview.chromium.org/1132203005/diff/1/test/mjsunit/es6/regress/regress-4097.js
File test/mjsunit/es6/regress/regress-4097.js (right):
https://codereview.chromium.org/1132203005/diff/1/test/mjsunit/es6/regress/regress-4097.js#newcode6
Reviewers: arv, Toon Verwaest,
Message:
PTAL
Description:
Fix the behavior of 'super.foo' assignment when receiver is not an object.
R=a...@chromium.org,verwa...@chromium.org
BUG=v8:4097
LOG=N
Please review this at https://codereview.chromium.org/1132203005/
Base URL:
On 2015/05/12 17:16:34, arv wrote:
https://codereview.chromium.org/1132203005/diff/20001/test/mjsunit/es6/regress/regress-4097.js
File test/mjsunit/es6/regress/regress-4097.js (right):
https://codereview.chromium.org/1132203005/diff/20001/test/mjsunit/es6/regress/regress-4097.js#newcode6
Yes, this makes sense to me
https://codereview.chromium.org/1138153003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/05/12 14:13:02, wingo wrote:
On 2015/05/12 14:03:43, wingo wrote:
On 2015/05/12 14:01:50, caitp wrote:
On 2015/05/12 13:51:25, wingo wrote:
Updated patch is pretty much complete modulo a couple of tests.
Three
things
aren't working:
1) {} is a valid BindingPattern
lgtm
https://codereview.chromium.org/1138243002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
On 2015/05/12 14:28:38, wingo wrote:
On 2015/05/12 14:19:55, Dmitry Lomov (chromium) wrote:
On 2015/05/12 14:13:02, wingo wrote:
So yeah then, I think this patch works fine. The only question is
how to
turn
off destructuring to be able to ship these things separately.
https://codereview.chromium.org/1138153003/diff/40001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1138153003/diff/40001/src/preparser.h#newcode677
src/preparser.h:677: void RecordArrowFormalParametersError(const
Scanner::Location loc,
Instead of having all
lgtm
https://codereview.chromium.org/1132573003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
wrote:
Can we do?
for (auto property : properties) {
Done.
https://codereview.chromium.org/1130623004/diff/1/src/pattern-matcher.cc#newcode196
src/pattern-matcher.cc:196: property-key(), RelocInfo::kNoPosition));
On 2015/05/08 19:09:49, arv wrote:
TODO(dslomov): Computed property names
PTAL - comments addressed
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1130623004/diff/20001/src/parser.cc#newcode2373
src/parser.cc:2373: // Create new block with one expected declaration.
On 2015/05/11
PTAL, this is ready for proper review.
https://codereview.chromium.org/1130623004/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
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
Great suggestions, all addressed, PTAL
https://codereview.chromium.org/1130623004/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1130623004/diff/60001/src/parser.cc#newcode2418
src/parser.cc:2418: decl.initialization_scope =
On 2015/05/11 13:18:29,
Comments addressed
https://codereview.chromium.org/1130623004/diff/80001/src/pattern-rewriter.cc
File src/pattern-rewriter.cc (right):
https://codereview.chromium.org/1130623004/diff/80001/src/pattern-rewriter.cc#newcode222
src/pattern-rewriter.cc:222: // TODO(dslomov): computed property names
https://codereview.chromium.org/1130623004/diff/11/src/pattern-rewriter.cc
File src/pattern-rewriter.cc (right):
https://codereview.chromium.org/1130623004/diff/11/src/pattern-rewriter.cc#newcode84
src/pattern-rewriter.cc:84:
On 2015/05/11 15:36:24, rossberg wrote:
Nit: remove empty
On 2015/05/08 19:09:49, arv wrote:
Looks like a good approach.
Some high level comments
- Use macro for visitor
- Can you use a non inner class and provide an .h file for PatternMatcher?
Parser.h is already huge.
This means that PatternMatcher has to be a friend class of Parser. Do you
1 - 100 of 1547 matches
Mail list logo
