LGTM, mostly nits
https://codereview.chromium.org/307593002/diff/20001/src/ast.h
File src/ast.h (right):
https://codereview.chromium.org/307593002/diff/20001/src/ast.h#newcode444
src/ast.h:444: const BailoutId DeclsId() const { return decls_id_; }
nit: the first const here does not have any meaning.
https://codereview.chromium.org/307593002/diff/20001/src/hydrogen.cc
File src/hydrogen.cc (right):
https://codereview.chromium.org/307593002/diff/20001/src/hydrogen.cc#newcode4283
src/hydrogen.cc:4283: set_scope(scope);
Hm, can pushing/popping the scope perhaps be factored into
BreakAndContinueScope, or a separate BlockScope abstraction?
https://codereview.chromium.org/307593002/diff/20001/src/ia32/lithium-codegen-ia32.cc
File src/ia32/lithium-codegen-ia32.cc (right):
https://codereview.chromium.org/307593002/diff/20001/src/ia32/lithium-codegen-ia32.cc#newcode5673
src/ia32/lithium-codegen-ia32.cc:5673:
CallRuntime(Runtime::kHiddenPushBlockContext, 2, instr);
On 2014/06/03 23:12:20, Michael Starzinger wrote:
Since this is only a runtime call, do we really need a separate
HInstruction for
it? Could we use HCallRuntime instead? Or are there any plans to
optimize
HAllocateBlockContext in the near future?
Perhaps not in the near future, but we will have to do it eventually,
presumably in the not so far future. That said, I don't have an opinion
what to prefer right now.
https://codereview.chromium.org/307593002/diff/20001/test/mjsunit/harmony/block-let-crankshaft.js
File test/mjsunit/harmony/block-let-crankshaft.js (right):
https://codereview.chromium.org/307593002/diff/20001/test/mjsunit/harmony/block-let-crankshaft.js#newcode36
test/mjsunit/harmony/block-let-crankshaft.js:36: f27, f28, f29, f30,
f31, f32, f33 ];
Perhaps add a test for throwing out of a crankshaft'ed function inside a
nested block, just in case.
https://codereview.chromium.org/307593002/diff/20001/test/mjsunit/harmony/block-scoping.js
File test/mjsunit/harmony/block-scoping.js (right):
https://codereview.chromium.org/307593002/diff/20001/test/mjsunit/harmony/block-scoping.js#newcode31
test/mjsunit/harmony/block-scoping.js:31: // TODO(ES6): properly
activate extended mode
Btw, we can remove this TODO now.
https://codereview.chromium.org/307593002/diff/20001/test/mjsunit/harmony/block-scoping.js#newcode67
test/mjsunit/harmony/block-scoping.js:67:
Why not assert optimization status here as well (and below)?
https://codereview.chromium.org/307593002/
--
--
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.
