Simplfy handling of exits from scoped blocks.

Simplfy handling of exits from scoped blocks.


BUG=
TEST=mjsunit/harmony/block-leave.js

Committed: http://code.google.com/p/v8/source/detail?r=9157

[Steven, 2011-09-02 15:07:21]

PTAL.

[Kevin Millikin (Chromium), 2011-09-02 15:17:43]

LGTM, with a few comments below and a suggestion for a simple cleanup as a
separate change.

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h
File src/full-codegen.h (right):

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h#newcode197
src/full-codegen.h:197: explicit NestedBlock(FullCodeGenerator* codegen, Block*
block)
No need for explicit for two argument constructors.

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h#newcode209
src/full-codegen.h:209: Block* block_;
This class doesn't need to store the block since the base class does.  You
probably will just need a cast:

virtual NestedStatement* Exit(int* stack_depth, int* context_length) {
  if (statement()->AsBlock()->block_scope() != NULL) ...

http://codereview.chromium.org/7792100/diff/1/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/7792100/diff/1/src/parser.cc#newcode1593
src/parser.cc:1593: exit->AddStatement(new(zone()) ExitContextStatement());
Now we're no longer making essential use of ExitContextStatement (it's
completely predictable where it will occur).

As a next change, can you get rid of it?  'With' already makes do without it by
compiling the code implicitly at the end of the body.  Catch and blocks with
scopes could do the same.

[Steven, 2011-09-05 08:54:49]

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h
File src/full-codegen.h (right):

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h#newcode197
src/full-codegen.h:197: explicit NestedBlock(FullCodeGenerator* codegen, Block*
block)
On 2011/09/02 15:17:44, Kevin Millikin wrote:
> No need for explicit for two argument constructors.

Done.

http://codereview.chromium.org/7792100/diff/1/src/full-codegen.h#newcode209
src/full-codegen.h:209: Block* block_;
On 2011/09/02 15:17:44, Kevin Millikin wrote:
> This class doesn't need to store the block since the base class does.  You
> probably will just need a cast:
> 
> virtual NestedStatement* Exit(int* stack_depth, int* context_length) {
>   if (statement()->AsBlock()->block_scope() != NULL) ...

Done.

http://codereview.chromium.org/7792100/diff/1/src/parser.cc
File src/parser.cc (right):

http://codereview.chromium.org/7792100/diff/1/src/parser.cc#newcode1593
src/parser.cc:1593: exit->AddStatement(new(zone()) ExitContextStatement());
Will do it.
On 2011/09/02 15:17:44, Kevin Millikin wrote:
> Now we're no longer making essential use of ExitContextStatement (it's
> completely predictable where it will occur).
> 
> As a next change, can you get rid of it?  'With' already makes do without it
by
> compiling the code implicitly at the end of the body.  Catch and blocks with
> scopes could do the same.