LGTM.

http://codereview.chromium.org/501076/diff/1/3
File src/compiler.cc (right):

http://codereview.chromium.org/501076/diff/1/3#newcode881
src/compiler.cc:881: // Supported.
It seems right to visit the subexpressions in a value context.  We will
generate code for them, and may eventually want to do something here.

http://codereview.chromium.org/501076/diff/1/4
File src/fast-codegen.cc (right):

http://codereview.chromium.org/501076/diff/1/4#newcode463
src/fast-codegen.cc:463: Label setup_try_handler, catch_entry, done;
This isn't as complicated as TryFinally, but I miss the nice big comment
explaining what's going on here.

http://codereview.chromium.org/501076/diff/1/4#newcode465
src/fast-codegen.cc:465: __ Call(&setup_try_handler);
You called this 'try_handler_setup' in TryFinally :)

http://codereview.chromium.org/501076/diff/1/4#newcode473
src/fast-codegen.cc:473: ASSERT_EQ(Variable::TEMPORARY,
catch_var->mode());
I think this is true, but the code doesn't depend on it.

http://codereview.chromium.org/501076/diff/1/4#newcode474
src/fast-codegen.cc:474: Slot* variable_slot =
catch_var->rewrite()->AsSlot();
No need for catch_var->rewrite()->AsSlot(), catch_var->slot() will do
the same thing.  (Using rewrite() just makes it a bit harder to get rid
of rewrites.  I thinking getting rid of rewrites would be a good thing.)

http://codereview.chromium.org/501076/diff/1/4#newcode707
src/fast-codegen.cc:707: Move(Expression::kValue, expr->key());
Seems just as simple to compile the key and variable.

Visit(expr->key());
Visit(expr->value());

(Asserting they have the right expreession context to be sure).

http://codereview.chromium.org/501076/diff/1/4#newcode729
src/fast-codegen.cc:729: Visit(expr->exception());
Might assert the expression context is value.

http://codereview.chromium.org/501076

-- 
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to