Kevin, For now it is OK to have the GenerateStoreCode methods take a scope parameter. I can envision that at some point these methods might need to take a CodeGenerator parameter to tailor the code based on particular state of the compilation such as variable or frame layout. Such information is not conveyed as part of the Scope.
-Ivan http://codereview.chromium.org/1889/diff/1/2 File src/ast.h (right): http://codereview.chromium.org/1889/diff/1/2#newcode164 Line 164: bool is_const_init) { Can you please use an enum for the is_const_init to improve the readability at the caller sites? http://codereview.chromium.org/1889/diff/1/3 File src/codegen-arm.cc (right): http://codereview.chromium.org/1889/diff/1/3#newcode2033 Line 2033: ASSERT(!ref->is_illegal()); Nit: Don't you want to assert before accessing any fields of ref? http://codereview.chromium.org/1889/diff/1/3#newcode3295 Line 3295: // CodeGenState::LOAD_TYPEOF_EXPR. Please assert that the CodeGenState is LOAD_TYPEOF_EXPR. That way you ensure that you are not entering here with UNDEFINED or a newly added state. http://codereview.chromium.org/1889/diff/1/4 File src/codegen-ia32.cc (right): http://codereview.chromium.org/1889/diff/1/4#newcode1141 Line 1141: ASSERT(!ref()->is_illegal()); Same nit as on ARM: Don't you want to assert before accessing ref? http://codereview.chromium.org/1889/diff/1/4#newcode3496 Line 3496: // CodeGenState::LOAD_TYPEOF_EXPR. Same comment as on ARM: Please assert that the CodeGenState is LOAD_TYPEOF_EXPR. That way you ensure that you are not entering here with UNDEFINED or a newly added state. http://codereview.chromium.org/1889 --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
