LGTM, but I think the labels for breakable and continuable statements should be in the nesting stack (not in the call stack and pointed to by the nesting stack). It just seems simpler.
http://codereview.chromium.org/466033/diff/7001/7006 File src/fast-codegen.cc (right): http://codereview.chromium.org/466033/diff/7001/7006#newcode236 src/fast-codegen.cc:236: Breakable nested_statement(this, stmt, &end_of_block); If the label is part of the state of the Breakable, you could write Breakable nested_statement(this, stmt); http://codereview.chromium.org/466033/diff/7001/7006#newcode239 src/fast-codegen.cc:239: __ bind(&end_of_block); And __ bind(nested_statement.break_target()); http://codereview.chromium.org/466033/diff/7001/7006#newcode284 src/fast-codegen.cc:284: NestedStatement* current = nesting_stack_; Might be nice to have a codegen comment "[ ContinueStatement" here (and below for BreakStatement). http://codereview.chromium.org/466033/diff/7001/7006#newcode355 src/fast-codegen.cc:355: Label body, test, exit, stack_limit_hit, stack_check_success; exit and test could be fields of the Iteration. http://codereview.chromium.org/466033/diff/7001/7006#newcode369 src/fast-codegen.cc:369: __ bind(&test); __ bind(loop_statement.continue_target()); http://codereview.chromium.org/466033/diff/7001/7006#newcode384 src/fast-codegen.cc:384: __ bind(&exit); __ bind(loop_statement.break_target()); http://codereview.chromium.org/466033/diff/7001/7006#newcode400 src/fast-codegen.cc:400: Iteration loop_stamt(this, stmt, &exit, &test); loop_statement? loop_stmt? http://codereview.chromium.org/466033/diff/7001/7006#newcode440 src/fast-codegen.cc:440: Iteration loop_stamt(this, stmt, &exit, &next); loop_statement? http://codereview.chromium.org/466033/diff/7001/7006#newcode616 src/fast-codegen.cc:616: __ Drop(stack_depth); Good. I much prefer adjusting the stack pointer to point to the handler rather than loading the stack pointer from the global handler. http://codereview.chromium.org/466033/diff/7001/7007 File src/fast-codegen.h (right): http://codereview.chromium.org/466033/diff/7001/7007#newcode91 src/fast-codegen.h:91: // Generate code to leave the nested statement. This includes We should probably document (in several places) that the result register must be preserved by Exit. http://codereview.chromium.org/466033/diff/7001/7007#newcode106 src/fast-codegen.h:106: FastCodeGenerator *codegen_; Space after star. http://codereview.chromium.org/466033/diff/7001/7007#newcode107 src/fast-codegen.h:107: NestedStatement *previous_; Same. http://codereview.chromium.org/466033/diff/7001/7007#newcode124 src/fast-codegen.h:124: Statement* statement() { return target_; } Might make return type BreakableStatement* in case you want to call BreakableStatement methods on it. http://codereview.chromium.org/466033/diff/7001/7007#newcode128 src/fast-codegen.h:128: Label* break_target_label_; Instead of passing a label pointer to the constructor, you could just have a label as a field. http://codereview.chromium.org/466033 -- v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev
