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:14:07, caitp wrote:
> This is a hack to make sure the re-declared variables are properly
context
> allocated. It's finicky, and I don't think it's quite right, but I'm
not 100%
> sure how to verify it.
>
> So this is really the area I'm most unsure about atm
Can we just skip the loop over params_ in AllocateParameterLocals() if
has_parameter_expressions_ is set? It seems like all the Variables in
there are
bogus at this point if that bit is set (since they got "undeclared").
Maybe, but there are a few things that might go wrong
- The redeclared variable might need to be context allocated, which
probably won't happen without this check
- Stack and heap slot counts need to be what they're expected to be so
that scopeinfo ends up usable and sane
I'll try that after the next PS and see if it works reasonably well
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc
File src/parser.cc (right):
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode1146
src/parser.cc:1146: bool hasParameterExpressions = false;
On 2015/05/26 19:58:10, adamk wrote:
Same style nit, use_underscores. I think this shows up a bunch of
other places,
should be an easy search/replace.
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode1147
src/parser.cc:1147: ZoneList<Expression*>* initializers =
On 2015/05/26 19:58:10, adamk wrote:
Please add a TODO here, since you're just dropping this on the floor.
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3424
src/parser.cc:3424: ZoneList<Statement*>* body = new (zone())
ZoneList<Statement*>(0, zone());
On 2015/05/26 19:58:10, adamk wrote:
Don't you know how big a list you need here (initializers->length())?
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3431
src/parser.cc:3431: static const int capacity = 1;
On 2015/05/26 19:58:10, adamk wrote:
More style nits: kCapacity
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3432
src/parser.cc:3432: static const bool is_initializer_block = true; // ?
On 2015/05/26 19:58:10, adamk wrote:
kIsInitializerBlock.
Also the "// ?" should either be gone or better explained.
Done. The "?" comment is a reminder to ask what the flag actually means
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3436
src/parser.cc:3436: static const VariableMode mode = LET;
On 2015/05/26 19:58:10, adamk wrote:
kMode (though I'm not sure this adds much)
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3452
src/parser.cc:3452: new (zone()) ZoneList<Expression*>(0, zone());
On 2015/05/26 19:58:10, adamk wrote:
s/0/1/ for the capacity arg here.
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3456
src/parser.cc:3456: if (initializer) {
On 2015/05/26 19:58:10, adamk wrote:
if (initializer != nullptr)
Done.
https://codereview.chromium.org/1127063003/diff/300001/src/parser.cc#newcode3459
src/parser.cc:3459: new (zone()) ZoneList<Expression*>(0, zone());
On 2015/05/26 19:58:10, adamk wrote:
s/0/1/ again.
Done.
https://codereview.chromium.org/1127063003/
--
--
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.
