Remove some of the state-dependent behavior from the code generator....

Remove some of the state-dependent behavior from the code generator.
Simplify the code generator by eliminating the access types STORE and
INIT_CONST and delegating code generation for stores to the appropriate AST
nodes.

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

[iposva, 2008-09-10 18:01:44]

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.

[Kevin Millikin (Chromium), 2008-09-11 07:15:51]

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) {
On 2008/09/10 18:01:44, iposva wrote:
> Can you please use an enum for the is_const_init to improve the readability at
> the caller sites?

Done.  InitState with values CONST_INIT and NOT_CONST_INIT defined in ast.h for
now, but will move to a more logical place as the codegen state evolves.

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());
On 2008/09/10 18:01:44, iposva wrote:
> Nit: Don't you want to assert before accessing any fields of ref?

Changed.  Illegal references have a type, so it's safe to access it.  But it
*is* clearer to read the preconditions earlier in the function.

http://codereview.chromium.org/1889/diff/1/3#newcode3295
Line 3295: // CodeGenState::LOAD_TYPEOF_EXPR.
On 2008/09/10 18:01:44, iposva wrote:
> 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.

Done.  Earlier assert already ruled out UNDEFINED, but the point about new
states is valid.

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());
On 2008/09/10 18:01:44, iposva wrote:
> Same nit as on ARM: Don't you want to assert before accessing ref?

Done.

http://codereview.chromium.org/1889/diff/1/4#newcode3496
Line 3496: // CodeGenState::LOAD_TYPEOF_EXPR.
On 2008/09/10 18:01:44, iposva wrote:
> 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.

Done.