On 2015/06/03 03:53:48, caitp wrote:
On 2015/06/02 16:05:00, caitp wrote:
> On 2015/06/02 14:55:18, caitp wrote:
> > On 2015/06/02 12:47:23, caitp wrote:
> > > The output from --trace-turbo-graph looks generally okay (at least
the
> > portions
> > > that I understand). So I'm not sure what's c
On 2015/06/02 16:05:00, caitp wrote:
On 2015/06/02 14:55:18, caitp wrote:
> On 2015/06/02 12:47:23, caitp wrote:
> > The output from --trace-turbo-graph looks generally okay (at least the
> portions
> > that I understand). So I'm not sure what's causing the arm64 sim test
to
fail.
> > https://
On 2015/06/02 14:55:18, caitp wrote:
On 2015/06/02 12:47:23, caitp wrote:
> The output from --trace-turbo-graph looks generally okay (at least the
portions
> that I understand). So I'm not sure what's causing the arm64 sim test to
fail.
> https://gist.githubusercontent.com/caitp/20f4ba77fc0d0303
On 2015/06/02 12:47:23, caitp wrote:
The output from --trace-turbo-graph looks generally okay (at least the
portions
that I understand). So I'm not sure what's causing the arm64 sim test to
fail.
https://gist.githubusercontent.com/caitp/20f4ba77fc0d03033552/raw/log for
a
(big) log with --tr
The output from --trace-turbo-graph looks generally okay (at least the
portions
that I understand). So I'm not sure what's causing the arm64 sim test to
fail.
https://gist.githubusercontent.com/caitp/20f4ba77fc0d03033552/raw/log for a
(big) log with --trace-turbo and --trace-turbo-graph.
ht
A revert of this CL (patchset #19 id:380001) has been created in
https://codereview.chromium.org/1163853002/ by caitpotte...@gmail.com.
The reason for reverting is: Broken on arm64.
https://codereview.chromium.org/1127063003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.googl
Committed patchset #19 (id:380001)
https://codereview.chromium.org/1127063003/
--
--
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
Patchset 19 (id:??) landed as
https://crrev.com/892c85485881f8be2f17bd83238980f858126576
Cr-Commit-Position: refs/heads/master@{#28739}
https://codereview.chromium.org/1127063003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this messa
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1127063003/380001
https://codereview.chromium.org/1127063003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subsc
CQ is trying da patch. Follow status at
https://chromium-cq-status.appspot.com/patch-status/1127063003/380001
https://codereview.chromium.org/1127063003/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subsc
https://codereview.chromium.org/1127063003/
--
--
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 it, sen
https://codereview.chromium.org/1127063003/diff/360001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/360001/src/parser.cc#newcode3440
src/parser.cc:3440: static const VariableMode kMode = LET;
On 2015/06/01 15:46:39, rossberg wrote:
Nit: drop this aux
lgtm
https://codereview.chromium.org/1127063003/diff/360001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/360001/src/parser.cc#newcode3440
src/parser.cc:3440: static const VariableMode kMode = LET;
Nit: drop this auxiliary
https://codereview.chromi
So, just to recap discussions from IRC:
This patch is the (as far as I can tell) simplest approach to implementing a
subset of the default parameters feature.
This subset includes:
- SingleNameBindings only, no BindingPattern support yet
- ReferenceError when parameters are referenced before ini
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h#newcode375
src/scopes.h:375: Variable* parameter(int index) const {
On 2015/05/26 20:59:11, caitp wrote:
On 2015/05/26 20:34:02, ada
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h#newcode375
src/scopes.h:375: Variable* parameter(int index) const {
On 2015/05/26 20:34:02, adamk wrote:
Is this called anywhere (be
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h
File src/scopes.h (right):
https://codereview.chromium.org/1127063003/diff/360001/src/scopes.h#newcode375
src/scopes.h:375: Variable* parameter(int index) const {
Is this called anywhere (besides your new caller in scopes.cc)? I
https://codereview.chromium.org/1127063003/
--
--
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 it, sen
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc#newcode1442
src/scopes.cc:1442: var = variables_.Lookup(var->raw_name());
On 2015/05/26 20:24:44, caitp wrote:
On 2015/05/26 20:0
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc#newcode1442
src/scopes.cc:1442: var = variables_.Lookup(var->raw_name());
On 2015/05/26 20:03:20, adamk wrote:
On 2015/05/26 17:1
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc#newcode1442
src/scopes.cc:1442: var = variables_.Lookup(var->raw_name());
On 2015/05/26 17:14:07, caitp wrote:
This is a hack to
Various style nits follow.
I think I'm about as uneasy as rossberg is about undeclaring variables.
https://codereview.chromium.org/1127063003/diff/31/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/31/src/parser.cc#newcode1146
src/parser.cc:114
https://codereview.chromium.org/1127063003/diff/280001/test/mjsunit/harmony/default-parameters-lazy.js
File test/mjsunit/harmony/default-parameters-lazy.js (right):
https://codereview.chromium.org/1127063003/diff/280001/test/mjsunit/harmony/default-parameters-lazy.js#newcode9
test/mjsunit/harmon
LGTM
This has some issues but lets revisit those in follow up CLs.
https://codereview.chromium.org/1127063003/diff/280001/test/mjsunit/harmony/default-parameters-lazy.js
File test/mjsunit/harmony/default-parameters-lazy.js (right):
https://codereview.chromium.org/1127063003/diff/280001/test/mj
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/280001/src/scopes.cc#newcode1442
src/scopes.cc:1442: var = variables_.Lookup(var->raw_name());
This is a hack to make sure the re-declared variables ar
On 2015/05/20 17:05:16, caitp wrote:
https://codereview.chromium.org/1127063003/diff/21/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/21/src/scopes.cc#newcode1433
src/scopes.cc:1433: if (IsLexicalVariableMode(var->mode())) {
So this approach
https://codereview.chromium.org/1127063003/diff/21/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/21/src/scopes.cc#newcode1433
src/scopes.cc:1433: if (IsLexicalVariableMode(var->mode())) {
So this approach doesn't work --- it causes the
`DCHECK(
https://codereview.chromium.org/1127063003/diff/11/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/scopes.cc#newcode513
src/scopes.cc:513: void Scope::ShadowParametersForExpressions() {
On 2015/05/20 11:29:38, caitp wrote:
On 2015/05/20 0
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc#newcode3575
src/parser.cc:3575: proxy->var()->set_maybe_assigned();
On 2015/05/20 11:29:38, caitp wrote:
On 2015/05/20 07:41:47,
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc#newcode3575
src/parser.cc:3575: proxy->var()->set_maybe_assigned();
On 2015/05/20 07:41:47, rossberg wrote:
On 2015/05/12 14:04:5
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc#newcode3575
src/parser.cc:3575: proxy->var()->set_maybe_assigned();
On 2015/05/12 14:04:53, rossberg wrote:
Why is this set?
Pi
https://codereview.chromium.org/1127063003/diff/11/test/mjsunit/harmony/optional-arguments.js
File test/mjsunit/harmony/optional-arguments.js (right):
https://codereview.chromium.org/1127063003/diff/11/test/mjsunit/harmony/optional-arguments.js#newcode55
test/mjsunit/harmony/optional-arg
Thanks
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/parser.cc#newcode4301
src/parser.cc:4301: body->AddAll(*inner_body, zone());
On 2015/05/12 14:04:53, rossberg wrote:
Shouldn't this
https://codereview.chromium.org/1127063003/diff/11/src/bootstrapper.cc
File src/bootstrapper.cc (right):
https://codereview.chromium.org/1127063003/diff/11/src/bootstrapper.cc#newcode1730
src/bootstrapper.cc:1730:
EMPTY_NATIVE_FUNCTIONS_FOR_FEATURE(harmony_optional_params)
Nit: harmony_d
On 2015/05/12 07:32:11, wingo wrote:
On 2015/05/11 16:58:19, caitp wrote:
> The preparser logs locations to be lazily parsed, we then parse (lazily)
with
> the full parser, with a really weird (and incorrect) scope tree, at
which
point
> ParseFormalParameterList is used on a scope which is no
On 2015/05/11 16:58:19, caitp wrote:
The preparser logs locations to be lazily parsed, we then parse (lazily)
with
the full parser, with a really weird (and incorrect) scope tree, at which
point
ParseFormalParameterList is used on a scope which is not a function scope.
That's what I'm talking
On 2015/05/11 16:53:21, wingo wrote:
On 2015/05/11 15:34:25, caitp wrote:
> https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
> File src/preparser.h (right):
>
>
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3541
> src/preparser.h:3541: DCHECK
On 2015/05/11 15:34:25, caitp wrote:
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3541
src/preparser.h:3541: DCHECK(scope_->is_function_scope());
On 2015/05/11 1
https://codereview.chromium.org/1127063003/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/60001/src/parser.cc#newcode1153
src/parser.cc:1153: ParseFormalParameter(scope, &error_locs, nullptr,
nullptr, has_rest,
On 2015/05/11 15:00:48, wingo w
One other little thought
https://codereview.chromium.org/1127063003/diff/60001/src/scopes.cc
File src/scopes.cc (right):
https://codereview.chromium.org/1127063003/diff/60001/src/scopes.cc#newcode468
src/scopes.cc:468: param_positions_.Add(pos, zone());
Don't we require already that parameters
https://codereview.chromium.org/1127063003/diff/60001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/60001/src/parser.cc#newcode1153
src/parser.cc:1153: ParseFormalParameter(scope, &error_locs, nullptr,
nullptr, has_rest,
nit: would be easier to read wi
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3492
src/preparser.h:3492: if (initializer && allow_harmony_optional_params()
&& Check(Token::ASSIGN)) {
On 2015/05/11
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3492
src/preparser.h:3492: if (initializer && allow_harmony_optional_params()
&& Check(Token::ASSIGN)) {
On 2015/05/11
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3492
src/preparser.h:3492: if (initializer && allow_harmony_optional_params()
&& Check(Token::ASSIGN)) {
This looks str
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h
File src/preparser.h (right):
https://codereview.chromium.org/1127063003/diff/60001/src/preparser.h#newcode3541
src/preparser.h:3541: DCHECK(scope_->is_function_scope());
Apparently this is a problem for lazy-parsed arrow func
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc#newcode3561
src/parser.cc:3561: Runtime::FunctionForId(Runtime::kInlineArguments);
On 2015/05/06 20:32:38, arv wrote:
I think Andreas wante
Great incremental first step
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/1/src/parser.cc#newcode3538
src/parser.cc:3538:
How about adding a comment block explaining how the desugaring is done?
https
This actually breaks rest parameters when used in conjunction with default
arguments, due to the ShadowParametersForInitializers() thing. I think
that's
okay, because it will be fixed subsequently when rest params are implemented
with desugaring
https://codereview.chromium.org/1127063003/
--
48 matches
Mail list logo
