On 2015/09/04 12:56:34, caitp wrote:
On 2015/09/04 12:45:23, caitp wrote:
> On 2015/09/04 12:34:32, caitp wrote:
> > On 2015/09/02 21:11:14, commit-bot: I haz the power wrote:
> > > Patchset 13 (id:??) landed as
> > > https://crrev.com/510baeacbab311798d5e8795800ff773d00d062c
> > > Cr-Commit-Posi
On 2015/09/04 12:45:23, caitp wrote:
On 2015/09/04 12:34:32, caitp wrote:
> On 2015/09/02 21:11:14, commit-bot: I haz the power wrote:
> > Patchset 13 (id:??) landed as
> > https://crrev.com/510baeacbab311798d5e8795800ff773d00d062c
> > Cr-Commit-Position: refs/heads/master@{#30550}
>
> Based on a
On 2015/09/04 12:34:32, caitp wrote:
On 2015/09/02 21:11:14, commit-bot: I haz the power wrote:
> Patchset 13 (id:??) landed as
> https://crrev.com/510baeacbab311798d5e8795800ff773d00d062c
> Cr-Commit-Position: refs/heads/master@{#30550}
Based on a simple microbenchmark, http://jsperf.com/v8-r
On 2015/09/02 21:11:14, commit-bot: I haz the power wrote:
Patchset 13 (id:??) landed as
https://crrev.com/510baeacbab311798d5e8795800ff773d00d062c
Cr-Commit-Position: refs/heads/master@{#30550}
Based on a simple microbenchmark, http://jsperf.com/v8-rest-parameters3/2,
this
may have been more
Patchset 13 (id:??) landed as
https://crrev.com/510baeacbab311798d5e8795800ff773d00d062c
Cr-Commit-Position: refs/heads/master@{#30550}
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messa
Committed patchset #13 (id:230001)
https://codereview.chromium.org/1272673003/
--
--
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 an
https://codereview.chromium.org/1272673003/diff/230001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1272673003/diff/230001/src/parser.h#newcode1324
src/parser.h:1324: !is_rest && pattern->IsVariableProxy() && initializer
== nullptr;
On 2015/09/02 20:11:31, caitp wrote:
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1272673003/230001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1272673003/230001
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
https://codereview.chromium.org/1272673003/diff/230001/src/parser.h
File src/parser.h (right):
https://codereview.chromium.org/1272673003/diff/230001/src/parser.h#newcode1324
src/parser.h:1324: !is_rest && pattern->IsVariableProxy() && initializer
== nullptr;
This makes sure the empty string is
On 2015/09/02 00:55:27, rossberg wrote:
Parser changes LGTM, haven't looked at the rest.
Hey, I just made one quick adjustment to the parser, in addition to a few
updates for mips to get tests passing on the mips bots.
I'll leave an inline comment to explain it, it would be good to get a quick
Parser changes LGTM, haven't looked at the rest.
https://codereview.chromium.org/1272673003/
--
--
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
On 2015/09/01 23:37:10, commit-bot: I haz the power wrote:
Try jobs failed on following builders:
v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_rel/builds/4291)
This needs a few more changes for MIPS (w
Try jobs failed on following builders:
v8_linux_mips64el_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_mips64el_compile_rel/builds/4291)
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://gr
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1272673003/210001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1272673003/210001
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
On 2015/09/01 23:10:43, commit-bot: I haz the power wrote:
Try jobs failed on following builders:
v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/builds/7472)
v8_linux64_asan_rel on tryserver.v8 (JOB_FAI
Try jobs failed on following builders:
v8_android_arm_compile_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_android_arm_compile_rel/builds/7472)
v8_linux64_asan_rel on tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1272673003/190001
View timeline at
https://chromium-cq-status.appspot.com/patch-timeline/1272673003/190001
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
On 2015/09/01 16:03:18, wingo wrote:
LGTM, thanks for humoring my nitpicking, doing the perfect thing seems
impossible here.
Regarding desugaring: to my limited understanding there is no inlining
hazard;
TF will be able to do just as well with this desugaring as with indexed
arguments access
LGTM, thanks for humoring my nitpicking, doing the perfect thing seems
impossible here.
Regarding desugaring: to my limited understanding there is no inlining
hazard;
TF will be able to do just as well with this desugaring as with indexed
arguments access. It seems to me that TF will have to
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h#newcode34
src/preparser.h:34: mutable int rest_array_literal_index = -1;
On 2015/09/01 15:17:24, caitp wrote:
On 2015/09/01
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/150001/src/preparser.h#newcode34
src/preparser.h:34: mutable int rest_array_literal_index = -1;
On 2015/09/01 14:41:20, wingo wrote:
Is mutable ne
Some more nits. I defer to Adam's review of course.
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4187
src/parser.cc:4187: }
No need for the if, just execute the body
On 2015/09/01 04:02:15, Benedikt Meurer wrote:
On 2015/09/01 03:55:02, caitp wrote:
> https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
> File src/parser.cc (right):
>
>
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4415
> src/parser.cc:4415: //
On 2015/09/01 03:55:02, caitp wrote:
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4415
src/parser.cc:4415: // for (var $argument_index = $rest_index;
On 2015/09/01 0
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4415
src/parser.cc:4415: // for (var $argument_index = $rest_index;
On 2015/09/01 03:09:10, Benedikt Meurer wrote:
I guess
LGTM with question and nit.
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/150001/src/parser.cc#newcode4415
src/parser.cc:4415: // for (var $argument_index = $rest_index;
I guess there was some dis
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4422
src/parser.cc:4422: DCHECK_NULL(parameter.initializer);
On 2015/08/31 21:17:28, adamk wrote:
That'll get rid of this
lgtm % one more wingo comment I point to
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4414
src/parser.cc:4414: if (parameter.is_rest) {
On 2015/08/31 13:07:31, wingo w
On 2015/08/31 20:44:10, adamk wrote:
This is looking good to me, will look once more after you've handled the
rest
of
wingo's comments.
I believe these have been addressed in the last patchset
I agree that we should do something nicer with the literals count, it's
quite
messy at the mom
This is looking good to me, will look once more after you've handled the
rest of
wingo's comments.
I agree that we should do something nicer with the literals count, it's
quite
messy at the moment and not at all obvious why the count is valid in all
cases.
+bmeurer for full-codegen OWNERs
On 2015/08/31 13:54:09, wingo wrote:
Hi :)
On 2015/08/31 13:23:40, caitp wrote:
>
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4187
> src/parser.cc:4187: }
> On 2015/08/31 13:07:31, wingo wrote:
> > Is this bit specific to this patch or is it a general fix?
>
Hi :)
On 2015/08/31 13:23:40, caitp wrote:
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode4187
src/parser.cc:4187: }
On 2015/08/31 13:07:31, wingo wrote:
> Is this bit specific to this patch or is it a general fix?
Well, it's a mix. See https://code.google.com/p/v
On 2015/08/31 13:28:17, caitp wrote:
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h#newcode1044
src/preparser.h:1044: (IsBinaryOperation() &&
HasRestField::decode(code
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1272673003/diff/110001/src/preparser.h#newcode1044
src/preparser.h:1044: (IsBinaryOperation() &&
HasRestField::decode(code_));
On 2015/08/31 13:23:40, caitp wrote:
Thanks Andy, few more cleanup points here =)
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode3947
src/parser.cc:3947: ReportMessageAt(params_loc,
MessageTemplate::kMalfor
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/110001/src/parser.cc#newcode3947
src/parser.cc:3947: ReportMessageAt(params_loc,
MessageTemplate::kMalformedArrowFunParamList);
Why is this necessary?
I think this addresses all of the comments
https://codereview.chromium.org/1272673003/diff/60001/src/ast-expression-visitor.cc
File src/ast-expression-visitor.cc (right):
https://codereview.chromium.org/1272673003/diff/60001/src/ast-expression-visitor.cc#newcode206
src/ast-expression-visitor.cc
https://codereview.chromium.org/1272673003/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/60001/src/parser.cc#newcode4012
src/parser.cc:4012: for (const auto p : parameters.params) {
On 2015/08/28 23:58:01, caitp wrote:
On 2015/08/28 23:26:3
https://codereview.chromium.org/1272673003/diff/60001/src/ast-expression-visitor.cc
File src/ast-expression-visitor.cc (right):
https://codereview.chromium.org/1272673003/diff/60001/src/ast-expression-visitor.cc#newcode206
src/ast-expression-visitor.cc:206: void
AstExpressionVisitor::VisitRestPa
Excited to see this passing tests!
As you suspected, I'm a little bit skeptical of adding a new AST node for
RestParameter. I'm probably missing something, but it seems like a big
hammer
just to make reindexing work, as no other AST visitor will ever care about
it.
Not sure who else would b
Thanks, it's appreciated! Is there anyone else who should make sure I'm not
doing something stupid with the new ast node/visitor methods? Or are you good
with that part
> On Aug 28, 2015, at 11:47 AM, ad...@chromium.org wrote:
>
> I will take a look at this today (sorry I've been slow to respon
I will take a look at this today (sorry I've been slow to respond this week,
have been swamped with non-engineering tasks)
https://codereview.chromium.org/1272673003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because yo
Hey, can folks take another look at this? I need someone to review the new
AST
node added to simplify some stuff.
This now works with lazy parsing for both regular functions and arrows, so
that's pretty cool.
Summary of changes:
- New AST node for rest parameters, to make literal reindexing
On 2015/08/06 14:43:53, rossberg wrote:
I also tried digging into it, but without much success. Something must be
pointing into the wild, but I couldn't find out what.
Some cases fail with an invalid access. I'd try to breakpoint on those in
the
debugger and try to figure out what there point
I also tried digging into it, but without much success. Something must be
pointing into the wild, but I couldn't find out what.
Some cases fail with an invalid access. I'd try to breakpoint on those in
the
debugger and try to figure out what there pointing at and where it is coming
from. Maybe
On 2015/08/06 00:23:16, adamk wrote:
On 2015/08/05 23:36:00, adamk wrote:
> After hacking on this for awhile, I think at least part of the problem
is
the
> existing rest-handling code in scopes.cc, which I think should probably
be
> deleted. At the least, deleting the special handling of
S
On 2015/08/05 23:36:00, adamk wrote:
After hacking on this for awhile, I think at least part of the problem is
the
existing rest-handling code in scopes.cc, which I think should probably be
deleted. At the least, deleting the special handling of
Scope::rest_parameter_
makes arrow functions s
After hacking on this for awhile, I think at least part of the problem is
the
existing rest-handling code in scopes.cc, which I think should probably be
deleted. At the least, deleting the special handling of
Scope::rest_parameter_
makes arrow functions stop crashing and start failing.
https
Looking into the crash locally, but here's some stylistic comments in the
meantime.
https://codereview.chromium.org/1272673003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1272673003/diff/1/src/parser.cc#newcode4326
src/parser.cc:4326: auto parameter_proxy =
49 matches
Mail list logo
